feat: 로그아웃 기능 구현 #493

coli-geonwoo · 2024-09-06T19:25:10Z

🚩 연관 이슈

close #478

📝 작업 내용

feat: 로그아웃 기능 구현 #478

🏞️ 스크린샷 (선택)

🗣️ 리뷰 요구사항 (선택)

각 상황에서 에러 코드가 적합한지 궁금합니다.
만료된 액세스 토큰으로 로그아웃 시도 시 > 400 반환
만료된 리프레시 토큰으로 로그아웃 시도 시 > 400 반환
액세스 토큰으로 파싱된 멤버와 리프레시 토큰 정보 불일치 > 400 반환
리프레시 토큰을 삭제시켜 로그아웃을 처리하고 있습니다.
안드로이드 측에서는 로그아웃 요청에 200을 응답받으면 기기에서 액세스 토큰을 삭제합니다.
다만, 이 상황에서도 액세스 토큰 하이재킹의 우려가 있습니다. 이에 따라 BlackList Entity를 만들어 로그아웃했지만 아직 유효한 accessToken, refreshToken 등을 관리하는 refrence가 많았어요. 안전성이나 보안측면에서요! 그러나, 이 방법은 토큰을 세션처럼 사용하는 느낌이 강하고 새로운 스키마 도입은 합의가 필요한 부분이라 생각했습니다. 다른 팀원들의 의견이 궁금해요!

github-actions · 2024-09-06T19:26:18Z

Test Results

127 tests +6 119 ✅ +6 4s ⏱️ -1s
40 suites ±0 8 💤 ±0
40 files ±0 0 ❌ ±0

Results for commit a5e40a0. ± Comparison against base commit 22d4168.

This pull request removes 11 and adds 17 tests. Note that renamed tests count towards both.

com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [1] accessToken=com.ody.auth.token.AccessToken@bdb5d84
com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [2] accessToken=com.ody.auth.token.AccessToken@6576cdd
com.ody.auth.service.AuthServiceTest$parseAccessToken ‑ [1] accessToken=
com.ody.auth.service.AuthServiceTest$parseAccessToken ‑ 유효하고 만료되지 않은 액세스 토큰을 파싱할 수 있다.
com.ody.auth.service.AuthServiceTest$renewTokens ‑ 만료되지 않은 액세스 토큰이면 기존 액세스 토큰, 리프레시 토큰을 반환한다.
com.ody.auth.service.AuthServiceTest$renewTokens ‑ 만료된 리프레시 토큰이면 401 에러가 발생한다.
com.ody.auth.service.AuthServiceTest$renewTokens ‑ 만료된 액세스 토큰, 만료되지 않은 액세스 토큰이면 새 액세스 토큰, 리프레시 토큰을 반환한다.
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [1] date=2024-09-06, time=11:03:00.941351166, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [2] date=2024-09-06, time=12:03:00.941382666, expected=true
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [3] date=2024-09-06, time=10:03:00.941370012, expected=false
…

com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [1] accessToken=com.ody.auth.token.AccessToken@5fcad922
com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [2] accessToken=com.ody.auth.token.AccessToken@1b5fda33
com.ody.auth.service.AuthServiceTest$LogoutTest ‑ 성공 : 유효한 액세스 토큰
com.ody.auth.service.AuthServiceTest$LogoutTest ‑ 실패 : 만료된 액세스 토큰으로 로그아웃 시도 시 401을 반환한다
com.ody.auth.service.AuthServiceTest$LogoutTest ‑ 회원이 이미 로그아웃 상태더라도 200을 반환한다
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [1] date=2024-09-10, time=04:41:38.911827102, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [2] date=2024-09-10, time=05:41:38.911860905, expected=true
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [3] date=2024-09-10, time=03:41:38.911845797, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [4] date=2024-09-11, time=04:41:38.911827102, expected=true
com.ody.meeting.repository.MeetingRepositoryTest ‑ 약속 기한이 지나지 않은 모임방을 조회한다.
…

♻️ This comment has been updated with latest results.

github-actions · 2024-09-06T19:26:20Z

📝 Test Coverage Report

Overall Project	77.53% `-0.29%`	❌
Files changed	85.26%	🍏

