Skip to content

최건위 sprint10 - #226

Open
geoni-98 wants to merge 2 commits into
codeit-bootcamp-spring:최건위from
geoni-98:최건위-sprint10

Hidden character warning

The head ref may contain hidden characters: "\ucd5c\uac74\uc704-sprint10"
Open

geoni-98 wants to merge 2 commits into
codeit-bootcamp-spring:최건위from
geoni-98:최건위-sprint10

Conversation

@geoni-98

Copy link
Copy Markdown
Collaborator

요구사항

기본

  • 기본 항목 1
  • 기본 항목 2

심화

  • 심화 항목 1
  • 심화 항목 2

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@geoni-98
geoni-98 requested a review from yerin-mentor May 29, 2026 12:28
@yerin-mentor

Copy link
Copy Markdown
Collaborator

@geoni-98 건위님 pr conflicts 해결해주세요 ~

@yerin-mentor yerin-mentor left a comment

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.

안녕하세요 건위님, 과제 잘 진행해 주셨네요 👍

아래 리뷰 드린것들만 확인 해보시면 좋을것 같아요.

과제하시느라 수고하셨습니다 :)


response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
ErrorResponse errorResponse = new ErrorResponse(
new RuntimeException("Authentication failed: Invalid user details"),

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.

RuntimeException 보단 customException을 사용하는걸 권장드립니다.

Comment on lines +112 to +117
private Cookie createRefreshTokenCookie(String refreshToken) {
Cookie cookie = new Cookie(JwtTokenProvider.REFRESH_TOKEN_COOKIE_NAME, refreshToken);
cookie.setHttpOnly(true);
cookie.setPath("/");
cookie.setMaxAge(REFRESH_TOKEN_COOKIE_MAX_AGE);
return cookie;

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.

CSFP 방어 정책에 따라 SameSite 설정도 고려해보면 좋을것 같습니다.

.flatMap(Queue::stream)
.anyMatch(jwtInformation -> isActive(jwtInformation)
&& jwtInformation.accessToken().equals(accessToken));
}

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.

모든 사용자의 모든 토큰을 순회하는것 같습니다.
유저 수가 많아질 경우 조회 비용이 커질 수 있겠네요 🤔

Comment on lines +94 to 102
@PutMapping("role")
public ResponseEntity<UserDto> updateRole(@RequestBody RoleUpdateRequest request) {
log.info("권한 수정 요청");
UserDto userDto = authService.updateRole(request);

return ResponseEntity
.status(HttpStatus.OK)
.body(userDto);
}

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.

권한 변경시 강제 로그아웃 구현 요구사항이 있지 않았나요? 확인 후 추가 부탁드립니다~

Comment on lines +31 to +33
private static final String TOKEN_TYPE_CLAIM = "tokenType";
private static final String ACCESS_TOKEN_TYPE = "ACCESS";
private static final String REFRESH_TOKEN_TYPE = "REFRESH";

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.

토큰 타입을 상수로 정의하고 관리 해주셨네요 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants