Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.access.ExceptionTranslationFilter;
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.cors.CorsConfigurationSource;
import org.springframework.web.cors.UrlBasedCorsConfigurationSource;

import static com.example.solidconnection.type.Role.ADMIN;

@Configuration
@EnableWebSecurity
@RequiredArgsConstructor
Expand Down Expand Up @@ -55,10 +58,13 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.formLogin(AbstractHttpConfigurer::disable)
.cors(corsConfigurer -> corsConfigurer.configurationSource(corsConfigurationSource()))
.sessionManagement((session) -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
.authorizeHttpRequests(auth -> auth.anyRequest().permitAll())
.authorizeHttpRequests(auth -> auth
.requestMatchers("/admin/**").hasRole(ADMIN.name())
.anyRequest().permitAll()
)
.addFilterBefore(jwtAuthenticationFilter, BasicAuthenticationFilter.class)
.addFilterBefore(signOutCheckFilter, JwtAuthenticationFilter.class)
.addFilterBefore(exceptionHandlerFilter, SignOutCheckFilter.class)
.addFilterAfter(exceptionHandlerFilter, ExceptionTranslationFilter.class)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public enum ErrorCode {
AUTHENTICATION_FAILED(HttpStatus.UNAUTHORIZED.value(), "인증이 필요한 접근입니다."),
ACCESS_TOKEN_EXPIRED(HttpStatus.UNAUTHORIZED.value(), "액세스 토큰이 만료되었습니다. 재발급 api를 호출해주세요."),
REFRESH_TOKEN_EXPIRED(HttpStatus.UNAUTHORIZED.value(), "리프레시 토큰이 만료되었습니다. 다시 로그인을 진행해주세요."),
ACCESS_DENIED(HttpStatus.FORBIDDEN.value(), "접근 권한이 없습니다."),

// s3
S3_SERVICE_EXCEPTION(HttpStatus.BAD_REQUEST.value(), "S3 서비스 에러 발생"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package com.example.solidconnection.custom.security.authentication;

import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.security.core.userdetails.UserDetails;

import java.util.Collections;

public abstract class JwtAuthentication extends AbstractAuthenticationToken {

Expand All @@ -9,7 +12,9 @@ public abstract class JwtAuthentication extends AbstractAuthenticationToken {
private final Object principal;

public JwtAuthentication(String token, Object principal) {
super(null);
super(principal instanceof UserDetails ?
((UserDetails) principal).getAuthorities() :
Collections.emptyList());
Comment thread
nayonsoso marked this conversation as resolved.
this.credentials = token;
this.principal = principal;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.example.solidconnection.custom.security.filter;

import com.example.solidconnection.custom.exception.CustomException;
import com.example.solidconnection.custom.exception.ErrorCode;
import com.example.solidconnection.custom.response.ErrorResponse;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.FilterChain;
Expand All @@ -9,12 +10,16 @@
import jakarta.servlet.http.HttpServletResponse;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;

import java.io.IOException;

import static com.example.solidconnection.custom.exception.ErrorCode.ACCESS_DENIED;
import static com.example.solidconnection.custom.exception.ErrorCode.AUTHENTICATION_FAILED;

@Component
Expand All @@ -31,25 +36,28 @@ protected void doFilterInternal(@NonNull HttpServletRequest request,
filterChain.doFilter(request, response);
} catch (CustomException e) {
customCommence(response, e);
} catch (AccessDeniedException e) {
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
ErrorCode errorCode = auth instanceof AnonymousAuthenticationToken ? AUTHENTICATION_FAILED : ACCESS_DENIED;
generalCommence(response, e, errorCode);
} catch (Exception e) {
generalCommence(response, e);
generalCommence(response, e, AUTHENTICATION_FAILED);
}
}

public void customCommence(HttpServletResponse response, CustomException customException) throws IOException {
SecurityContextHolder.clearContext();
ErrorResponse errorResponse = new ErrorResponse(customException);
writeResponse(response, errorResponse);
writeResponse(response, errorResponse, customException.getCode());
}

public void generalCommence(HttpServletResponse response, Exception exception) throws IOException {
SecurityContextHolder.clearContext();
ErrorResponse errorResponse = new ErrorResponse(AUTHENTICATION_FAILED, exception.getMessage());
writeResponse(response, errorResponse);
public void generalCommence(HttpServletResponse response, Exception exception, ErrorCode errorCode) throws IOException {
ErrorResponse errorResponse = new ErrorResponse(errorCode, exception.getMessage());
writeResponse(response, errorResponse, errorCode.getCode());
Comment on lines 48 to +55
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다시 보니 customCommence(), generalCommence() 이 함수들 접근제한자도 private 이 되는게 좋겠네요🥹

}

private void writeResponse(HttpServletResponse response, ErrorResponse errorResponse) throws IOException {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
private void writeResponse(HttpServletResponse response, ErrorResponse errorResponse, int statusCode) throws IOException {
SecurityContextHolder.clearContext();
response.setStatus(statusCode);
Comment on lines +63 to +60
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이제야 보이는 부분이긴 한데😅

SecurityContextHolder.clearContext(); 를
writeResponse() 함수 안으로 옮기는건 어떤가요?
중복되는 코드 같아서요!

response.setContentType("application/json");
response.setCharacterEncoding("UTF-8");
response.getWriter().write(objectMapper.writeValueAsString(errorResponse));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.example.solidconnection.custom.security.mapper;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 클래스는 SiteUserDetails 에서만 사용되니
security.mapper 가 아니라 security.userdetails 패키지에 있는건 어떤가요?


import com.example.solidconnection.type.Role;
import org.springframework.security.core.authority.SimpleGrantedAuthority;

import java.util.List;

public class SecurityRoleMapper {

private static final String ROLE_PREFIX = "ROLE_";

private SecurityRoleMapper() {
}
Comment on lines +12 to +
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인스턴스 생성 방지 좋네요😊


public static List<SimpleGrantedAuthority> mapRoleToAuthorities(Role role) {
return List.of(new SimpleGrantedAuthority(ROLE_PREFIX + role.name()));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.example.solidconnection.custom.security.userdetails;

import com.example.solidconnection.custom.security.mapper.SecurityRoleMapper;
import com.example.solidconnection.siteuser.domain.SiteUser;
import lombok.Getter;
import org.springframework.security.core.GrantedAuthority;
Expand Down Expand Up @@ -27,7 +28,7 @@ public String getUsername() {

@Override
public Collection<? extends GrantedAuthority> getAuthorities() {
return null;
return SecurityRoleMapper.mapRoleToAuthorities(siteUser.getRole());
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/example/solidconnection/type/Role.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.example.solidconnection.type;

public enum Role {

ADMIN,
MENTOR,
MENTEE
MENTEE;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE site_user
modify ROLE enum ('MENTEE', 'MENTOR', 'ADMIN') NOT NULL;
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.context.SecurityContextHolder;

import java.util.stream.Stream;
Expand Down Expand Up @@ -82,10 +85,46 @@ void setUp() {
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
}

@Test
void 익명_사용자의_접근_거부시_401_예외_응답을_반환한다() throws Exception {
// given
Authentication anonymousAuth = getAnonymousAuth();
SecurityContextHolder.getContext().setAuthentication(anonymousAuth);
willThrow(new AccessDeniedException("Access Denied")).given(filterChain).doFilter(request, response);

// when
exceptionHandlerFilter.doFilterInternal(request, response, filterChain);

// then
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
}

@Test
void 인증된_사용자의_접근_거부하면_403_예외_응답을_반환한다() throws Exception {
// given
Authentication auth = new TestingAuthenticationToken("user", "password", "ROLE_USER");
SecurityContextHolder.getContext().setAuthentication(auth);
willThrow(new AccessDeniedException("Access Denied")).given(filterChain).doFilter(request, response);

// when
exceptionHandlerFilter.doFilterInternal(request, response, filterChain);

// then
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
}

private static Stream<Throwable> provideException() {
return Stream.of(
new RuntimeException(),
new CustomException(ErrorCode.INVALID_TOKEN)
);
}

private Authentication getAnonymousAuth() {
return new AnonymousAuthenticationToken(
"key",
"anonymousUser",
AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ class SiteUserDetailsServiceTest {
);
}

@Test
void 사용자_권한_정보를_정상적으로_반환한다() {
// given
SiteUser siteUser = siteUserRepository.save(createSiteUser());
String username = getUserName(siteUser);
Copy link
Copy Markdown
Collaborator

@nayonsoso nayonsoso Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SiteUserDetailsServiceTest 에서 SiteUserDetails의 getAuthorities() 함수 동작을 테스트하지 않고, SiteUserDetailsTest 에서 "어떤 사용자의 권한을 getAuthorities() 로 반환한다" 는 내용을 테스트하는건 어떤가요?
그게 더 작은 단위의 테스트가 될 것 같아요!


// when
SiteUserDetails userDetails = (SiteUserDetails) userDetailsService.loadUserByUsername(username);

// then
assertThat(userDetails.getAuthorities())
.extracting("authority")
.containsExactly("ROLE_" + siteUser.getRole().name());
}

@Nested
class 예외_응답을_반환한다 {

Expand Down