Skip to content
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

[Spring Core] (배포) 신지훈 미션 제출합니다. #115

Open
wants to merge 92 commits into
base: 0094-gengar
Choose a base branch
from

Conversation

developowl
Copy link

@developowl developowl commented Jan 15, 2025

루카님 안녕하세요! 그리디에서의 처음과 끝을 루카님과 함께 하네요!!😆
사실 지난번 밋업 때 감사한 마음을 꼭 전하고 싶었는데 제대로 전달하지 못한 것 같아서 아쉽습니다,,,ㅜ 첫 미션에서 루카님이 제 상황을 고려해서 해주신 조언 덕분에 개발을 처음 접하게 되어 방향성을 못잡을 때 정말 큰 도움이 되었습니다! 나중에 저와 같은 상황에 놓인 뉴비(?)를 만나게 된다면..! 루카님에게 받은 좋은 영향력을 기억하고 저도 다른 누군가(뉴비)에게 좋은 영향력을 끼칠 수 있는 개발자가 꼭 되고 싶습니다!🔥🙇🏻‍♂️
한학기 동안 정말 감사했고 루카님이 보여주신 열정, 잊지 않고 정말 열심히 공부해서 좋은 개발자가 되겠습니다!! 항상 응원합니다!!

이번 미션도 인증, jpa 못지 않게 너무나도 고난이도였습니다.... 앞선 두 미션들도 미션을 완료하고 리뷰어님과 티키타카를 주고 받으며 전반적인 흐름을 겨우 이해했는데, 이번 미션도 흐름 파악, 왜 쓰는가? 등 전체적인 그림을 그리는 것이 좀 많이 어려웠던 것 같습니다. 제가 잘못 이해한 것일수도 있지만 "요구사항이 모호하다" 라는 생각도 참 많이 들었던 것 같습니다. 구현을 꾸역꾸역 하긴 했으나 이번 리뷰를 통해서 미션에서 구현한 것들이 "왜" 필요한지에 대해 스스로 충분한 이해를 할 수 있게 되면 좋겠습니다!

질문1(!해결!)

<7단계 요구사항 힌트 중>
불필요한 DB 접근 최소화

  • JWT 토큰에는 사용자 식별 정보와 권한 정보가 들어갑니다.
  • 만약 이 두 정보만 필요하다면 DB 접근이 필요하지 않습니다.

이 문장에서 고민을 정말 많이 했는데요, 우선 사용자 식별 정보를 memberId라고 생각을 했는데 1단계의 response의 토큰을 분석해보면

{
"sub": "1",
"name": "admin",
"role": "ADMIN"
}

이런 payload를 갖습니다. sub(=subject)가 response들을 구분하는/고유한/식별하는 key값이라고 알고 있습니다. 이것을 memberId와 동일하게 생각해도 될까? 라는 고민이 먼저 들었습니다. 뭔가 모호하다는 생각이 들어서 다음으로 name을 사용자 식별 정보로 간주한다고 했을 때(동명이인은 어떻게 구분하지?), createToken에 email, password가 들어가게 되고 이 두 정보만을 토대로 해당 사용자의 name과 role을 토큰에 추가해야한다고 생각했습니다.(이 판단에 대해 계속 의문이 들었습니다..) 이 과정에서 DB에 접근을 하게 되는 것이 아닌가? 그렇다면 앞 뒤가 안맞는데..? / 여기까지 딜레마에 빠져 있는 시간이 좀 길었던 것 같습니다. 강조한 부분인 "불필요한 DB 접근 최소화"에서 접근 금지가 아닌 '최소화'라면 토큰에 사용자를 식별할 수 있는 정보들을 담으면 어떨까 라는 생각을 했습니다. (물론 보안상 좋지 않다는 것을 알지만 일단 진행했습니다.) 그래서 토큰에 role, name, id 를 담았고(response에 알맞게) 미션을 진행했습니다.
저의 한국어 이슈일수도 있지만, 이에 대해서 어떻게 로직을 처리하는게 바람직한지, 또 어떠한 의도로 저러한 힌트를 제공했는지 도무지 모르겠습니다..
(다른 스터디원들의 코드를 보니 name과 role을 토큰에 포함시켰는데 name이 사용자 식별 정보라고 봐도 되는지 의문입니다...근데 선택지가 name 밖에 없네요...)

