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

[FEAT] 세션 방식의 인증인가로 수정 #165

Merged
merged 8 commits into from
Dec 5, 2024
Merged

Conversation

haejinyun
Copy link
Collaborator

What is this PR? 🔍

Changes 📝

세션 방식의 인증 인가를 구현하였습니다.
로그인을 한 이후 받아오는 accessToken과 refreshToken을 적절하게 관리하기 위하여
'use server'로 서버 컴포넌트에서 fetch를 해온 뒤 cookie에 accessToken을 넣고 토큰들을 반환합니다. 이후 상위의 클라이언트 컴포넌트에서 서버 컴포넌트에서 전달받은 토큰을 브라우저 스토리지에 넣습니다.

또한 api를 호출할 때 사용하는 apiClient를 생성하는 createApiClient에서도 서버냐 아니냐에 따라 authorization header에 값을 넣는 것을 달리하였습니다.

ScreenShot 📷

Precaution

바뀐 부분이 많아 조금씩 잘라서 진행하려 합니다.
현재는

  • 로그인 요청을 하면 token을 쿠키와 스토리지에 저장하는 것
  • api를 호출할때 token을 header에 넣는 것
  • 미들웨어 수정
  • 로그인 성공 후 이전 페이지로 돌아가기

기능이 수정 되었습니다.

남은 이후 pr로 로그아웃, refresh로 access 재발급 or 로그아웃 로직을 추가할 예정입니다.
또한 webSocket을 사용하는 곳에서 401에러가 나고 있는데 이는 header를 추가해도 변경되지 않아 담당 BE분과 에러를 조금 더 살펴보겠습니다.

✔️ 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

Copy link

github-actions bot commented Dec 4, 2024

Bundle sizes [F2-omocha-FE]

Compared against 03b4698

Dynamic import Size (gzipped) Diff
apis/queryFunctions/apiClient.ts -> next/headers 3.42 KB added

@haejinyun haejinyun self-assigned this Dec 4, 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.

고생하셨습니다~

