feat: 관리자 role 추가 #192
feat: 관리자 role 추가 #192
Conversation
- JwtAuthentication 생성자에서 권한 정보를 상위 클래스에 전달하도록 수정
- JwtAuthentication 생성자에서 권한 정보 처리 로직 수정
src/main/java/com/example/solidconnection/custom/security/filter/ExceptionHandlerFilter.java
Outdated
Show resolved
Hide resolved
| private void writeResponse(HttpServletResponse response, ErrorResponse errorResponse, int statusCode) throws IOException { | ||
| response.setStatus(statusCode); |
There was a problem hiding this comment.
이제야 보이는 부분이긴 한데😅
SecurityContextHolder.clearContext(); 를
writeResponse() 함수 안으로 옮기는건 어떤가요?
중복되는 코드 같아서요!
src/main/java/com/example/solidconnection/custom/security/filter/ExceptionHandlerFilter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/custom/security/authentication/JwtAuthentication.java
Show resolved
Hide resolved
| private static final String ROLE_PREFIX = "ROLE_"; | ||
|
|
||
| public List<SimpleGrantedAuthority> getAuthorities() { | ||
| return List.of(new SimpleGrantedAuthority(ROLE_PREFIX + name())); | ||
| } |
There was a problem hiding this comment.
정말 코드를 잘 작성해주셨는데, 이것보다 더 좋은 구조가 있을 것 같아요.
지금은 도메인이 스프링 시큐리티를 알고 있어요.
도메인은 최대한 프레임워크에 대한 정보를 모르는게 좋아요.
그리고 스프링 시큐리티와 관련된 코드는 한군데에 모아지는게 좋을 것 같아요!
Role 객체에서 String ROLE_PREFIX + name() 만 반환하게 하고,
UserDetails 에서 SimpleGrantedAuthority 객체를 만들도록 바꾸게 하거나,
아니면 전용 매퍼 클래스를 두는 것도 괜찮다 생각해요.
지금보다는 Role이라는 도메인과 스프링 시큐리티가 덜 결합되면 좋겠어요🥺
| private SecurityRoleMapper() { | ||
| } |
| @@ -0,0 +1,18 @@ | |||
| package com.example.solidconnection.custom.security.mapper; | |||
There was a problem hiding this comment.
이 클래스는 SiteUserDetails 에서만 사용되니
security.mapper 가 아니라 security.userdetails 패키지에 있는건 어떤가요?
| @Test | ||
| void 사용자_권한_정보를_정상적으로_반환한다() { | ||
| // given | ||
| SiteUser siteUser = siteUserRepository.save(createSiteUser()); | ||
| String username = getUserName(siteUser); |
There was a problem hiding this comment.
SiteUserDetailsServiceTest 에서 SiteUserDetails의 getAuthorities() 함수 동작을 테스트하지 않고, SiteUserDetailsTest 에서 "어떤 사용자의 권한을 getAuthorities() 로 반환한다" 는 내용을 테스트하는건 어떤가요?
그게 더 작은 단위의 테스트가 될 것 같아요!
| 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()); |
There was a problem hiding this comment.
다시 보니 customCommence(), generalCommence() 이 함수들 접근제한자도 private 이 되는게 좋겠네요🥹
관련 이슈
작업 내용
Role에 ADMIN 추가 및 Spring Security 권한 처리 추가하였습니다.
Security 필터 체인 실행 순서를 조정하였습니다.
인증되지 않은 사용자가 관리자 api 호출 시 401, 권한 없는 사용자가 관리자 api 호출 시 403 에러를 반환하도록 하였습니다.
특이 사항
어드민 인가 관련 예외처리를 SecurityFilterChain에서 한 번에 하도록 하였는데 이 처리를 하는 필터인 ExceptionTranslationFilter는 커스텀으로 정의한 ExceptionHandlerFilter 필터보다 뒤에 있어 ExceptionHandlerFilter에서 예외를 처리할 수 없었습니다. 그래서
.addFilterAfter(exceptionHandlerFilter, ExceptionTranslationFilter.class)필터 순서를 ExceptionTranslationFilter 이후로 조정하였습니다.
참고자료
Security Filters