->> 토큰 생성 시 memberRepository에 "한 번"만 접근하여 member 객체 정보를 불러오고 여기서 id, role을 토큰에 주입시켜주는 방식으로 리팩토링 했습니다!

질문2(!!해결!!)

8단계를 진행하기 전까지는 토큰 만료로 인한 오류가 생긴 적이 없어서 해당 내용을 고민하지 않았던 것 같습니다. 8단계를 진행하고 나니 계속 토큰 기한 만료 에러가 납니다..(리뷰 반영 기간에 계속해서 리팩토링 해보겠습니다..) properties에서 유효기간을 아무리 많이 부여해도 계속 만료 오류가 발생하는데 이 부분은 어떻게 해결을 하면 좋을 지 궁금합니다. 우선, 제가 생각한 방법은 토큰을 재발급하면 될 것 같습니다.(이 부분은 mvc~core 미션 중에 언급이 없어서(?) 놓친 것 같습니다.) 이에 대해 refreshToken 로직을 추가하면 될 것 같은데 어느 시점에 해당 메서드를 실행하면 좋을지 모르겠습니다. 검색을 좀 해보니 /refresh 앤드포인트를 추가하는 방식도 있지만 현재 제공된 코드들 중에서는 이 앤드포인트와 매핑이 되는 UI도 없는 것 같아서 적절한 방식은 아닌 것 같습니다. 그렇다면 토큰을 검증하는 로직에 추가를 해야할 것 같은데 여기서 살짝 감이 잘 안잡힙니다.

->> 다른 오류 및 로직들을 정리하다보니 해결됐습니다!!


  • @PostMapping("/login") 과 @GetMapping("/login/check") api는 로직을 이상하게 구현한 것 같습니다. 수정을 잘못한 것 같은데, 이 부분은 목요일에 바로 수정하겠습니다.
    ->> 해결했습니다!

** core 미션까지 오면서 앞서 했던 인증, jpa 코드들에 대한 의문(?)이 계속 생겨나는 중이라 계속 리팩토링 하면서 잘 모르겠는 부분은 코멘트로 질문 남기겠습니다!

*** 구현과 더불어 이번 미션에서 구현한 내용들이 "왜" 필요한지, "왜" 구현했는지 잘 이해할 수 있게끔 질문을 좀 해주시면 감사하겠습니다...! 개념이 갑자기 딥해진 느낌이라 와닿는 정도가 앞선 미션들보다 현저히 떨어지는 것 같습니다.

+(1/19) 경환이 형..! DTO를 record 클래스로 변경하는건 어떨까요?

@developowl developowl force-pushed the Core branch 2 times, most recently from 76b5f64 to abdef60 Compare January 21, 2025 09:52
Copy link

@dooboocookie dooboocookie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요, 지훈!
리뷰어 루카입니다.🐳

리뷰하기에 앞서,
리뷰를 너무너무너무너무 늦게 드려 정말정말정말정말 죄송합니다🙇🙇🙇🙇🙇
변명의 여지가 없습니다.

그래도 약간의 변명을 할 수 있는 기회를 주신다면, 이번 미션이 유독 리뷰하기 쉽지 않았는데요.
Spring Configuration이 어려운 영역인 것도 맞지만,
지훈님이 요청하신대로 이 요구사항들이 왜 있는지 파악하는 것이 선행되어야 더 좋은 미션을 할 수 있는 것 같습니다.
그래서, 이번 1차 리뷰에선 디테일보단 큰 틀에서 리뷰하였습니다.

🎯 리뷰 포인트

  1. roomescape와 auth 패키지를 분리하는 목적
  2. argumentresolver나 interceptor의 역할

🔮 질문 답변

DTO를 record 클래스로 변경하는건 어떨까요?

좋습니다! ㅋㅋ
이건 왜 망설이셨는지 궁금한데요?


지훈, 일단 다시한번 리뷰 늦게 드려 죄송합니다.
일단, 이번 7,8단계 목적을 같이 고민해봤으면 좋겠어서 넓은 관점에서 리뷰드렸습니다.
고민 해보면서, 조금 더 학습 포인트를 좁혀나가죠!

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