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

[REFACTOR] cookies 활용 jwt 방식으로 마이그레이션 #176

Merged
merged 23 commits into from
Dec 12, 2024

Conversation

haejinyun
Copy link
Collaborator

@haejinyun haejinyun commented Dec 10, 2024

What is this PR? 🔍

Changes 📝

  • cookies 활용 jwt 방식으로 마이그레이션
  • axios를 활용하던 fetching을 fetch를 활용하였습니다.
  • next-cookies를 활용하여 클라이언트와 서버에서 쿠키를 사용할 수 있게 구현하였습니다.
  • 이런 fetch을 관리하는 함수 createFetchApiClient를 생성하여 구현하였습니다.
  • 인증이 필요한 api호출 시에는 const tokens = getAuthTokens();을 사용하여 tokens을 fetch문으로 넘겨주어 인증합니다.

ScreenShot 📷

Precaution

api call 부분에서는 아쉬움이 많이 남는데, 기능이 잘 돌아가는지 동작이 잘 되는지를 기준으로 봐주시면 감사하겠습니다.

✔️ Please check if the PR fulfills these requirements

  • It's submitted to the correct branch, not the develop branch unconditionally?
  • If on a hotfix branch, ensure it targets main?
  • There are no warning message when you run npm run lint

@haejinyun haejinyun self-assigned this Dec 10, 2024
Copy link
Collaborator

@kimeodml kimeodml left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.
소켓쪽이 세션으로 접근하는데 수정이 필요해 보여요!

...options,
};

const refreshAccessToken = async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

요 함수 로직 밖으로 빼도 괜찮을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

요 부분을 저도 빼려고 했는데

const response = await createFetchApiClient<CustomResponse<PostLoginResponseData>>(
      '/v2/auth/token-reissue',
      {
        method: 'POST',
        body: JSON.stringify({ refresh_token: refreshToken }),
      },
    );

createFetchApiClient요 부분에서 함수를 사용하고 있어서 애매하더라구요...
일단 동작하면 냅두고 다른 기능 해결하고 다시 봐도 괜찮을까요...

Copy link
Collaborator

Choose a reason for hiding this comment

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

넵넵 아님 lint-disabled 방식을 고려해도 좋을 것 같아요!

const refreshToken = cookies().get('refreshToken')?.value;

if (!refreshToken) {
deleteTokenCookies();
Copy link
Collaborator

Choose a reason for hiding this comment

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

redirect로 메인이나 로그인페이로 이동하면 좋을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현재 서버 컴포넌트로 되어있어서 router나 window를 쓰지 못해서 redirect()를 사용했습니다.

const newRefreshToken = refreshAccessTokenResponse.result_data.refresh_token;
// 여기서 토큰을 새로 refreshAccessTokenResponse

if (newAccessToken && newRefreshToken) {
Copy link
Collaborator

@kimeodml kimeodml Dec 10, 2024

Choose a reason for hiding this comment

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

accessToken만 확인해도 괜찮지 않을까 싶어요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

재발급 로직을 타면 access랑 refresh모두 새로 발급받게 되어서 둘다 값이 있는지 잘 저장되었는지 확인하기 위해서 두개 다 넣었습니다!


return (await response.json()) as T;
} catch (error) {
console.error('Fetch error:', error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

로그인 페이지로 이동하게 하는 거 어떨까요?

Copy link
Collaborator

@kimeodml kimeodml left a comment

Choose a reason for hiding this comment

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

이 부분만 수정하면 될 것 같아요!

const defaultOptions: RequestInit = {
headers: {
'Content-Type': 'application/json',
...(accessToken && { Authorization: `${accessToken}` }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

newAccessToken으로 업데이트 필요합니다!
좋아요 쪽이 지금 이상한 것 같아요. newAccessToken으로 변경하면 다른 건 다 잘 요청되는데 '좋아요'만 에러로 넘어가네요. 저도 같이 확인해 볼게요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

위쪽에서 새롭게 set을 해두어서 여기서는 accessToken을 가져와서 호출하면 되는 로직으로 생각했습니다..!

router.refresh();
router.push('/');
setIsLoggedIn(false);
showToast('success', '로그아웃 되었습니다.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

queryClient.clear();이 없어서 로그아웃 해도 사용자 정보가 보이고 있어요!

@kimeodml
Copy link
Collaborator

좋아요/삭제하기/등록하기에서 에러 발생하는데 merge 하기 전에 전체적으로 다시 한 번 잘 작동되는지 확인이 필요해 보여요!

@@ -0,0 +1,6 @@
import { deleteCookie } from 'cookies-next';

export function handleLogout() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수명 deleteCookie 이런걸로 바꾸면 어떨까요??

Comment on lines 82 to 83
deleteCookie('accessToken');
deleteCookie('refreshToken');
Copy link
Collaborator

Choose a reason for hiding this comment

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

handleLogout으로 호출하는 거 어떨까요?

@haejinyun haejinyun merged commit 43ac6da into develop Dec 12, 2024
2 checks passed
@haejinyun haejinyun deleted the refactor/#174 branch December 12, 2024 11:28
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.

[REFACTOR] 인증 인가 수정 쿠키 + fetch로 수정
2 participants