이미 탈퇴한 사용자의 GitHub 토큰 백필 파기 - #222
Merged
Merged
Conversation
#198 은 탈퇴 시 GitHub 토큰을 폐기하도록 했지만 **그 시점에 이미 탈퇴해 있던** 사용자들은 다루지 않았다. V28 은 CHECK 제약을 완화하기만 했고 백필이 없다. 그 사람들은 앞으로 User.withdraw() 가 불릴 일도 없어서, 백필하지 않으면 repo 스코프 토큰이 영원히 남는다 — #198 이 없애려던 상태 그 자체다. V31 로 is_deleted=TRUE 인 행의 토큰을 NULL 로 정리한다. 함께: InternalGithubTokenService 가 findById 를 쓰고 있어 삭제 여부를 보지 않았다. #198 이후로는 토큰이 비어 hasGithubLink 에서 걸리지만, 그건 "값이 비어 있어서" 막히는 데이터 상태 의존 방어다. findByIdAndDeletedFalse 로 바꿔 상태와 무관하게 탈퇴 계정을 먼저 막는다. 이 둘은 함께 있을 때 의미가 있다 — 백필 전 데이터의 위임 경로가 바로 이 조회였다. 이 서비스에 테스트가 없어 3건 추가(정상 위임 / 탈퇴 계정 거부 / Google 전용 계정).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
문제 — #198 의 빠진 절반
#198 로 탈퇴 시 GitHub access token 을 폐기하게 했다. 그런데 그 시점에 이미 탈퇴해 있던 사용자들은 다루지 않았다.
V28은 토큰을 NULL 로 만들 수 있도록 CHECK 제약을 완화하기만 했고 백필이 없다. 이미 탈퇴한 계정은 앞으로User.withdraw()가 불릴 일도 없으니, 백필하지 않으면 그 토큰은 영원히 남는다.repo스코프 토큰이다 — 비공개 레포까지 읽을 수 있는 살아있는 자격증명. #198 이 없애려던 상태가 정확히 이 사람들의 상태다. 내가 #198 에서 빠뜨린 부분이다.두 번째 갭 — 위임 경로
InternalGithubTokenService.fetchPlainAccessToken이findById를 써서 삭제 여부를 보지 않았다.#198 이후로는 탈퇴 시 토큰이 비니
hasGithubLink()에서 걸린다. 하지만 그건 "값이 비어 있어서" 막히는 데이터 상태 의존 방어다. 백필 전 데이터에는 값이 있으므로 그대로 뚫린다 — 즉 위 백필 대상의 유출 경로가 바로 이 조회였다.두 수정이 함께 있어야 의미가 있어서 한 PR 로 묶었다.
수정
V31 — 백필:
서비스 —
findById→findByIdAndDeletedFalse. 삭제 여부로 먼저 막아 데이터 상태와 무관하게 닫는다.테스트
이 서비스에 테스트가 하나도 없었다. 3건 추가:
USER_NOT_FOUND)AUTH_GITHUB_NOT_LINKED(NPE 가 500 으로 새지 않게)함께 확인했고 문제없던 것
X-Internal-API-Key뒤에 있다