-
Notifications
You must be signed in to change notification settings - Fork 3
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
[ Common ] 엑세스 토큰 관리 #338
[ Common ] 엑세스 토큰 관리 #338
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
pr을 꼼꼼하게 적어줘서 도리의 코드 의도를 잘 이해하고 넘어갈 수 있었엉요 !!!
깔끔하게 잘 고쳐진 것 같아용 !! LGTM !!!
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.
굿!!! 요청 사항과 논의했던 사항 모두 의도대로 잘 반영해준 것 같네요!!! 체고체고 👍🏻👍🏻👍🏻
export const api = () => { | ||
const api = axios.create({ | ||
baseURL: import.meta.env.VITE_APP_BASE_URL, | ||
}); | ||
|
||
const token = sessionStorage.getItem('token'); | ||
api.defaults.headers.common['Authorization'] = `Bearer ${token}`; | ||
|
||
return api; | ||
}; |
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.
P5: 이런 방식이라면 api()로 호출할 때마다 새로운 axios 인스턴스를 반환받을 텐데 지금 로그인 후에 인스턴스 객체 내용이 거의 변함이 없을 것 같아서 계속 인스턴스를 생성하는 것이 비효율적이지 않을까 하는 생각을 해봤습니다.
export const api = () => { | ||
const api = axios.create({ | ||
baseURL: import.meta.env.VITE_APP_BASE_URL, | ||
}); | ||
|
||
const token = sessionStorage.getItem('token'); | ||
api.defaults.headers.common['Authorization'] = `Bearer ${token}`; | ||
|
||
return api; | ||
}; |
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.
P5: 대안을 한번 생각해봤는데 이런 방식은 어떨까요?! 인스턴스를 한 번 생성하고 이를 재사용하도록 하는 방법입니다!!
export const api = () => { | |
const api = axios.create({ | |
baseURL: import.meta.env.VITE_APP_BASE_URL, | |
}); | |
const token = sessionStorage.getItem('token'); | |
api.defaults.headers.common['Authorization'] = `Bearer ${token}`; | |
return api; | |
}; | |
let apiInstance; | |
export const api = () => { | |
if (!apiInstance) { | |
apiInstance = axios.create({ | |
baseURL: import.meta.env.VITE_APP_BASE_URL, | |
}); | |
const token = sessionStorage.getItem('token'); | |
apiInstance.defaults.headers.common['Authorization'] = `Bearer ${token}`; | |
} | |
return apiInstance; | |
}; |
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.
@eunbeann 맞아요!!! 근데 다시 생각해보니 별도의 로그아웃 기능이 있으니 서비스 도중 토큰 값이 바뀔 가능성이 있네요! 원래 도윤이 코드대로 하는 게 맞을 것 같습니다아 :)
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.
사실 이거 저도 고민을 해보았었습니다.. 기존 코드를 뜯어보다 보니까 axios를 호출할 때마다 axios.create로 새로운 인스턴스가 계속해서 만들어지는게 아닌가? 불필요한 반복이지 않나?라는 생각을 했었는데 그걸 정우도 똑같이 해주었네요 ㅎㅎ
+저기 세션스토리지에 저장되는 토큰 값은 로그인을 할 때마다 변합니다! 로그인이 되어있는 한 토큰 값이 변하지는 않아요! 세션스토리지에 토큰을 저장해서 창을 종료하거나 로그아웃을 하기 전까지는 동일한 토큰값이 저장되어있을 겁니다..! 그래서 정우가 제안해준 방법도 저는 좋다고 생각합니다! 비니가 말한 것처럼 토큰이 업데이트 되었는지 확인한 다음에 인스턴스를 다시 생성하는 방식으로 하면 안정적이지 않을까 ..! 하는 생각이 들어요!
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.
이거 일단 해당 브랜치 머지하구 새 브랜치 파서 다시 진행해볼게요 !
yarn.lock
Outdated
hamt_plus@1.0.2: | ||
version "1.0.2" | ||
resolved "https://registry.yarnpkg.com/hamt_plus/-/hamt_plus-1.0.2.tgz#e21c252968c7e33b20f6a1b094cd85787a265601" | ||
integrity sha512-t2JXKaehnMb9paaYA7J0BX8QQAY8lwfQ9Gjf4pg/mk4krt+cmwmU652HOoWonf+7+EQV97ARPMhhVgU1ra2GhA== | ||
|
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.
ASK: 해싱되어서 잘 모르겠는데 어떤 의존성이 추가된 걸까요...?
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.
hamt_plus
이거인 것 같은데 찾아보니까 리코일 설치 하면서 설치된 친구인 듯해요 예리한 정우
@doyn511 리코일 삭제했으니까 이것도 삭제해도 좋을 듯!
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.
삭제 완료 !!
🔥 Related Issues
✅ 작업 내용
📸 스크린샷 / GIF / Link
X
📌 이슈 사항
기존에 로컬스토리지에서 관리하던 토큰을 axios header + sessionStorage 에서 관리하는 방법으로 변경하였습니다.
기존 코드
변경한 코드
시도했지만 실패한 방법
axios.defaults.headers.common['Authorization'] = `Bearer ${token}`;
의 방법 대신 바로 axios.create 할 때 header에 토큰 값을 넣어주는 방법으로도 시도를 해보았는데, 이렇게 하니 첫 로그인에서는 토큰 값이 제대로 들어가지만, 로그아웃 이후 다시 로그인을 시도하려 할 때 header에 token값이 null로 들어가 제대로 동작하지 않았습니다....따라서 위의 변경된 코드로 구현했습니다. 저렇게 구현하니 기존 api의 type은 AxiosInstance였기에 api를 호출할 때 그냥 api.get.~ 로 호출했었는데 type이 () => AxiosInstance로 수정되어 api를 호출할 때 api().get~ 으로 호출하도록 모든 api의 코드를 수정했습니다.. (그래서 file changes가 굉장히 많아요..)
로그인 여부를 전역상태관리 라이브러리인 Recoil을 사용하려 했는데 생각해보니 sessionStorage에 token값이 저장되기 때문에 해당 값 존재 여부로 분기 처리를 하면 된다고 판단해
isLogin = sessionStorage.getItem("token")
으로 로그인 여부를 판별하였고, nickname 또한 localStorage에서 sessionStorage로 변경해서 저장해두었습니다.이렇게 수정하니 로그아웃 할 때
sessionStorage.clear()
로 한번에 스토리지를 비워주면 되더라고요 !+presignedUrl 중 put을 하는 경우, 레큐 서버의 BASE_URL이 필요한 게 아니라 get해온 url로 바로 put 해주면 되기 때문에 api().put 대신 axios.put()으로 코드를 수정하였습니다.
✍ 궁금한 것
X