File	Coverage
JwtTokenProvider.java	97.32%	🍏
AuthService.java	96.38% `-3.62%`	🍏
AccessToken.java	90.74%	🍏
AuthController.java	78.79% `-21.21%`	❌
MemberService.java	66.07%	🍏
Member.java	63.33% `-1.67%`	🍏
RefreshToken.java	56.98%	🍏

hyeon0208

여러 상황 고려해서 로그아웃 기능 작성해주신 것 같네요
고생했어요 콜리 ~ 🥦
approve 드리고 갑니다 ~

각 상황에서 에러 코드가 적합한지 궁금합니다.

저는 유효하지 않는 토큰이든 만료된 토큰이든 해당 토큰으로 요청하면
인증 문제라 생각해 401 에러가 적합하다고 생각해요
관련 내용을 찾아보니 카카오에서는 아래처럼 처리하더라구요
https://devtalk.kakao.com/t/accesstoken/113706

액세스 토큰 하이재킹의 우려

세션 처럼 서버에 저장할 필요가 없이 인증 과정을 수행할 수 있다는 것이 JWT의 장점이라 생각하는데
BlackList를 관리하게 되면 콜리가 생각한 것 처럼 세션이랑 다를 바가 없다고 생각이 들어요
그래서 BlackList를 관리하는 것 보다는 엑세스 토큰의 만료 시간을 조절해보는 건 어떨지 의견 드립니다. 😄

backend/src/main/java/com/ody/member/service/MemberService.java

mzeong

각 상황에서 에러 코드가 적합한지 궁금합니다.

만료되거나 유효하지 않은 액세스 토큰의 경우에 401을 반환해야 한다고 생각합니다.

401을 반환하게 되면 안드 측에서 액세스 갱신 api를 호출하게 될텐데 이때
만료되었다면 갱신 후 다시 로그아웃 요청을 보낼 것이고, 유효하지 않다면 400을 보내는 것이 매끄러운 흐름이라 생각합니다

개인적으로 리프레시 토큰은 오로지 액세스 토큰 갱신 목적으로 쓰이는 것이라 생각해서 로그아웃 시에 리프레시를 보내지 않고 관련 검증 로직도 필요없다고 생각합니다 (관련 코멘트를 남겨두었으니 어떤 부분에서 리프레시 토큰이 필요했는지 알려주세요!!)

다만, 이 상황에서도 액세스 토큰 하이재킹의 우려가 있습니다. 이에 따라 BlackList Entity를 만들어 로그아웃했지만 아직 유효한 accessToken, refreshToken 등을 관리하는 refrence가 많았어요. 안전성이나 보안측면에서요! 그러나, 이 방법은 토큰을 세션처럼 사용하는 느낌이 강하고 새로운 스키마 도입은 합의가 필요한 부분이라 생각했습니다. 다른 팀원들의 의견이 궁금해요!

저는 보통 액세스 토큰 유효기간을 짧게 잡는 것으로 하이재킹에 대응했는데, blacklist를 만드는 방식도 긍정적으로 생각합니다

backend/src/main/java/com/ody/auth/controller/AuthControllerSwagger.java

mzeong · 2024-09-09T06:23:12Z

backend/src/main/java/com/ody/auth/service/AuthService.java

+        AuthorizationHeader authorizationHeader = new AuthorizationHeader(rawAuthorizationHeader);
+        AccessToken accessToken = authorizationHeader.getAccessToken();
+        RefreshToken refreshToken = authorizationHeader.getRefreshToken();


[질문] 왜 로그아웃을 할 때 클라이언트로부터 리프레시 토큰을 받나요??

차람도 비슷한 피드백을 주어서 로직을 수정했습니다
comment에 남겨두도록 할게요!

eun-byeol

이미 구두로 이야기했지만, 다시 적어놓을게요!

각 상황에서 에러 코드가 적합한지 궁금합니다.
만료된 액세스 토큰으로 로그아웃 시도 시 > 400 반환
만료된 리프레시 토큰으로 로그아웃 시도 시 > 400 반환

만료된 상황에 대해서는 401이 적합하다고 생각해요. 만료된 상황에 대해서는 분기처리가 필요한데, 에러코드로 구분되는게 안드에게 편할것 같습니다.

