jwt 인증에 관련된 기능들 생성 #57
Conversation
…main-back into feat/#39/jwt-verification
…main-back into feat/#48/redis-apply-test
gwgw123
requested review from
JeonHeena,
dg1418,
Contingency1,
NicoDora and
cyoure
as code owners
src/auth/guard/jwt.guard.ts
Outdated
| // refresh 토큰이 있는지 검사 | ||
| if (!refreshToken) { | ||
| throw new UnauthorizedException('No Refresh Token provided'); | ||
| } | ||
|
|
||
| try { | ||
| // refresh 토큰 변조, 만료를 검사 | ||
| const payload = jwt.verify( | ||
| refreshToken, | ||
| this.configService.get<string>('REFRESH_SECRET'), | ||
| ) as any; | ||
|
|
||
| const { userId } = payload; | ||
|
|
||
| // redis에 해당 refresh 토큰 있는지 확인인 | ||
| const redisRefreshToken = await this.redisService.get(userId); | ||
| if (!redisRefreshToken || redisRefreshToken !== refreshToken) { | ||
| throw new UnauthorizedException('Invalid Refresh Token'); | ||
| } | ||
|
|
||
| // access 토큰 재발급 및 쿠키에 담기 | ||
| const newAccessToken = | ||
| await this.tokenService.createAccessToken(userId); | ||
| this.setAccessTokenCookie(response, newAccessToken); | ||
|
|
||
| const user = await this.usersService.getUser(userId); | ||
| if (!user) throw new UnauthorizedException('User not found'); | ||
|
|
||
| request['user'] = user; | ||
| return true; | ||
| } catch (refreshError) { | ||
| throw new UnauthorizedException('Invalid or Expired Refresh Token'); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
가드에서 리프레시 토큰까지 검증하여 액세스 토큰을 재발급 하는 로직은 맞지 않는거 같습니다.
가드 본인의 역할(액세스 토큰 검증)만 하도록 작성하고 액세스 토큰 재발급은 따로 API를 만드셔서 token.service 파일에서 로직 작성하시면 더 깔끔해질 것 같습니다.
이렇게 기본 기능만 담백하게 담아놓으면 나중에 토큰을 검증하는 더 복잡한 가드를 만든다고 했을 때 해당 가드만 가져다가 extends 하면 되기 때문에 확장성도 높아집니다.
There was a problem hiding this comment.
네 확실히 가드의 역할과 다르게 설계된거 같습니다.
이후 확장성을 위해서 재발급 로직은 비즈니스 로직에서 담당하도록 API를 만들어야 하겠습니다
src/auth/guard/logout.guard.ts
Outdated
There was a problem hiding this comment.
LogoutGuard에서 리프레시 토큰을 검증하고 있는데, 리프레시 토큰은 오직 액세스 토큰 재발급에만 사용되어야 하기 때문에 이런 검증방식은 적절하지 않아 보입니다. (물리적으론 액세스 토큰, 리프레시 토큰 둘 다 인증이 가능하지만, 논리적으로 분리하는게 좋습니다.)
그리고 LogoutGuard에서 검증하는 토큰을 액세스 토큰으로 바꾸게 되면, 결국에는 JwtGuard랑 로직이 같아지기 때문에 실제 로그아웃 API에서도 JwtGuard만 사용하면 될 것 같습니다.
There was a problem hiding this comment.
jwt의 설계부터가 잘 못 되다보니 이렇게 가드를 하나 더 만들게되네요
지금 가드 두개에 중복되는 기능이 있으니
JwtGuard 하나를 확장성 있게 사용하도록 수정해야겠습니다
|
기존 jwt 가드에서 |
|
환경변수를 토큰이나 쿠키 세팅 값으로 넣어줄때 |
src/auth/auth.controller.ts
Outdated
| } | ||
|
|
||
| // refreshToken을 검증하고 accessToken을 재발급 | ||
| @Get('verify-refreshToken') |
There was a problem hiding this comment.
verify-refreshToken이 틀린건 아니지만 리프레시 토큰을 검증하고 새 액세스 토큰을 재발급하기 때문에 이 API의 목적은 '액세스 토큰 재발급' 이라 볼 수 있습니다. 따라서 API의 원래 목적을 바로 알 수 있도록 url을 new-accessToken이나 accessToken으로 네이밍 하는걸 추천드립니다.
There was a problem hiding this comment.
오 네이밍 추천감사합니다
new-accessToken 이 맘에 드는거 같습니다 ~
src/auth/redis/redis.module.ts
Outdated
| host: configService.get<string>('REDIS_HOST'), | ||
| port: configService.get<number>('REDIS_PORT'), | ||
| password: configService.get<string>('REDIS_PASSWORD'), | ||
| ttl: Number(configService.get<number>('REDIS_EXPIRATION_TIME')), |
There was a problem hiding this comment.
| ttl: Number(configService.get<number>('REDIS_EXPIRATION_TIME')), | |
| ttl: Number(configService.get<string>('REDIS_EXPIRATION_TIME')), |
env에서 REDIS_EXPIRATION_TIME을 string타입으로 가져오기 때문에 string으로 명시해줘야 오해가 없을거 같습니다.
There was a problem hiding this comment.
맞네요 어차피 스트링타입으로 가져오니까 그렇게 변경해야겠습니다~
|
👍👍👍 |
🔗 관련 이슈
#39 해당 pr을 올린 브랜치의 이슈입니다.
#46 prisma Client가 binaryTargets 에 관한 버전 불일치 에러를 발생시켜
Dockerfile에 openssl3 버전을 명시하여 해결했습니다.
#48 redis 적용을 위한 이슈를 생성했었습니다.
📝작업 내용
🔍 변경 사항
💬리뷰 요구사항 (선택사항)