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

Create Token #9

Merged
merged 12 commits into from
Dec 22, 2021
Merged

Create Token #9

merged 12 commits into from
Dec 22, 2021

Conversation

louis7308
Copy link
Contributor

  1. Oauth2 연동 작업 개발 완료
  2. 대나무숲 전용 JWT 개발 완료
  3. JWT 생명유지 / 인증 개발 완료
  4. 기존 유저가 다시 로그인 했을때 DB값을 추가하지 않고 UPDATE해서 값을 추가 개발 완료
  5. 학교 이메일하고 지금 년도 비교 해서 학생인지 판단하는거 개발 완료

Copy link
Member

@sunrabbit123 sunrabbit123 left a comment

Choose a reason for hiding this comment

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

전체적으로 급하게 커밋을 한 느낌이 납니다. 주말 푹 쉬고, 필요가 없는 코드들은 이미 커밋 기록에 남아있으니 과감히 지우시고, 필요한 것들만 남김녀 좋을 것 같다는 생각이 듭니다.
예를 들어 OAuth 파트 중 클라이언트에서 처리하는 부분이 나와있는것들이 있네요.

Copy link
Member

@sunrabbit123 sunrabbit123 left a comment

Choose a reason for hiding this comment

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

리팩토링은 서비스 개발 완료 후에 추가로 진행하는 것이 좋을 것 같네요
몇가지 점만 수정하고 머지하면 될 것 같아요

if(accessToken === null) {
if(refreshToken === null) {
return createErrorRes({
errorCode: ERROR_CODE.JL001,
Copy link
Member

Choose a reason for hiding this comment

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

에러코드 JL001은 옳지않은 Origin일때 나타나는 에러입니다
새로 에러 코드를 선언해서 작성해주시길 바랍니다.

const decodeAccessToken: string | JwtPayload = await jwt_decode(req.headers.accessToken)
const refreshToken: string | JwtPayload = await decodeToken(req.headers.refreshToken)
const repo = getRepository(User);
const id = await repo.find({
Copy link
Member

Choose a reason for hiding this comment

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

유저의 id 조회는 필요할때만 조회해도 된다고 생각합니다.
if문 안쪽으로 넣는건 어떨까요?

const tokens : any = event.headers.id_token;
if(!tokens) {
return createErrorRes({
errorCode: ERROR_CODE.JL004,
Copy link
Member

Choose a reason for hiding this comment

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

JL004는 예상치못한 에러를 의미합니다.
새로 에러를 선언해서 작업해주시길 바랍니다.
참고 util/http.ts

src/util/http.ts Outdated
};
};

export const decodeToken = (token : string) : null | string | JwtPayload => {
Copy link
Member

Choose a reason for hiding this comment

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

http.ts보다는 token.ts가 어울릴 것 같아요

@louis7308 louis7308 merged commit f32a621 into develop Dec 22, 2021
@louis7308 louis7308 deleted the feature/oauth2 branch December 22, 2021 11:36
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.

2 participants