회원가입 로직 변경 #405

NamJwong · 2022-09-11T14:49:06Z

⛓ Related Issues

close 로그인 로직 수정 #385

📋 작업 내용

기존 로직에 보안 상 문제가 있어 서버에서 수정해준 로그인/회원가입 api에 따라 바뀌어야 할 부분을 작업했습니다.
이 과정에서 리팩토링도 몇 가지 했습니다.

회원가입 로직 변경에 대해선 아래 PR Point에 설명을 썼습니다.
그 자세한 구현 방법이랑 리팩토링 한 내용에 대해서는 코드에 코멘트를 달아놓겠습니다!

크게 문제 되지 않는 자잘한 버그와 리팩토링하고 싶은 요소들이 남아있는데.. (일단 #385 에 정리해두었습니다)
이 이슈에 변경 사항이 너무 많아져서 분리를 하려고 합니다.
그리고 기존에 함께 하려고 했던 refresh token 적용도 따로 이슈 파두었습니다.

📌 PR Point

회원가입 로직 변경

왜 변경하게 됐는지에 대해서도 함께 적었는데, 작업 내용만 확인하시려면 [클라이언트 수정 사항] 부분을 봐주세요!

[발생한 문제]

기존 회원가입 로직은 다음과 같습니다.

login post 요청
회원가입을 해야 하는 경우라면 카카오 서버에서 내려준 토큰을 response로 받음
회원가입 뷰로 navigate 후 유저 정보 입력을 받음
(서버에서 회원가입을 완료할 수 있도록) 입력 받은 유저 정보와 토큰을 가지고 join post 요청
요청 성공 시 등록된 유저 정보와 서버에서 발급한 토큰(암호화 시킴)을 response로 받음

여기서 문제점은 2~4 과정에서 암호화되지 않은 카카오 토큰이 노출된다는 것입니다.
어진이가 보안 상 위험하다고 한 것이 이것입니다..

[해결 방법]

기존 방법대로 했던 이유는 서버가 유저 정보를 가지고 있는 상태에서 유저 생성을 마치기 위함이었습니다.
따라서 login post 요청 시 일단 서버 내부에서 카카오 토큰을 가지고 유저 생성을 마친 후, 유저 정보 post 요청을 하기로 했습니다.

[클라이언트 수정 사항]

고려해야 하는 것

최초 로그인을 하면 일단 유저 생성은 되지만, 유저 정보가 모두 입력되어야 서비스를 이용할 수 있습니다.
그런데 최초 로그인 시도 후 사용자가 유저 정보를 입력하지 않고 이탈하는 일이 발생할 수도 있게 되었습니다.

따라서 회원 상태를 아래와 같이 나누어 관리해야 했습니다.

최초 로그인 시도 후 유저 등록을 마치지 않음 (이하 회원가입 미완료 상태)
최초 로그인 시도 후 유저 등록을 마침 (이하 회원가입 완료 상태)

적용 방법
유저 정보에 isJoined라는 값을 추가하여 회원 상태를 구분하기로 했습니다.
isJoined가 false라면 서비스 이용이 불가하며 회원가입 뷰로 navigate 되도록 했습니다.
- isJoined 만드는 법

회원가입 미완료 상태일 경우 서버에서 주는 response는 다음과 같은 형태로 받기로 했습니다.

{
	"status":200,
	"success":true,
	"message":"회원 가입 성공",
	"data":{
		"user":{
			"id":758,
			"profileId":"",
			"name":"",
			"provider":"kakao",
			"createdAt":"2022-08-29T15:57:59.643Z",
			"updatedAt":"2022-08-29T15:57:59.643Z",
			"isDeleted":false,
                         "refreshToken": [유저의 너가소개서 refresh 토큰],
			"image":""
		},
		"accesstoken": [유저의 너가소개서 access 토큰]
	}
}

유저가 회원가입 뷰에서 필수로 입력해야 하는 profileId, name가 빈 값입니다.
따라서 login post 요청 후 response에서 위 두 가지 값을 확인하고 모두 빈 값이라면 isJoined를 false로 설정했습니다.

SeojinSeojin · 2022-09-11T14:49:20Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-11T15:01:27Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-11T15:11:41Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-13T06:48:06Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-13T06:56:24Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-13T07:03:22Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-15T09:05:55Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

NamJwong

변경 사항이 많아 설명이 필요한 부분은 각각 코멘트로 달아두었습니다 😀

NamJwong · 2022-09-15T13:17:57Z

src/application/utils/constant.ts

+const isProduction = process.env.NODE_ENV === 'production';
+export const DOMAIN = isProduction ? 'https://naegasogaeseo-dev.kro.kr' : 'http://localhost:3000';
+
+export const PAGES = { KEYWORD: 15, NOTICE: 15, PICK: 10, SEARCHED_USER: 16 };


constant.ts가 꽤 복잡해져서 파일 분리를 할까 하다가 일단 페이지라도 모아주었습니다
마음에 안 드시면 바꾸어도 돼용

파일 분리보다는 이 방식이 더 좋은 것 같습니다 👍🏻

NamJwong · 2022-09-15T13:19:48Z

src/application/utils/constant.ts

@@ -17,4 +19,5 @@ export const STATUS_CODE = {
  SERVICE_UNAVAILABLE: 503,
  DB_ERROR: 600,
 };
-export const SEARCHED_USER_PAGE = 16;
+
+export const TOKEN_KEYS = { ACCESS: 'accessToken', REFRESH: 'refreshToken' };


refresh 토큰을 활용하게 될 거여서 로컬스토리지의 토큰 키 명을 'token'에서 바꾸어야 했는데 아무래도 상수로 관리하는 게 좋겠다 싶어서 뺐습니다~

NamJwong · 2022-09-15T13:20:49Z

src/infrastructure/api/login-user.ts

+  postLogin(authorizationCode: string): Promise<LoginUser>;
+  postUserInfo(userInfo: FormData): Promise<LoginUser | null>;


컨벤션과 맞지 않게 이 파일 내에 선언돼있던 함수들을 지우고 interface에 추가해주었습니다

NamJwong · 2022-09-15T13:23:42Z

src/infrastructure/remote/user.ts

@@ -46,6 +46,7 @@ export function userDataRemote(): UserService {
        throw new NotFoundError('사용자를 찾을 수 없습니다.');
    });
    return {
+      id: response.data.user.id,


User 타입에 DB 아이디를 추가하여 적용해주었습니다

NamJwong · 2022-09-15T13:26:03Z

src/presentation/pages/Join/index.tsx

-function JoinPage() {
  return (
    <>
-      <JoinForm />


이런 방식이 다른 페이지 컴포넌트의 컨벤션과 달라 JoinForm 컴포넌트의 내용을 가져와 합쳤습니다
그리고 JoinPage 컴포넌트 명 또한 ~Page 형식이 컨벤션에 맞지 않아 Join으로 수정했습니다

NamJwong · 2022-09-15T13:27:12Z

src/presentation/pages/Login/index.tsx

-function LoginPage() {
  return (
-    <>
-      <LoginForm />
-    </>


Join 컴포넌트와 같은 이유로 컴포넌트를 합치고 컴포넌트 명을 바꾸었습니다

NamJwong · 2022-09-15T14:57:43Z

src/presentation/pages/OAuthRedirectHandler/index.tsx

-    });
+    const authorizationCode = new URL(window.location.href).searchParams.get('code') ?? '';
+    if (authorizationCode.length) login(authorizationCode);
+    else throw '카카오 인가 코드 조회 실패';


혹시라도 인가 코드를 받아오지 못했을 때 유저가 이 컴포넌트에 갇히게 되어 에러바운더리에 걸리도록 에러 처리를 해주었습니다

NamJwong · 2022-09-15T15:01:37Z

src/presentation/pages/OAuthRedirectHandler/index.tsx

-  const setKakaoAccessToken = useSetRecoilState(kakaoAccessTokenState);
-  const setKakaoRefreshToken = useSetRecoilState(kakaoRefreshTokenState);


이 전역 state들은 아예 삭제해주었습니다
loginUser를 전역에 저장하고 있고, 그 안에 로그인 유저 관련 정보(isJoined, 유저 정보, 토큰들)를 모두 모아서 관리하는 것이 좋을 것 같아 토큰도 그 안에 넣었습니다

참고로 기존에도 loginUser에 토큰 정보를 포함하고 있어서 중복이기도 했습니다

NamJwong · 2022-09-15T15:11:55Z

src/application/utils/constant.ts

-export const NOTICE_PAGE = 15;
-export const PICK_PAGE = 10;
+const isProduction = process.env.NODE_ENV === 'production';
+export const DOMAIN = isProduction ? 'https://naegasogaeseo-dev.kro.kr' : 'http://localhost:3000';


이거는... dev에 머지되면 안되는가?

언니가 작성한 대로 dev 브랜치에 반영하고,
나중에 dev -> release로 머지할 때 까먹지 않고 release 브랜치에서만 아래처럼 바꾸면 괜찮을 것 같습니다!

export const DOMAIN = isProduction ? 'https://neogasogaeseo.com' : 'http://localhost:3000';

NamJwong · 2022-09-15T15:14:12Z

src/application/stores/login-user.ts

+export const isAuthenticatedState = atom<boolean>({
+  key: 'isAuthenticatedState',


사용할 때의 이름과 맞추는 것이 컨벤션이어서 수정했습니다~

역시 컨벤션 경찰 🚨

100Gyeon · 2022-09-18T14:25:30Z

src/presentation/pages/Join/index.tsx

+          onChange={(value) => {
+            setInputName(value);
+          }}


Suggested change

onChange={(value) => {

setInputName(value);

}}

onChange={(value) => setInputName(value)}

이 부분 한 줄로 줄이는 건 어떨까용?

100Gyeon · 2022-09-18T14:44:58Z

src/application/hooks/useLoginUser.ts

+      const refreshToken = localStorage.getItem(TOKEN_KEYS.REFRESH);
+      if (accessToken && refreshToken) {
+        const user = await api.loginUserService.getUserInfo(accessToken);
+        if (user.userID && user.username && user.profileImage) {


여기 user.이 반복되는데 구조 분해 할당 사용하면 어떨까요?

아래서 user를 통째로 쓰기도 해야 하는데, 그럼 구조 분해 할당을 위해 한 줄을 추가해야 해서 요렇게 했습니다!

100Gyeon · 2022-09-18T14:49:15Z

src/infrastructure/remote/login-user.ts

+      });
+    const { id, profileId, name, image, refreshToken } = response.data.user;
+    return {
+      isJoined: profileId.length > 0 && name.length > 0,


Suggested change

isJoined: profileId.length > 0 && name.length > 0,

isJoined: profileId.length && name.length,

> 0 없어도 똑같이 동작할 것 같아요 !!

이미 머지 해버렸지만...... 이렇게 하면 isJoined에 number가 들어가서 안됩니다 !! 저도 이걸 잊고 그대로 반영해버렸는데 #408 에서 고칠게용

100Gyeon

PR 꼼꼼하게 적어주셔서 이해하기 편했습니다 고생하셨어요 😊
슬랙 한 번만 확인 부탁드려용!

SeojinSeojin · 2022-09-21T03:03:30Z

src/App.tsx

@@ -9,6 +9,7 @@ import ToastList from '@components/common/Toast/List';

 function App() {
  const { initLoginUser } = useLoginUser();
+


엔터 좋습니다 ..

SeojinSeojin · 2022-09-21T03:03:59Z

src/application/hooks/useLoginUser.ts

    initLoginUser();
  };

  const removeAccessToken = () => {
-    localStorage.removeItem('token');
+    localStorage.removeItem(TOKEN_KEYS.ACCESS);


우와 Enum사용 넘 좋습니당

SeojinSeojin · 2022-09-21T03:05:16Z

src/application/hooks/useLoginUser.ts

+    setLoginUser({
+      isJoined: false,
+      accessToken: '',
+      refreshToken: '',
+      user: { id: -1, userID: '', username: '', profileImage: '' },
+    });


이것은 초기화할때마다 쓸 이니샬 스테이트니까 constant 같은 곳에 빼두면 좋을듯해용~!!

레코일에서 atom 처음 선언할 때도 쓰는 친구네용
나중에 수정할 때 한번에 하면 편하니 상수로 빼도 좋을 것 같아용~!

SeojinSeojin · 2022-09-21T03:06:26Z

src/application/hooks/useLoginUser.ts

+            user: user,
+          });
+        }
+      } else throw '토큰이 없습니다';


이거 그 .. 에러 코드로 싼 그걸로 감싸서 뜨로우해주면 좋을 것 같아용

SeojinSeojin · 2022-09-21T03:07:32Z

src/application/hooks/useLoginUser.ts


 export function useLoginUser() {
  const [loginUser, setLoginUser] = useRecoilState(loginUserState);
-  const [isAuthenticated, setIsAuthenticated] = useRecoilState(authState);
+  const [isAuthenticated, setIsAuthenticated] = useRecoilState(isAuthenticatedState);


사실 이 스테이트. . 로그인유저스테이트랑 합쳐도 된다고 생각했는데, 지금도 좋아 보여용

SeojinSeojin · 2022-09-21T03:11:03Z

src/application/hooks/useLoginUser.ts

+    localStorage.setItem(TOKEN_KEYS.ACCESS, loginUser.accessToken);
+    localStorage.setItem(TOKEN_KEYS.REFRESH, loginUser.refreshToken);


이 부분 .. loginUser이 바뀔 때마다 호출되어야 하는 친구같은데, useEffect안에 의존성 배열에 [loginUser]을 담고 바뀔때마다 호출해주면 어떨까용?

위에 있는 removeAccessToken이랑 합쳐서,
loginUser.accessToken이 ''이면 removeItem하고 ''이 아니면 setItem해주는거죵

지금 setLoginUser를 할 수단이 saveLoginUser랑 removeAccessToken 밖에 없는 상황이어서여 그 안에서 로컬스토리지까지 다 관리하고 있으니 괜찮지 않을까요..?!

SeojinSeojin · 2022-09-21T03:14:07Z

src/presentation/pages/Join/index.tsx

+    id: false,
+    name: false,
+  });
+  const [errorMsg, setErrorMsg] = useState('');


에러가 없으면 에러메쉬지가 아예 undefined면 구별할 때 좋을 것 같아용
하지만 이것은 자유 입니다 ..

SeojinSeojin · 2022-09-21T03:15:05Z

src/presentation/pages/Join/index.tsx

+        saveLoginUser(loginUser);
+        navigate('/join/complete');
+      } else {
+        fireToast({ content: '중복된 아이디입니다.' });


loginUser이 없으면 중복된 아이디인거에용?

204가 오면.. 입니다 안 그래도 그게 코드상으로 표현이 잘 안돼서 고민했던 부분이긴 함다 . .

SeojinSeojin · 2022-09-21T03:16:36Z

src/presentation/routes/common/PrivateRoute.tsx

  if (error) throw error;
-  return isLoading ? <></> : isAuthenticated ? <Outlet /> : <Navigate to="/" />;


지옥의 삼항연산자 .. if로 풀어쓰는게 더 좋아보이네용

SeojinSeojin · 2022-09-26T01:23:44Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-26T02:18:42Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-26T02:26:16Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-26T02:37:42Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-26T03:47:01Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-26T04:03:46Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-27T06:27:18Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

SeojinSeojin · 2022-09-27T06:54:52Z

울 웹쁜이 고생많았어 ! 여기서 미리보기로 보면서 쉬어~ 다른 웹쁜이들한테도 자랑해줘~

NamJwong added 9 commits September 4, 2022 12:43

🔨 refactor: constants.ts의 page 관련 상수 한 객체로 모으기

62e6174

🔨 refactor: 인가 코드 변수명 authorizationCode로 통일

c479199

📝 chore: 변경될 로그인 로직에 맞춰 관련 타입 수정

c089c25

✨ feat: 로그인 요청 함수 새로 만들기

bd2c1d0

🔨 refactor: LoginForm과 LoginPage 컴포넌트 합치기

7294a26

📝 chore: isJoined 판단 로직 더욱 안전하게 만들기

0f084a3

🔨 refactor: recoil atom 네이밍 컨벤션 맞추기

0087ce0

🔨 refactor: JoinPage 컴포넌트와 JoinForm 컴포넌트 합치기

006cce6

✨ feat: 로그인 로직 수정 (isJoined 활용)

84989c6

NamJwong added the feature 🎄 기능 개발 label Sep 11, 2022

NamJwong requested review from SeojinSeojin and 100Gyeon September 11, 2022 14:49

NamJwong self-assigned this Sep 11, 2022

📝 chore: User 타입에 DB id 추가하면서 바뀐 타입 수정

dabf16b

📝 chore: loginUserState 아톰 타입 수정

9c3a410

NamJwong added 2 commits September 13, 2022 15:18

📝 chore: api 에러 코드 수정 사항 반영

f3d3f5c

🐛 fix: 회원가입 진행 중인 유저 private router 접근 막는 방법 수정

3480462

NamJwong added 2 commits September 13, 2022 15:52

Merge branch 'dev' into feat/#385

81cd737

📝 chore: production용 도메인 내가소개서로 수정

dfd0f5c

NamJwong added 2 commits September 13, 2022 16:01

🔨 refactor: 토큰 키 상수 네이밍 수정

2607131

📝 chore: 바뀐 로컬스토리지 토큰 key 반영

4a619f9

📝 chore: 서버 api 수정 사항 반영

25d284e

📝 chore: postUserInfo privateAPI로 변경

0a40f96

NamJwong commented Sep 15, 2022

View reviewed changes

100Gyeon reviewed Sep 18, 2022

View reviewed changes

100Gyeon approved these changes Sep 18, 2022

View reviewed changes

SeojinSeojin reviewed Sep 21, 2022

View reviewed changes

NamJwong force-pushed the feat/#385 branch from 77045dc to c2941bd Compare September 27, 2022 06:27

NamJwong added 3 commits September 27, 2022 15:34

🔨 refactor: 코드 길이 줄이기

1539d09

🔨 refactor: LoginUser 디폴트 값 상수로 관리

7019f4f

📝 chore: throw 에러 객체로 감싸기

7d959c4

NamJwong merged commit 8f9d9e5 into dev Sep 30, 2022

NamJwong deleted the feat/#385 branch September 30, 2022 06:31

		postLogin(authorizationCode: string): Promise<LoginUser>;
		postUserInfo(userInfo: FormData): Promise<LoginUser \| null>;

		const setKakaoAccessToken = useSetRecoilState(kakaoAccessTokenState);
		const setKakaoRefreshToken = useSetRecoilState(kakaoRefreshTokenState);

		export const isAuthenticatedState = atom<boolean>({
		key: 'isAuthenticatedState',

	isJoined: profileId.length > 0 && name.length > 0,
	isJoined: profileId.length && name.length,

		@@ -9,6 +9,7 @@ import ToastList from '@components/common/Toast/List';

		function App() {
		const { initLoginUser } = useLoginUser();

		localStorage.setItem(TOKEN_KEYS.ACCESS, loginUser.accessToken);
		localStorage.setItem(TOKEN_KEYS.REFRESH, loginUser.refreshToken);

		if (error) throw error;
		return isLoading ? <></> : isAuthenticated ? <Outlet /> : <Navigate to="/" />;

회원가입 로직 변경 #405

회원가입 로직 변경 #405

Conversation

NamJwong commented Sep 11, 2022 • edited

⛓ Related Issues

📋 작업 내용

📌 PR Point

회원가입 로직 변경

SeojinSeojin commented Sep 11, 2022

SeojinSeojin commented Sep 11, 2022

SeojinSeojin commented Sep 11, 2022

SeojinSeojin commented Sep 13, 2022

SeojinSeojin commented Sep 13, 2022

SeojinSeojin commented Sep 13, 2022

SeojinSeojin commented Sep 15, 2022

NamJwong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NamJwong Sep 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NamJwong Sep 27, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

100Gyeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeojinSeojin commented Sep 26, 2022

SeojinSeojin commented Sep 26, 2022

SeojinSeojin commented Sep 26, 2022

SeojinSeojin commented Sep 26, 2022

SeojinSeojin commented Sep 26, 2022

SeojinSeojin commented Sep 26, 2022

SeojinSeojin commented Sep 27, 2022

SeojinSeojin commented Sep 27, 2022

NamJwong commented Sep 11, 2022 •

edited

NamJwong Sep 15, 2022 •

edited

NamJwong Sep 27, 2022 •

edited