Comment on lines +15 to +18
client.interceptors.request.use(async config => {
if (isServer) {
// 서버에서 실행되는 경우에는 쿠키에서 access_token 가져옵니다.
const { cookies } = await import('next/headers');
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분 깔끔하게 eslint diabled 추가해서 require로 선언하는 거 어떨까요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

음...다른 곳에서도 모두 Import 문법을 쓰고 있어서 통일 하는게 더 낫다고 느껴지기는 하네요ㅠㅠ
import 하면 상단에 정의되어 비동기문으로 처리해야하는 것이 조금 불편할 수 있지만 저렇게 처리 하는 경우도 있다고 들어서요..!
혹시 성능에 큰 문제가 없다면 import문으로 통일하시는건 어떤가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

넵 확인했습니다~

Comment on lines 40 to 55
try {
await clientLogin({
email: data.emailRequired,
password: newPassword,
});
setIsLoggedIn(true);
if (prevUrl?.startsWith('/join') || prevUrl?.startsWith('/login')) {
router.push('/');
} else {
router.push(prevUrl || '/');
}

router.refresh();
} catch (error) {
console.error('Login failed:', error);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

import { useMutation } from '@tanstack/react-query';
import { AxiosError } from 'axios';
import { useRouter, useSearchParams } from 'next/navigation';

import { useAuth } from '@/provider/authProvider';
import { useToast } from '@/provider/toastProvider';

import { LoginParams } from '../types/Auth';
import { Response } from '../types/common';

import login from './login';

export function useClientLogin() {
  const { showToast } = useToast();
  const router = useRouter();
  const searchParams = useSearchParams();
  const prevUrl = searchParams.get('prevUrl');
  const { setIsLoggedIn } = useAuth();

  const { mutate } = useMutation({
    mutationFn: (loginParams: LoginParams) => login(loginParams),
    onSuccess: data => {
      sessionStorage.setItem('accessToken', data.accessToken);
      localStorage.setItem('refreshToken', data.refreshToken);
      setIsLoggedIn(true);
      if (prevUrl?.startsWith('/join') || prevUrl?.startsWith('/login')) {
        router.push('/');
      } else {
        router.push(prevUrl || '/');
      }
    },
    onError: (e: AxiosError<Response<string>>) => {
      if (e.response) {
        showToast('error', `${e.response.data.result_msg}`);
      } else {
        // 네트워크 에러나 기타 처리되지 않은 에러 처리
        showToast('error', '알 수 없는 오류가 발생했습니다. 새로고침을 진행해 주세요.');
      }
    },
  });

  return { mutate };

  // 세션 스토리지에 저장
  // if (typeof window !== 'undefined') {
  //   sessionStorage.setItem('accessToken', accessToken);
  //   localStorage.setItem('refreshToken', refreshToken);
  // }
}

이전 usePostLogin이랑 동일해요. mutationFn이 단지 fetch로 동작하는 것 뿐입니다!

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 { mutate } = useMutation({
    mutationFn: (loginParams: LoginParams) => setTokenCookies(loginParams),
    onSuccess: data => {
      sessionStorage.setItem('accessToken', data.accessToken);
      localStorage.setItem('refreshToken', data.refreshToken);
      setIsLoggedIn(true);
      showToast('success', '로그인 되었습니다.');
      if (prevUrl?.startsWith('/join') || prevUrl?.startsWith('/login')) {
        router.push('/');
      } else {
        router.push(prevUrl || '/');
      }
    },
    onError: (e: AxiosError<Response<string>>) => {
      if (e.response) {
        showToast('error', `${e.response.data.result_msg}`);
      } else {
        // 네트워크 에러나 기타 처리되지 않은 에러 처리
        showToast('error', '알 수 없는 오류가 발생했습니다. 새로고침을 진행해 주세요.');
      }
    },
  });

  return { mutate };

이런식으로 변경해보았어요~
해당 파일은 queryHook => Auth 쪽으로 옮겨두었어요

@@ -23,6 +23,10 @@ function useChatSocket({
onMessage,
checkBottom,
}: UseChatSocketParams) {
const accessToken = sessionStorage.getItem('accessToken');

console.log('accessToken: useChatSocket.ts', accessToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 지워도 괜찮을 것 가탕요!

@haejinyun
Copy link
Collaborator Author

고생하셨습니다~

166번 브랜치의 작업과 많이 섞이게 되어 166과 160의 분기를 맞춘 후 리뷰 반영하였습니다!
160 브랜치 수정 완료 후 머지하면 그때 dev 브랜치와 분기 맞춰서 refresh로직 개발하던 166 개발 이어가겠습니다

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.

고생하셨습니다~

Comment on lines +15 to +18
client.interceptors.request.use(async config => {
if (isServer) {
// 서버에서 실행되는 경우에는 쿠키에서 access_token 가져옵니다.
const { cookies } = await import('next/headers');
Copy link
Collaborator

Choose a reason for hiding this comment

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

넵 확인했습니다~

Comment on lines 33 to 41
try {
await login({
email: data.emailRequired,
password: newPassword,
});
} catch (error) {
console.error('Login failed:', error);
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

mutation에서 onSuccess/onError로 결과물이 출력되기 때문에 try-catch문이 없어도 괜찮니 않나요..??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 맞네요 바꾸고 못지웠네요ㅜㅜ

import { cookies } from 'next/headers';

function deleteTokenCookies() {
cookies().set('accessToken', '', {
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete를 accessToken만 하고 있는데 추후에 refresh도 추가할 예정일까요??

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만 넣어서 그렇습니다! 이후 쿠키에 access도 넣게 된다면 추가 해야될 것입니다!

const accessToken = loginData.access_token;
const refreshToken = loginData.refresh_token;

setCookie('accessToken', accessToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기에 refresh는 저장할 필요는 없나요..??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

음... 으음........흠......
리프레시 로직이 있을라면 있어야겠네요...
근데 뭔가 아쉽긴 한데...ㅠㅠ 더 좋은 방법이 있는지 고민해볼게요..
리프레시 로직은 166 브랜치에서 작업중이라 이후 브랜치에서 refresh 적용하고 위에 남겨주신 리뷰도 반영할게요!

import { ResponseCookie } from 'next/dist/compiled/@edge-runtime/cookies';
import { cookies } from 'next/headers';

export const setCookie = (key: string, value: string, options?: Partial<ResponseCookie>) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거 파일명이랑 함수명이랑 동일했으면 좋겠습니다!

@haejinyun haejinyun merged commit 308d128 into develop Dec 5, 2024
2 checks passed
@haejinyun haejinyun deleted the feat/#160 branch December 5, 2024 01:51
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.

[FEAT] 인증/인가 변경
2 participants