-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: 하나의 디바이스를 사용하는 회원 정보가 여러 개일 때, 로그아웃이 정상적으로 되지 않는 버그 해결 #544
base: dev
Are you sure you want to change the base?
The head ref may contain hidden characters: "fix/#543_\uB85C\uADF8\uC544\uC6C3_\uBC84\uADF8\uD574\uACB0"
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빠르게 고쳐주셨군여👍
|
||
@Test | ||
@DisplayName("동일한 디바이스 정보가 여러 회원에 걸쳐 존재할 때에도 로그아웃이 정상 동작해야 한다.") | ||
void deviceLogoutBug() { | ||
// Given : 회원이 새로 가입하여 로그인한 후 로그아웃한다. | ||
authService.joinMember(new JoinRequest(("[email protected]"), "password123!", "nickname", "sameDevice")); | ||
authService.login(new LoginRequest("[email protected]", "password123!", "sameDevice")); | ||
authService.logout("sameDevice"); | ||
|
||
// When : 동일한 디바이스를 가지고 새로운 회원이 가입하여 로그인한 후 로그아웃한다. | ||
authService.joinMember(new JoinRequest(("[email protected]"), "password123!", "nickname2", "sameDevice")); | ||
authService.login(new LoginRequest("[email protected]", "password123!", "sameDevice")); | ||
assertThatCode(() -> authService.logout("sameDevice")) | ||
.doesNotThrowAnyException(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요거 MemberServiceTest라기 보단 AuthServiceTest에 가까운것같아욤
그리고 DisplayName은 갑자기 왜 나타났죠! 한글 메서드명이 암묵적인 관행인줄 알았는데요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 쎗~~ 테스트코드 친 지 너무 오래돼서 까먹어버렸군요
@@ -188,7 +188,7 @@ public void deactivateOtherDevices(final Member member, final String deviceToken | |||
|
|||
@Transactional | |||
public void deactivateDevice(final String deviceToken) { | |||
final Optional<Device> device = deviceRepository.findByDeviceToken(deviceToken); | |||
final Optional<Device> device = deviceRepository.findByDeviceTokenAndIsActiveIsTrue(deviceToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문제상황이 해결된 것 같군요 굿
그런데 isActive = true인 디바이스 토큰이 두개 이상 있는 경우는 없겠죠...??(정상적인 상황에선 없을 것 같긴 하다만요)
글구 오늘에서야 깨달았는데 갤럭시 보안폴더 이용하면 하나의 핸드폰으로 두개의 이돈이면을 동시에 이용가능 ~ㅋㅋ 디바이스 토큰도 다르더라구여 싱기방기
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 그리고 이건 예외는 발생하지 않는 사항이긴 한데,
보안폴더 내의 이돈이면A와 보안폴더 밖의 이돈이면B에서 같은 계정을 이용할 경우,(좀더 확장하면 A디바이스와 B디바이스)
A에서 계정 로그인 -> B에서 동일 계정 로그인하면
A의 디바이스 토큰이 비활성화되긴 하는데 A에서는 로그아웃되지 않고 디바이스토큰만 비활성화된 상태로 서비스는 이용 가능하더라구여. 즉 최근에 로그인한 B 디바이스로는 알람이 가지만 A에겐 알림이 가지 않는다는 소리.
그래서 생각해본 방안.
1번: B기기로 로그인할 때 A기기를 로그아웃 시킨다
이참에 깔끔하게 B기기로 로그인할 때 A 기기에 대해 강제로 로그아웃시키는 건 어떤가요 호이씨?? 세션저장소를 도입할 생각이 있으시다면 어케 잘 해볼 수 있지 않을까요?? 😁😁😁😁😁😁 @This2sho
머 저희가 보내는 알람이 중대한 알림은 아니라 알람만 안오는상태로 이용해도 괜찮다고 생각하긴 합니다만.... 애써 만든 알림이 무용지물 되는 것은 슬프자나여~
2번: 하나의 계정에서 여러 기기에 대해 디바이스 토큰을 활성화할 수 있게 한다
지금은 로그인시, 하나의 회원에 대하여 최근 로그인한 기기말곤 다른 기기들의 디바이스 토큰을 비활성화해주고 있는데요! (아마 예상치 못하게 디바이스 토큰이 비활성화되지 못하고 로그아웃당한 상황이나 세션만료된 상황을 염두해둔게 아닐까싶습니당)
(1) 자의적으로 기존 기기에 대한 로그인을 유지한채 다른 기기로 로그인해서 두 기기에 대해 디바이스 토큰을 활성화유지하는 것과
(2) 로그인하고 있던 기기가 세션만료당한 후 다른 기기로 다시 로그인해서 기존 세션만료당한 기기에 대한 디바이스 토큰을 비활성화 시키는 것
이거 두개를 깔끔히 구분해서 처리할 마땅한 방안이 떠오르지 않기도 하구여..
사용자가 명시적으로 "나 로그아웃 할래!"라고 한 디바이스만 잘 비활성화시켜 주면, 로그인 시 동일계정의 다른 기기들(예를들어 세션만료당한 기기)을 애써 비활성화할 필요가 없다는 생각이 들기도 하네용(세션이 만료되었어도 알림으로 자꾸 앱 들어와라 유인할 수 있긴 하니ㅎㅎㅎ)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 보안폴더를 사용하면 그런 종류의 실험도 가능하군요 ...??? 전 어릴때 네이버웹툰 보는 거 안 들키려고 보안폴더 써본 기억밖에 없는데 ㅋㅋㅋㅋㅋㅋ
생각해보니 JWT 사용을 극구 반대하면서 들었던 근거가 '세션을 사용해서 원격 로그아웃 등을 구현하게 될 가능성이 높다' 였는데, 막상 이 기능을 실제로 구현해보진 않은 것 같아요.
한 디바이스에서만 알림을 받도록 하는 것보다 아예 로그아웃을 시켜버리는 게 더 깔끔하겠죠? 이슈를 한 번 생성해 보겠습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 동일한 디바이스 토큰은 반드시 하나만 활성화되어 있도록 애플리케이션 단에서 설계했기 때문에, 그 걱정은 하지 않아도 될 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
케로&이리내 : 로그아웃을 할 때 토큰을 비활성화하는 것보다 그냥 삭제하는 편이 좋다고 생각한다. 왜냐하면 동일한 디바이스로 로그인하더라도 캐시를 삭제하거나 앱을 재설치하는 경우 다른 디바이스 토큰을 할당받기 때문이다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 논의에서 2번을 채택하기로 한다.
- 사용자가 명시적으로 로그아웃을 하지 않는 이상 여러 디바이스에도 알림을 발송할 수 있다.
- 디바이스에서 활성/비활성 여부를 없앤다.
- 명시적으로 로그아웃하는 경우 그냥 디바이스 토큰 정보를 물리적 삭제한다.
🔥 연관 이슈
📝 작업 요약
잘못된 디바이스 호출 방법으로 인해, 둘 이상의 회원이 동일한 디바이스로 로그인한 이력이 있는 경우 로그아웃이 일어나지 않는 버그가 발생했습니다.
🔎 작업 상세 설명
버그 발생 상황
해결 방법
DeviceToken만을 where 조건으로 사용하는 findByDeviceToken 대신 '활성화된' 디바이스 정보를 가져오는 findByDeviceTokenAndIsActiveIsTrue 메소드를 사용하도록 변경함으로써 해결하였습니다.
🌟 리뷰 요구 사항