Skip to content

[Spring MVC(인증)] 신지훈 미션 제출합니다. #107

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

Merged
merged 27 commits into from
Jan 16, 2025

Conversation

developowl
Copy link

@developowl developowl commented Jan 1, 2025

은우님 안녕하세요! 이번 미션은 유난히 어려웠던 것 같습니다..ㅜ 미션 진행하면서 의문점이 들었던 것들에 대해 적어두겠습니다!(사실 매순간이 의문 투성이였던 미션,,,,)

  1. payload로부터 토큰을 생성할 때 "정보를 어디까지 포함시켜야 하는가?" 라는 의문이 들었습니다. 많은 정보를 포함시킬 수록 토큰이 복잡해져서 보안이 강해지고 토큰에 있는 정보만으로 응답을 할수도 있다는 생각도 들었습니다. 한편으로는 시크릿키가 유출이 되었을 때 토큰에 포함되어 있는 정보가 치명적인 정보(비밀번호, role 등)가 아니어도 '정보 유출' 이라는 관점에서는 치명적이지 않을까 라는 생각도 들었습니다. 그래서 일단 가장 기본적인 정보만 담도록 코드를 작성했는데 보통 개발자들은 어떻게 하는지, 또 제 의문들에 대한 답을 듣고 싶습니다!!

  2. LoginMember, Member, MemberDao + MemberRequest, MemberResponse 이다섯가지 클래스들의 경우 필드가 묘하게 비슷한 애들이 많아서 정확한 역할 구분이 안되었던 적이 많았습니다. 전체 흐름을 보았을 때 "딱 여기까지는 이 데이터베이스 관련 클래스를 사용" 이 한눈에 파악이 잘 안되었습니다...

  3. ArgumentResolver는 컨트롤러에 진입하기 전에 쿠키를 통해 정보를 조회하는 역할을 한다고 생각했고, 실제 코드에서는 컨트롤러에서 authService를 사용하지 않게끔 도와주는 역할인 것 같았습니다. 근데 ArgumentResolver 코드 내부에서 authService를 사용하는 부분이 필연적으로 필요한 것 같은데 직접 authService를 사용하는 것과 authService를 클래스 내부에서 사용하는 것(LoginMemberArgumentResolver)에서 의문이 들었습니다. 직접 사용하는 것과 다른 방식을 사용하는것처럼 보이나 그 내부에서 authService를 사용하는 것이 차이가 있는것인지 궁금합니다..

  4. WebConfig와 Interceptors 대해 찾아보던 중
    org.springframework.boot:spring-boot-starter-security
    라는 의존성을 알게 되었습니다. interceptors와 비슷하게 특정 앤드포인트에 접근제한을 둘 수 있고 특정 앤드포인트에는 접근 제한이 없이 누구나 접근할 수 있도록 하는 옵션도 있던 것 같습니다. sercurity 의존성을 사용하는 방식으로 로컬에서 시도해보니 localhost/8080으로 접속을 시도해도 무조건 localhost/8080/login으로 연결이 되고 로그인을 무조건 시도해야하는(?) 페이지가 나왔습니다. 현재 제공된 데이터베이스 정보를 통해 admin으로 접속을 시도해도 로그인이 계속 안되어서 포기한 방법인데 security 의존성이 이번 미션에서 적합한 의존성이었는지, 또 이 의존성은 어떠한 상황에 사용하는 것이 맞는지 궁금합니다..

  5. 로컬에서 이번 미션을 실행시켰을 때 오른쪽 상단에 Login 텍스트를 클릭하게 되면 로그인 페이지로 이동합니다. 이때 IDE의 콘솔창을 보면 400 Bad Request가 뜨는데 도무지 왜 400이 뜨는지 감이 안잡힙니다. 한 번만 봐주시면 감사하겠습니다...

**드디어 학기가 끝나서(종강을 12/27에 해버린,,,) 스프링 공부에 좀 더 집중해보려고 합니다! 이번 인증/인가 미션이 너무 어려워서 인터넷도 많이 찾아보고 gpt한테 많이 물어보기도 했지만,, 미션 완료보다 흐름을 이해하는 것에 초점을 두고 코드가 돌아가도 왜 돌아가는지 이해하려고 하니 조금씩 이해가 되더군요.. 이해하는데 시간을 많이 써도 좋으니 인증 부분에서 제가 좀 더 알았으면 하는 부분이나 확실히 짚고 넘어갔으면 하는 부분이 있으면 꼭 말씀해주세요!!