다만, 이 상황에서도 액세스 토큰 하이재킹의 우려가 있습니다. 이에 따라 BlackList Entity를 만들어 로그아웃했지만 아직 유효한 accessToken, refreshToken 등을 관리하는 refrence가 많았어요. 안전성이나 보안측면에서요! 그러나, 이 방법은 토큰을 세션처럼 사용하는 느낌이 강하고 새로운 스키마 도입은 합의가 필요한 부분이라 생각했습니다. 다른 팀원들의 의견이 궁금해요!

카키와 제리의 의견처럼 토큰의 만료시간을 짧게 가져가는 방법에 동의해요. 단, 우리 서비스에서 BlackList까지 관리하는 것은 리소스 낭비라고 생각합니다.

eun-byeol · 2024-09-09T08:22:53Z

backend/src/test/java/com/ody/auth/service/AuthServiceTest.java

    @Nested
-    class parseAccessToken {
+    class LogOutTest {


[필수]
logout으로 통일해주세요~

Suggested change

class LogOutTest {

class LogoutTest {

반영 완료!

eun-byeol · 2024-09-09T08:24:54Z

backend/src/test/java/com/ody/auth/service/AuthServiceTest.java

-        void parseAccessTokenFail(String accessToken) {
+        @DisplayName("실패 : 만료된 액세스 토큰으로 로그아웃 시도 시 400을 반환한다")
+        @Test
+        void logOutFail_When_ExpiredAccessToken() {


[질문]
합의만 되면 상관없는데, 메서드 이름에 _도 컨벤션 허용인가요??🤔

아니요! 합의하지 않았어요. 테스트 네이밍 모두 수정해주었습니다!

합의하지 않고 이전에 사용했던 테스트 네이밍을 습관적으로 도입하려 했던 것 같아요. 컨벤션 의식하며 한번 더 생각하고 네이밍하겠습니다 💪

수정된 테스트 네이밍

logoutSuccess

logoutFailWhenExpiredAccessToken

logoutFailWhenTryAlreadyLogoutMember

LogOut > Logout

coli-geonwoo · 2024-09-09T09:02:01Z

피드백 + 차람과의 협의를 기반으로 개선된 로직입니다.

리프레시 토큰 + 액세스 토큰을 통한 로그아웃 > 액세스 토큰만으로 검증
만료된 액세스 토큰으로 로그아웃 시도 시 > 401 반환
이미 로그아웃된 멤버일 경우 > 400 반환

블랙 리스트의 경우 과도한 리소스 투입 + 세션과 유사해진다는 조조의 의견에 동의하여 도입하지 않았고 추후 액세스 토큰 만료 기간을 짧게 잡는 방식으로 보안성을 높이는 방향이 좋다고 생각합니다.

mzeong · 2024-09-10T02:14:46Z

backend/src/main/java/com/ody/auth/service/AuthService.java

+    private void checkAlreadyLogout(long memberId) {
+        Member member = memberService.findById(memberId);
+        if(member.isLogout()){
+            throw new OdyBadRequestException("이미 회원이 로그아웃 상태입니다.");
        }
    }


[질문] 로그아웃 상태에서 로그아웃을 다시 시도했을 때 400과 200 중 무엇을 줄지 차람과 함께 고려했나요? 회원 삭제 API에서는 이런 상황에서 200을 주기로 결정해서 질문드립니다

반영했습니다! 좋은 포인트의 리뷰 감사합니다 🙇

a5e40a0

coli-geonwoo added 11 commits September 6, 2024 20:18

chore: swagger 문서 작성

a4fcad2

feat: 생성자 로직 변경

3ba15ae

feat: 멤버 refreshToken 삭제 로직 구현

60f9e77

feat: 로그아웃 기능 구현

fd84f24

feat: 리프레시 토큰 초기화 로직 일관성 준수

be6ecc8

feat: 리프레시 토큰 validate 추가

e4d3c42

chore: 메서드 순서 변경

00e4f45

test: 불필요한 테스트 삭제

ddd4820

feat: member 리프레시 토큰 update 기능 구현

6e78ed2

feat: 로그아웃 기능 구현

dac90a6

chore: description 수정

66e3e64

coli-geonwoo added BE FEATURE labels Sep 6, 2024

coli-geonwoo requested review from eun-byeol, mzeong and hyeon0208 September 6, 2024 19:25

coli-geonwoo self-assigned this Sep 6, 2024

coli-geonwoo linked an issue Sep 6, 2024 that may be closed by this pull request

feat: 로그아웃 기능 구현 #478

Closed

1 task

hyeon0208 approved these changes Sep 7, 2024

View reviewed changes

backend/src/main/java/com/ody/member/service/MemberService.java Show resolved Hide resolved

mzeong requested changes Sep 9, 2024

View reviewed changes

eun-byeol approved these changes Sep 9, 2024

View reviewed changes

coli-geonwoo added 3 commits September 9, 2024 17:53

chore: 로그아웃 시 401 description 설명 추가

a65ac99

refactor: 액세스 토큰만 받도록 로직 수정

6569d1b

style: test 네이밍 변경

d1746b1

LogOut > Logout

coli-geonwoo added 2 commits September 9, 2024 18:12

test: 테스트 에러 해결

aa038fd

refactor: 불필요한 메서드 삭제

6e169dd

coli-geonwoo requested a review from mzeong September 10, 2024 01:23

mzeong reviewed Sep 10, 2024

View reviewed changes

coli-geonwoo requested a review from mzeong September 10, 2024 02:27

refactor: 이미 로그아웃 상태더라도 200을 반환하도록 수정

a5e40a0

mzeong approved these changes Sep 10, 2024

View reviewed changes

coli-geonwoo merged commit aa49059 into develop Sep 10, 2024
3 checks passed

coli-geonwoo deleted the feature/478 branch September 10, 2024 04:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 로그아웃 기능 구현 #493

feat: 로그아웃 기능 구현 #493

coli-geonwoo commented Sep 6, 2024 •

edited

Loading

github-actions bot commented Sep 6, 2024 •

edited

Loading

github-actions bot commented Sep 6, 2024 •

edited

Loading

hyeon0208 left a comment

mzeong left a comment

mzeong Sep 9, 2024

coli-geonwoo Sep 9, 2024

eun-byeol left a comment

eun-byeol Sep 9, 2024

coli-geonwoo Sep 9, 2024

eun-byeol Sep 9, 2024

coli-geonwoo Sep 9, 2024

coli-geonwoo commented Sep 9, 2024

mzeong Sep 10, 2024

coli-geonwoo Sep 10, 2024 •

edited

Loading

feat: 로그아웃 기능 구현 #493

feat: 로그아웃 기능 구현 #493

Conversation

coli-geonwoo commented Sep 6, 2024 • edited Loading

🚩 연관 이슈

📝 작업 내용

🏞️ 스크린샷 (선택)

🗣️ 리뷰 요구사항 (선택)

github-actions bot commented Sep 6, 2024 • edited Loading

Test Results

github-actions bot commented Sep 6, 2024 • edited Loading

📝 Test Coverage Report

hyeon0208 left a comment

Choose a reason for hiding this comment

여러 상황 고려해서 로그아웃 기능 작성해주신 것 같네요 고생했어요 콜리 ~ 🥦 approve 드리고 갑니다 ~

mzeong left a comment

Choose a reason for hiding this comment

mzeong Sep 9, 2024

Choose a reason for hiding this comment

coli-geonwoo Sep 9, 2024

Choose a reason for hiding this comment

eun-byeol left a comment

Choose a reason for hiding this comment

eun-byeol Sep 9, 2024

Choose a reason for hiding this comment

coli-geonwoo Sep 9, 2024

Choose a reason for hiding this comment

eun-byeol Sep 9, 2024

Choose a reason for hiding this comment

coli-geonwoo Sep 9, 2024

Choose a reason for hiding this comment

coli-geonwoo commented Sep 9, 2024

mzeong Sep 10, 2024

Choose a reason for hiding this comment

coli-geonwoo Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

coli-geonwoo commented Sep 6, 2024 •

edited

Loading

github-actions bot commented Sep 6, 2024 •

edited

Loading

github-actions bot commented Sep 6, 2024 •

edited

Loading

여러 상황 고려해서 로그아웃 기능 작성해주신 것 같네요
고생했어요 콜리 ~ 🥦
approve 드리고 갑니다 ~

coli-geonwoo Sep 10, 2024 •

edited

Loading