feat(nest.js/auth): add refreshToken api#147
Conversation
WalkthroughThis pull request introduces a token refresh flow across the backend and frontend. The NestJS backend adds a new Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Controller as AuthController
participant Service as AuthService
participant TokenSvc as TokenService
participant Redis as Redis
Client->>Controller: POST /auth/token/refresh<br/>{token: refreshToken}
Controller->>Service: refreshToken(body.token)
Service->>Service: Decode token payload
Service->>Service: Validate token<br/>(check no refreshTokenJti)
Service->>TokenSvc: getTokenByJti(id, jti, 'rt')
TokenSvc->>Redis: Get refresh token<br/>from cache
Redis-->>TokenSvc: Token record
TokenSvc-->>Service: Token found
Service->>Service: Extract associated<br/>access token JTI
Service->>TokenSvc: Revoke access token
TokenSvc->>Redis: Delete access token
Service->>TokenSvc: Revoke refresh token
TokenSvc->>Redis: Delete refresh token
Service->>Service: Generate new<br/>token pair
Service->>TokenSvc: Issue new tokens
TokenSvc->>Redis: Store new tokens
Service-->>Controller: {accessToken,<br/>refreshToken,<br/>TTLs}
Controller-->>Client: 200 OK<br/>New token pair
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
template/tinyvue/src/utils/auth.ts (1)
2-25: Refresh token is never cleared byclearToken(logout leaves a valid refresh token behind).
clearToken()(Line 21) still only removesTOKEN_KEY, so any stored refresh token underREFRESH_TOKEN_KEYpersists across logout and error paths. Given the new refresh flow, this means a “logged out” client could still silently obtain new access tokens via the saved refresh token.I’d treat this as an essential fix and have
clearTokenclear both keys:-const clearToken = () => { - sessionStorage.removeItem(TOKEN_KEY); -}; +const clearToken = () => { + sessionStorage.removeItem(TOKEN_KEY); + sessionStorage.removeItem(REFRESH_TOKEN_KEY); +};This will also ensure that
getRefreshToken()returnsnullafter logout/reset, keeping the store state consistent.
♻️ Duplicate comments (1)
template/tinyvue/src/store/modules/user/index.ts (1)
96-105: Align login/logout error paths with the new refresh token lifecycle.In
loginyou now set bothaccessTokenandrefreshTokenin state and storage (Lines 100–105), but in the error path (Lines 135–137) and inlogout(Lines 151–160) you only callclearToken(). Combined with the currentclearTokenimplementation, this leaves the refresh token untouched until the utils fix is applied.Once you update
clearTokento removeREFRESH_TOKEN_KEYas well (seetemplate/tinyvue/src/utils/auth.tscomment), these paths will correctly clear both tokens. Please just double‑check that there are no other custom “partial clear” scenarios that relied onclearTokenremoving only the access token.Also applies to: 151-160
🧹 Nitpick comments (3)
template/nestJs/src/auth/dto/refresh-token.dto.ts (1)
3-6: Add@IsString()decorator for type validation.The
@IsNotEmpty()decorator only checks that the value is not empty but doesn't validate the type. Adding@IsString()ensures the token is a string and prevents potential type coercion issues.-import { IsNotEmpty } from "class-validator"; +import { IsNotEmpty, IsString } from "class-validator"; export class RefreshToken { + @IsString() @IsNotEmpty() token: string; }template/tinyvue/src/api/user.ts (1)
47-53: Consider renamingflushTokentorefreshTokenfor clarity.The function name
flushTokenis unconventional for a token refresh operation. "Flush" typically implies clearing or emptying, whereas this function obtains a new token pair. Consider renaming torefreshTokenorrenewTokenfor better semantic clarity and alignment with the endpoint name/auth/token/refresh.-export interface RefreshToken { +export interface RefreshTokenRequest { token: string; } -export function flushToken(data: RefreshToken){ +export function refreshToken(data: RefreshTokenRequest){ return axios.post<LoginResponse>(`${import.meta.env.VITE_BASE_API}/auth/token/refresh`, data) }template/nestJs/src/auth/auth.controller.ts (1)
1-21: Refresh endpoint wiring looks correct and consistent with DTO/frontend.The new
POST /auth/token/refreshendpoint correctly uses theRefreshTokenDTO and delegates toauthService.refreshToken(body.token), matching the frontendflushTokencontract.Optionally, you could add an explicit return type for
refreshToken()(e.g.,Promise<LoginResponse>or similar) to keep controller signatures self‑documenting, in line with other methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
template/nestJs/src/auth/auth.controller.ts(1 hunks)template/nestJs/src/auth/auth.service.ts(2 hunks)template/nestJs/src/auth/dto/refresh-token.dto.ts(1 hunks)template/nestJs/src/auth/token.service.ts(2 hunks)template/tinyvue/src/api/user.ts(1 hunks)template/tinyvue/src/store/modules/user/index.ts(4 hunks)template/tinyvue/src/store/modules/user/types.ts(1 hunks)template/tinyvue/src/utils/auth.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
template/tinyvue/src/api/user.ts (3)
template/nestJs/src/auth/dto/refresh-token.dto.ts (1)
RefreshToken(3-6)template/tinyvue/src/store/modules/user/index.ts (1)
flushToken(69-81)template/nestJs/src/auth/dto/login-in.dto.ts (1)
LoginResponse(1-6)
template/nestJs/src/auth/dto/refresh-token.dto.ts (1)
template/tinyvue/src/api/user.ts (1)
RefreshToken(47-49)
template/nestJs/src/auth/auth.service.ts (3)
template/nestJs/src/auth/entity/token.ts (2)
AccessTokenPayload(11-15)RefreshTokenPayload(16-20)template/nestJs/src/auth/auth.controller.ts (1)
refreshToken(17-21)template/nestJs/libs/utils/pick.ts (1)
pick(1-16)
template/nestJs/src/auth/auth.controller.ts (2)
template/nestJs/src/auth/dto/refresh-token.dto.ts (1)
RefreshToken(3-6)template/tinyvue/src/api/user.ts (1)
RefreshToken(47-49)
template/tinyvue/src/store/modules/user/index.ts (4)
template/tinyvue/src/utils/auth.ts (4)
getRefreshToken(25-25)getToken(25-25)setRefreshToken(25-25)setToken(25-25)template/nestJs/src/auth/auth.service.ts (2)
getToken(28-30)refreshToken(46-76)template/nestJs/src/auth/auth.controller.ts (1)
refreshToken(17-21)template/tinyvue/src/api/user.ts (1)
flushToken(51-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-test
🔇 Additional comments (4)
template/nestJs/src/auth/token.service.ts (2)
185-192: LGTM!The
getTokenByJtimethod is well-designed with a clear interface. The type parameter effectively reuses the key prefix pattern used elsewhere in the service. Consider adding an explicit return type annotation for documentation purposes.async getTokenByJti( id: number, jti: string, type: 'at' | 'rt' - ) { + ): Promise<string | null> { const redis = this.redisService.getRedis(); return redis.get(`${type}:${id}:${jti}`); }
36-36: LGTM!Making
revokeExpiredTokenprivate is appropriate encapsulation since it's only used internally withinissueToken.template/nestJs/src/auth/auth.service.ts (1)
36-44: LGTM with a note on silent success.The logout flow improvement to look up the specific token before revocation is good. The silent success when token is not found (already expired/revoked) is acceptable behavior for logout operations.
template/tinyvue/src/store/modules/user/types.ts (1)
41-41: Consider making token fields optional if the store is used before authentication.The
refreshTokenandaccessTokenfields are currently required strings. If the store can be initialized before login, this may cause type errors. Verify the store initialization pattern: if tokens are always set before state creation, the current approach is acceptable; otherwise, consider usingstring | nullto allow for pre-authentication states.
| async refreshToken( | ||
| maybeToken: string | ||
| ){ | ||
| const token = this.jwtService.decode<AccessTokenPayload | RefreshTokenPayload>(maybeToken); | ||
| if ('refreshTokenJti' in token) { | ||
| throw new HttpException( | ||
| this.i18n.translate('exception.common.tokenError'), | ||
| HttpStatus.BAD_REQUEST | ||
| ) | ||
| } | ||
| const refresTokenObject = token as RefreshTokenPayload; | ||
| const {id, jti, accessTokenJti, email} = refresTokenObject; | ||
| const refreshToken = await this.tokenService.getTokenByJti(id, jti,'rt'); | ||
| if (!refreshToken) { | ||
| throw new HttpException( | ||
| this.i18n.translate('exception.common.tokenExpire'), | ||
| HttpStatus.UNAUTHORIZED | ||
| ) | ||
| } | ||
| const accessToken = await this.tokenService.getTokenByJti(id, accessTokenJti, 'at'); | ||
| if (accessToken){ | ||
| await this.tokenService.revokeToken(accessToken); | ||
| } | ||
| await this.tokenService.revokeToken(refreshToken); | ||
| const tokenPair = await this.tokenService.createToken(id, email); | ||
| // 颁发一个新的token | ||
| // issueToken 内部会在颁发前踢出最老的会话, 也会删除过期的会话, 这里就不用调用 this.tokenService.revokeExpiredToken 了 | ||
| await this.tokenService.issueToken(id, tokenPair); | ||
| // 返回一个新的TokenPair | ||
| return pick(tokenPair, ['accessToken', 'accessTokenTTL', 'refreshToken', 'refreshTokenTTL']) | ||
| } |
There was a problem hiding this comment.
Critical: revokeToken called with refresh token will fail.
The revokeToken method in TokenService (line 28) expects an access token because it decodes an AccessTokenPayload and extracts refreshTokenJti. However, on line 69, you're passing a refresh token string, which has accessTokenJti instead of refreshTokenJti. This will cause the revocation to fail or delete incorrect keys.
You need to revoke the refresh token directly using its JTI and ID rather than calling revokeToken:
const accessToken = await this.tokenService.getTokenByJti(id, accessTokenJti, 'at');
if (accessToken){
await this.tokenService.revokeToken(accessToken);
}
- await this.tokenService.revokeToken(refreshToken);
+ // Revoke refresh token directly - cannot use revokeToken() as it expects access token format
+ const redis = this.redisService.getRedis();
+ await redis.del(`rt:${id}:${jti}`);
+ await redis.lrem(`user:${id}:rt`, 0, jti);Alternatively, add a dedicated revokeRefreshToken method to TokenService that handles refresh token revocation properly.
Committable suggestion skipped: line range outside the PR's diff.
| async refreshToken( | ||
| maybeToken: string | ||
| ){ | ||
| const token = this.jwtService.decode<AccessTokenPayload | RefreshTokenPayload>(maybeToken); |
There was a problem hiding this comment.
Security concern: Token decoded without signature verification.
The decode method only parses the JWT without verifying its signature, unlike logout which uses verify() on line 37. An attacker could craft a JWT with arbitrary claims. While the Redis lookup at line 58 provides some protection (the token string must match), consider using verify() for consistency and defense-in-depth.
- const token = this.jwtService.decode<AccessTokenPayload | RefreshTokenPayload>(maybeToken);
+ const verifyResult = await this.jwtService.verify<RefreshTokenPayload>(maybeToken);
+ if (!verifyResult.valid) {
+ throw new HttpException(
+ this.i18n.translate('exception.common.tokenError'),
+ HttpStatus.BAD_REQUEST
+ );
+ }
+ const token = verifyResult.payload;
if ('refreshTokenJti' in token) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In template/nestJs/src/auth/auth.service.ts around line 49, the code uses
jwtService.decode(...) which parses the JWT without verifying its signature;
replace decode with jwtService.verify<AccessTokenPayload |
RefreshTokenPayload>(maybeToken) and wrap the call in a try/catch to handle
JsonWebTokenError/TokenExpiredError, returning null or throwing an auth-specific
error on verification failure; keep the subsequent Redis token lookup intact to
ensure the exact token string is still checked, and ensure the verified payload
is properly type-guarded before use.
| refreshToken: getRefreshToken(), | ||
| accessToken: getToken() | ||
| }), |
There was a problem hiding this comment.
Guard against missing/invalid refresh token and improve flushToken action behavior.
flushToken() currently assumes this.refreshToken is always a non‑null string (Line 70) and casts it, then calls the API without any guard or error handling. After your utils change to clear both tokens, this.refreshToken can legitimately be null, and this action would send { token: null } to the backend and give callers no way to await or react to failures.
Consider tightening this action along these lines:
- If there is no
refreshTokenin state (or storage), short‑circuit (e.g., throw or trigger a logout) instead of calling the API. - Make the action
asyncand return the Promise so callers canawaitit and handle errors. - On refresh failure (e.g., 401/invalid refresh token), clear local tokens and user state to avoid being stuck with unusable credentials.
For example:
- flushToken(){
- const refreshToken = this.refreshToken as string;
- flushToken({token: refreshToken})
- .then((tokenPair) => {
- return tokenPair.data
- })
- .then((data) => {
- this.refreshToken = data.refreshToken;
- this.accessToken = data.accessToken;
- setRefreshToken(data.refreshToken)
- setToken(data.accessToken);
- })
- },
+ async flushToken() {
+ const refreshToken = this.refreshToken;
+ if (!refreshToken) {
+ // e.g. optionally: await this.logout(); or throw an error
+ throw new Error('No refresh token available');
+ }
+ const { data } = await flushToken({ token: refreshToken });
+ this.refreshToken = data.refreshToken;
+ this.accessToken = data.accessToken;
+ setRefreshToken(data.refreshToken);
+ setToken(data.accessToken);
+ },You can then add targeted handling (e.g., try/catch + logout) at call sites if needed.
Also applies to: 69-81
|
@GaoNeng-wWw 想了解下这个新的 API 有什么作用呢？已经有了 setToken / getToken 了，登录功能也是正常的，增加 setRefreshToken/getRefreshToken 的作用是什么？使用场景是什么呢？没有这个 API 会有什么问题呢？是用来做 token 过期之后自动刷新 token，持续保持登录态对吗？ |
没有这个API其实不会有任何问题，主要是上一个pr写了refreshToken, 不写一个刷新令牌的接口总感觉很奇怪。 并不是保持登录态，而是根据refreshToken来获取一个新的TokenPair。 |
|
2 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
提供了令牌刷新接口
output.mp4
What is the current behavior?
Issue Number: N/A
What is the new behavior?
可以通过调用
来获取一个新的令牌对.
Does this PR introduce a breaking change?
对于存量应用, 后端无需修改, 前端需要提供令牌刷新及吊销的支持. 可以参考
template/tinyvue/src/store/modules/user/index.tsOther information
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.