신년에도 보잘 것 없는 제 코드 리뷰 해주셔서 감사하고 새해 복 많이 받으세요 은우님!!

@be-student
Copy link

be-student commented Jan 2, 2025

질문 부터 먼저 답변드릴게요

  1. 대부분의 경우에는 userId 만을 넣어두는 것 같아요
    userId 를 통해서 role 을 조회할 수 있을테니 필요한 순간에 userId 를 꺼내서 조회하는 방식으로 진행하는 것 같습니다
    그 이유로는 말씀해주신 부분이 정확합니다 정보 유출이 당연히 되면 안되는 것도 정말 당연하고요
    role 이나 비밀번호를 믿지 않는 것은 클라이언트를 신뢰하지 않고 검증하는 것도 당연히 해주셔야 하고요

  2. 이 부분은 정확하게 어떤 질문인지 모르겠네요
    코드를 보고 뭔가 생각나면 답변드릴게요

  3. 이 부분도 약간 애매한 질문이라서
    아마도 직접 AuthService 를 호출하면 되는데 왜 ArgumentResolver 를 통해서 주입하느냐는 것이겠죠?
    코드 테스트 관점과 중복 코드의 관점이 있을텐데요
    매번 반복되는 코드를 줄일 수 있다는 것도 정말 큰 장점일 것이고요
    테스트시에 parameter 를 봤을 때 userId 만 있으면 테스트하기 힘들 수 있는데, User 전체가 있다면 가짜 객체를 만들기 더 쉬워지는 것도 있을 것 같아요

  4. spring-boot-starter-security
    이거는 약간 취향 차이인 것 같아요
    사실 사용하게 되면 복잡도가 정말 많이 올라가는 친구라서 디버깅이 정말 힘들어요...
    대부분의 프로젝트에서는 사용하지 않는 것이 좋을 것 같긴 하고요 (filter 종류만 10개가 넘게 등록되다보니 디버깅이 정말 빡셌던 것으로 알고 있어요 사실상 거의 불가능...)
    만약 보안이 중요한 프로젝트라면 그때부터는 필요하다고 생각해요 (현업에 가서는 필요하지만, 그 전에는 잘 모르겠는 정도의 느낌입니다!)

아마 login 이 안되었던 이유는 다음과 같을 것 같아요
아마 기본적으로 모든 api endpoint 에 login 페이지가 생기고, 그 로그인 페이지에서 admin, admin 으로 입력했어야 했던 것으로 기억해요
그러면 basic auth 를 진행하게 되고, 그 토큰을 매번 검사해주는 그런 기능이 기본 설정이었던 것 같아요 <-- 이런 것들을 모르면 사실 디버깅이 너무 힘들어요
그러다보니 이번 미션이나 취직 전까지는 하지 않는 것을 권장드립니다

  1. 왜 400이냐

쿠키에 토큰이 없다면, ArgumentResolver에서 예외를 발생시키는데, 그 예외가 발생하면 400을 주는 코드가 controller advice 로 있기에 이렇게 되는 것 같아요
스크린샷 2025-01-03 오전 12 48 11

Copy link

@be-student be-student left a comment

Choose a reason for hiding this comment

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

시간이 충분히 있으시다고 하니 먼저 공부해볼만한 부분 위주로 봐드렸습니다!
공부해보면 좋을 키워드들

  1. 쿠키 속성들이 어떤 것들이 있는지
  2. jwt 의 필드들
  3. refreshToken, accessToken

2번 질문을 이제 확실히 이해하게 되었는데요

토스에서를 기준으로 보도록 하겠습니다
저희 회사에서는 LoginMember 클래스가 존재하지 않아요
무조건 Member 만을 들고 다녀요
그래서 비즈니스 로직에서는 사실 Member 만을 알게 되는 경우가 많고요
생성, 응답에 해당하는 dto 는 대부분의 경우에 모든 서비스에서 비슷할 것 같아요
복잡하더라도 어쩔 수 없지만 2번 구현해야 합니다...

