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

Feature/#186 userid name 세션스토리지에서 빼기 #187

Conversation

pipisebastian
Copy link
Member

@pipisebastian pipisebastian commented Nov 25, 2024

📝 변경 사항

  • id, name, accessToken 모두 zustand로 관리합니다. 이는 새로고침할 경우 모두 새롭게 불러옵니다.

🔍 변경 사항 설명

  • 기존 동작은 세션스토리지에 id, name이 있다면 refresh 요청을 보내지 않았는데,
  • 세션 스토리지를 아예 없애면서 매번 새로고침할때마다 refresh 요청을 보내게 됩니다.
  • 그렇게 되면 처음 페이지를 들어올때도 refresh 요청 + 에러를 받게 되면서, 재로그인 모달창이 뜨게 됩니다.

image

  • 맨 처음에도 refresh 요청을 보내고 에러를 받는다는게 이상하다 생각하지만, 조건처리할 값이 없습니다..
    • 차라리 쿠키에 있는 refresh token의 여부를 판단해서 refresh 요청을 보낼지? -> httpOnly 쿠키를 확인할 수 없음
    • name만이라도 세션 스토리지에 넣어야할지?
  • 위에 2가지 방법이 아니면, refresh 요청을 특정상황에서 막는게 불가능하다 판단했습니다.
    • 그래서 지금은 무조건 refresh 요청을 보내고, 에러를 받는게 network 탭에서 보입니다...
    • 일단 새로고침 했을때만이라도 재로그인 모달창을 없애는게 좋겠다 생각했는데, 이를 처리하는데 시간이 많이 걸릴 것 같아
    • 401 에러를 띄워줄 때 모든 상황에서 재로그인 모달창을 없앴습니다.

🙏 질문 사항

  • 저번 멘토링에서 현재 토큰을 관리하는 방식에 대해 질문드렸었는데
  • accessToken은 탈취되더라도 서버의 secret key로 서명되어 있어 위조가 불가능
  • 새로고침시마다 서버 요청하는 게 꼭 필요할까?
  • localStorage나 sessionStorage에 저장해도 문제가 없다고 생각한다!
  • 이렇게 되면 refresh 요청도 특정 상황에서 access token이 만료될 때만 요청하면 돼서, 지금 PR에 적은 문제가 사라집니다.
  • 물론 구조를 다시 한번 뜯어고쳐야하겠지만요,,, 지금 PR은 맨 처음 홈페이지를 접속했을때 refresh 요청 에러가 뜨는게 너무 신경쓰입니다..
  • 다른 분들 의견이 궁금합니다!! 만약 모든걸 sessionStorage에 넣게 된다면, 지금 PR은 날려버려야합니다.

📷 스크린샷 (선택)

  • UI 변경이 있는 경우 스크린샷이나 GIF를 첨부합니다.

✅ 작성자 체크리스트

  • Self-review: 코드가 스스로 검토됨
  • Unit tests 추가 또는 수정
  • 로컬에서 모든 기능이 정상 작동함
  • 린터 및 포맷터로 코드 정리됨
  • 의존성 업데이트 확인
  • 문서 업데이트 또는 주석 추가 (필요 시)

@pipisebastian pipisebastian deleted the Feature/#186_userid_name_세션스토리지에서_빼기 branch November 26, 2024 04:14
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.

1 participant