수정은 간단한 것들이지만, 공부해보면서 적용해볼 것들은 꽤 있을 것 같아요!
언제든 질문주시면 답변드리겠습니다!

일단 무조건 바로 approve 는 하지만 요청 주시면 계속 확인하겠습니다~!

Copy link

@be-student be-student left a comment

Choose a reason for hiding this comment

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

이번에도 짧게 짧게 왔다 갔다 할 수 있도록 적은 리뷰를 달았습니다!
변경해주시고 요청주시면 또 봐드릴게요

변경하면 좋을 부분들

  1. static 변수로 같은 값이 반복 계산되는 것을 옮기기
  2. LoginMember 제거
  3. 쓰레드에서 말씀해주신 컨트롤러 로직 Service 로 옮기기
  4. Jwt Claim 어떤 속성들이 있는지 보고 일반적으로 사용되는 속성 채워넣어보기
  5. method 네이밍에 맞는 반환 타입을 적용하기
    가 있을 것 같아요!

@be-student
Copy link

be-student commented Jan 13, 2025

전체적으로 고생 많으셨어요!
고민하신 부분이 많이 느껴지는 코드였습니다 🙇
더 빠르고 더 잘 리뷰해드렸으면 정말 좋았겠다는 생각이 많이 들더라고요

그럼에도 또 충분히 고민할 수 있을법한 키워드를 던져드리면서 이번 미션 마무리를 해보도록 하겠습니다
면접을 보면 매번 나오는, 하지만 중요한 질문인데요

쿠키에 로그인 토큰을 담아도 될까? 위험하지 않을까?

로그인 토큰은 cookie, header, body 에 전부 다 줄 수 있습니다
이때 header, body 의 역할은 거의 비슷하지만 cookie 는 서로 다른데요

위쪽에도 간단하게 남겨드린 답변을 보시면 다양한 공격 방식을 통해서 토큰 탈취를 시도하는 것을 알 수 있습니다
이 과정에서 cookie 를 함부로 다루는 것이 정말 위험하다는 것을 많은 사람이 알게 되었죠

이때문에 생긴 보안 정책이 정말 많은데요
대표적인 보안 정책으로 cors 인데요
cors 와 withCredential true 속성을 한꺼번에 보시면 조금 더 이해가 잘 될 수 있을 것 같아요
https://inpa.tistory.com/entry/AXIOS-%F0%9F%93%9A-CORS-%EC%BF%A0%ED%82%A4-%EC%A0%84%EC%86%A1withCredentials-%EC%98%B5%EC%85%98
추가로 이제 다른 사이트에 쿠키를 함부로 설정하지 못하도록 sameSite 속성도 있죠
https://seob.dev/posts/%EB%B8%8C%EB%9D%BC%EC%9A%B0%EC%A0%80-%EC%BF%A0%ED%82%A4%EC%99%80-SameSite-%EC%86%8D%EC%84%B1/

이런 정책들로 막지만, 완벽하게 방어하는 것은 불가능한 상황입니다


그렇다면 이제 다른 생각이 들기 시작합니다
정말 쿠키로 인증을 하는 것이 최선일까?
header, body 에 로그인 토큰을 담아주는 것은 안될까? 하는 부분이죠
그렇게 되면 쿠키에 통하는 공격을 막을 수 있게 되는데요
그렇게 되면 이제 훨씬 더 안전할 수 있겠죠
반대로 또 header, body 에 해당하는 데이터를 가져오는 공격도 있죠
그렇다보니 어느 한쪽이 무조건 안전하다, 위험하다라는 것을 두는 것 보다는 어느 쪽이든 장단이 있다고 바라보시면서 작성해보시면 좋을 것 같습니다

이런 보안적인 측면도 고려하시게 되면 훨씬 더 안전한 방식으로 코딩을 할 수 있을 것이라고 생각하며, 앞으로도 좋은 코드를 짤 수 있도록 응원합니다 🙇
고생하셨어요

@boorownie boorownie merged commit 67fec8e into next-step:0094-gengar Jan 16, 2025
@developowl developowl deleted the MVC branch January 16, 2025 02:24
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.

3 participants