Add JWT refresh token support with rotation, validation and middleware enforcement#209
Draft
Add JWT refresh token support with rotation, validation and middleware enforcement#209
Conversation
WalkthroughAdds access+refresh JWT support with separate TTLs, token-pair generation and refresh/rotation flows, DB schema and repository changes for refresh tokens, new handler endpoint POST /token/refresh, middleware helpers for token access, and functional options for scope checks. Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthHandler
participant JWTService
participant Repository
participant Database
rect rgba(200,150,255,0.5)
Note over Client,AuthHandler: Initial token issuance
Client->>AuthHandler: POST /token (credentials)
AuthHandler->>JWTService: GenerateTokenPair(userID, scopes, accessTTL)
JWTService->>JWTService: build access & refresh claims, sign tokens
JWTService->>Repository: Insert(accessToken, refreshToken)
Repository->>Database: save rows
JWTService-->>AuthHandler: TokenPairInfo
AuthHandler-->>Client: {access_token, refresh_token, expires_at}
end
rect rgba(150,200,255,0.5)
Note over Client,AuthHandler: Refresh flow
Client->>AuthHandler: POST /token/refresh (Authorization: Bearer <refresh>)
AuthHandler->>JWTService: RefreshTokenPair(refreshToken)
JWTService->>JWTService: Parse & validate refresh token
JWTService->>Repository: RotateRefreshToken(currentJTI, nextRefresh, nextAccess)
Repository->>Database: lock current row, validate, mark revoked, insert new tokens
Repository-->>JWTService: success
JWTService-->>AuthHandler: TokenPairInfo(new)
AuthHandler-->>Client: {new_access, new_refresh, expires_at}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🧹 Nitpick comments (10)
internal/sms-gateway/jwt/config_test.go (2)
1-1: Consider using external test packagejwt_test.The linter flags that test files should use the
jwt_testpackage to ensure tests only access the public API. This helps validate the package's public interface and prevents tests from depending on unexported internals.Proposed fix
-package jwt +package jwt_test + +import ( + "testing" + "time" + + "github.com/android-sms-gateway/server/internal/sms-gateway/jwt" +)Then reference
jwt.Configinstead ofConfigin the test cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/config_test.go` at line 1, Change the test package from "jwt" to the external package "jwt_test" in config_test.go so tests exercise only the public API; update all references to the type/values to use the exported package qualifier (e.g., replace Config with jwt.Config and any other unqualified symbols), and add an import for the jwt package if not already present.
8-45: Expand test coverage forConfig.Validate.The test only covers 2 scenarios. Consider adding cases for other validation paths in
config.go:
- Empty secret
- Secret shorter than 32 bytes
- Zero or negative
AccessTTL- Zero or negative
RefreshTTL- Missing
Issuer(if validated)Additional test cases
tests := []struct { name string cfg Config wantErr bool }{ { name: "valid", cfg: Config{ Secret: "01234567890123456789012345678901", AccessTTL: 15 * time.Minute, RefreshTTL: 24 * time.Hour, Issuer: "sms-gate.app", }, }, { name: "refresh not greater than access", cfg: Config{ Secret: "01234567890123456789012345678901", AccessTTL: 24 * time.Hour, RefreshTTL: 24 * time.Hour, }, wantErr: true, }, + { + name: "empty secret", + cfg: Config{Secret: "", AccessTTL: time.Minute, RefreshTTL: time.Hour}, + wantErr: true, + }, + { + name: "secret too short", + cfg: Config{Secret: "short", AccessTTL: time.Minute, RefreshTTL: time.Hour}, + wantErr: true, + }, + { + name: "zero access ttl", + cfg: Config{Secret: "01234567890123456789012345678901", AccessTTL: 0, RefreshTTL: time.Hour}, + wantErr: true, + }, + { + name: "negative refresh ttl", + cfg: Config{Secret: "01234567890123456789012345678901", AccessTTL: time.Minute, RefreshTTL: -time.Hour}, + wantErr: true, + }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/config_test.go` around lines 8 - 45, Add additional subtests to TestConfigValidate that call Config.Validate to cover the missing/invalid-field branches: include cases for empty Secret, Secret shorter than 32 bytes, AccessTTL zero and negative, RefreshTTL zero and negative, and missing Issuer (if Validate checks it). For each case (referencing the Config struct fields Secret, AccessTTL, RefreshTTL, Issuer and the method Validate), set wantErr=true and assert an error is returned; keep the existing valid case expecting no error. Ensure each case has a descriptive name so test output pinpoints the failing validation branch.internal/sms-gateway/jwt/service.go (4)
173-177: Variable shadowing:errshadows outer declaration.The linter flags that
errat line 175 shadows theerrdeclared at line 143. While the logic works, shadowing can mask bugs and reduces readability.Proposed fix
nextRefreshModel := newTokenModel(nextRefreshClaims.ID, claims.UserID, TokenUseRefresh, nextRefreshClaims.ExpiresAt.Time, &claims.ID) nextAccessModel := newTokenModel(nextAccessClaims.ID, claims.UserID, TokenUseAccess, nextAccessClaims.ExpiresAt.Time, &claims.ID) - if err := s.tokens.RotateRefreshToken(ctx, claims.ID, nextRefreshModel, nextAccessModel); err != nil { - return nil, err + if rotateErr := s.tokens.RotateRefreshToken(ctx, claims.ID, nextRefreshModel, nextAccessModel); rotateErr != nil { + return nil, rotateErr }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 173 - 177, The err variable in the RotateRefreshToken call is shadowing an outer err; change the short-declaration (:=) to an assignment to the existing err so it reuses the earlier declaration. Specifically, after building nextRefreshModel and nextAccessModel with newTokenModel (using claims.ID and claims.UserID), call s.tokens.RotateRefreshToken(ctx, claims.ID, nextRefreshModel, nextAccessModel) and assign its result to the previously declared err (use = not :=) and handle the error as before to avoid shadowing.
70-131: Consider extracting helpers to reduce cognitive complexity.The linter flags cognitive complexity of 21 (threshold: 20). While only slightly over, extracting the validation and token creation logic into separate methods would improve readability.
Example extraction
func (s *service) validateTokenPairParams(userID string, scopes []string, accessTTL, refreshTTL time.Duration) error { if userID == "" { return fmt.Errorf("%w: user id is required", ErrInvalidParams) } if len(scopes) == 0 { return fmt.Errorf("%w: scopes are required", ErrInvalidParams) } if accessTTL < 0 || refreshTTL < 0 { return fmt.Errorf("%w: ttl must be non-negative", ErrInvalidParams) } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 70 - 131, The GenerateTokenPair method in service has high cognitive complexity; extract the validation and token generation/insertion logic into small helpers: add validateTokenPairParams(userID, scopes, accessTTL, refreshTTL) to return the parameter error checks, and add a helper like buildAndStoreToken(ctx, userID, scopes, use TokenUse, ttl time.Duration) (or createTokenWithInsert) that creates claims via s.newClaims, signs them with s.sign, and inserts the token via s.tokens.Insert (propagate errors). Replace the inline checks and repeated sign/insert blocks in GenerateTokenPair with calls to validateTokenPairParams and two calls to the build/store helper, then assemble TokenPairInfo from the returned token infos to reduce complexity.
253-255: Non-standard use of JWTaud(Audience) claim forparentJTI.Storing
parentJTIin theAudienceclaim is unconventional. Theaudclaim is intended to identify recipients of the token, not to store relational metadata.Consider using a custom claim field in the
Claimsstruct instead:Proposed change
In
jwt.go, add to Claims:ParentJTI *string `json:"parent_jti,omitempty"`Then in
newClaims:- if parentJTI != nil { - claims.RegisteredClaims.Audience = []string{*parentJTI} - } + claims.ParentJTI = parentJTI🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 253 - 255, The code is misusing RegisteredClaims.Audience to store parentJTI; update the Claims struct in jwt.go to add a custom field (e.g., ParentJTI *string `json:"parent_jti,omitempty"`) and change newClaims (where you currently set claims.RegisteredClaims.Audience = []string{*parentJTI}) to instead assign the pointer to claims.ParentJTI. Also update any code that reads parentJTI from RegisteredClaims.Audience to read from Claims.ParentJTI and keep json/tag naming consistent.
103-104: Format long line per linter.Proposed fix
- accessClaims := s.newClaims(userID, scopes, TokenUseAccess, now.Add(min(accessTTL, s.config.AccessTTL)), nil) - refreshClaims := s.newClaims(userID, []string{ScopeRefresh}, TokenUseRefresh, now.Add(min(refreshTTL, s.config.RefreshTTL)), nil) + accessClaims := s.newClaims( + userID, scopes, TokenUseAccess, + now.Add(min(accessTTL, s.config.AccessTTL)), nil, + ) + refreshClaims := s.newClaims( + userID, []string{ScopeRefresh}, TokenUseRefresh, + now.Add(min(refreshTTL, s.config.RefreshTTL)), nil, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 103 - 104, The two long assignment lines creating accessClaims and refreshClaims exceed the line-length linter; break the arguments across multiple lines when calling s.newClaims so each parameter is on its own (or grouped) line for readability—e.g., format the s.newClaims calls for accessClaims and refreshClaims by placing userID, scopes/[]string{ScopeRefresh}, TokenUseAccess/TokenUseRefresh, the now.Add(min(...)) expression, and nil on separate lines; ensure you keep the same identifiers (accessClaims, refreshClaims, s.newClaims, TokenUseAccess, TokenUseRefresh, ScopeRefresh, accessTTL, refreshTTL, s.config.AccessTTL, s.config.RefreshTTL) and maintain the same argument order and semantics.internal/sms-gateway/jwt/repository.go (2)
50-60: Consider returning a sentinel error instead of(nil, nil)for not-found.The linter flags returning both a nil error and nil value. This pattern can be confusing for callers. Consider using a sentinel error like
ErrTokenNotFoundor returninggorm.ErrRecordNotFounddirectly.Additionally, the exported method returns an unexported type
*tokenModel. If this method is only used internally, consider making it unexported (getByID), or export the model type.Option A: Return sentinel error
+var ErrTokenNotFound = errors.New("token not found") + func (r *Repository) GetByID(ctx context.Context, jti string) (*tokenModel, error) { var token tokenModel if err := r.db.WithContext(ctx).Where("id = ?", jti).First(&token).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { - return nil, nil + return nil, ErrTokenNotFound } return nil, fmt.Errorf("can't get token by id: %w", err) } return &token, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/repository.go` around lines 50 - 60, Repository.GetByID currently returns (nil, nil) when a record isn't found and exposes an unexported tokenModel from an exported method; change it to return a sentinel error (e.g., declare ErrTokenNotFound) or return gorm.ErrRecordNotFound instead of nil error so callers can distinguish "not found", and update the method signature/visibility accordingly — either make GetByID unexported (getByID) if tokenModel stays unexported, or export tokenModel (TokenModel) if GetByID must remain exported; ensure the error wrap uses fmt.Errorf(...: %w) or returns the sentinel directly when errors.Is(err, gorm.ErrRecordNotFound).
62-109: Wrap the transaction error to satisfywrapcheckand add context.The linter flags that the error returned from
Transactionshould be wrapped to provide context for debugging.Proposed fix
func (r *Repository) RotateRefreshToken( ctx context.Context, currentJTI string, nextRefresh, nextAccess *tokenModel, ) error { - return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + err := r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { // ... transaction body unchanged ... }) + if err != nil { + return fmt.Errorf("rotate refresh token transaction failed: %w", err) + } + return nil }The rotation logic itself is well-structured:
- Row lock prevents concurrent rotation
- Validates not-revoked, not-replaced, correct token use, not expired
- Atomically marks old token and creates new pair
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/repository.go` around lines 62 - 109, The Transaction call in Repository.RotateRefreshToken is returned directly and must be wrapped for context; assign the result of r.db.WithContext(ctx).Transaction(...) to a variable (e.g., txErr) and if non-nil return fmt.Errorf("rotate refresh token: %w", txErr) so the outer error adds context for wrapcheck, keeping the internal transaction logic unchanged.internal/sms-gateway/jwt/disabled.go (1)
20-23: Format long function signature per linter.The linter reports a formatting issue. Break the parameter list across multiple lines for readability.
Proposed fix
// GenerateTokenPair implements Service. -func (d *disabled) GenerateTokenPair(_ context.Context, _ string, _ []string, _, _ time.Duration) (*TokenPairInfo, error) { +func (d *disabled) GenerateTokenPair( + _ context.Context, + _ string, + _ []string, + _, _ time.Duration, +) (*TokenPairInfo, error) { return nil, ErrDisabled }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/disabled.go` around lines 20 - 23, The GenerateTokenPair method on type disabled currently has a long single-line signature; reformat the function signature to comply with the linter by breaking the parameter list across multiple lines (one parameter per line or grouped logically), keeping the same parameter names and types and the same return values, and preserving the body that returns nil, ErrDisabled; update the signature for func (d *disabled) GenerateTokenPair(...) to span multiple lines so it is more readable and linter-friendly.internal/sms-gateway/handlers/thirdparty/auth.go (1)
160-162: Format long line per linter.Break the multi-condition case into separate lines or individual cases for readability.
Proposed fix
- case errors.Is(err, jwt.ErrInvalidToken), errors.Is(err, jwt.ErrTokenRevoked), errors.Is(err, jwt.ErrInvalidTokenUse), errors.Is(err, jwt.ErrTokenReplay): - return fiber.ErrUnauthorized + case errors.Is(err, jwt.ErrInvalidToken), + errors.Is(err, jwt.ErrTokenRevoked), + errors.Is(err, jwt.ErrInvalidTokenUse), + errors.Is(err, jwt.ErrTokenReplay): + return fiber.ErrUnauthorized🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/handlers/thirdparty/auth.go` around lines 160 - 162, The switch case that currently groups multiple jwt error checks in one line should be split into separate case lines for each condition to satisfy the linter; replace the single combined case using errors.Is(err, jwt.ErrInvalidToken), errors.Is(err, jwt.ErrTokenRevoked), errors.Is(err, jwt.ErrInvalidTokenUse), errors.Is(err, jwt.ErrTokenReplay) with individual case entries that each return fiber.ErrUnauthorized (refer to the switch handling of err in auth.go and the jwt error symbols jwt.ErrInvalidToken, jwt.ErrTokenRevoked, jwt.ErrInvalidTokenUse, jwt.ErrTokenReplay).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/sms-gateway/handlers/thirdparty/auth.go`:
- Line 54: Replace the scope requirement on the refresh endpoint so it uses the
dedicated refresh scope instead of the manage scope: in the router registration
that calls router.Post("/token/refresh",
permissions.RequireScope(ScopeTokensManage), h.postRefreshToken), change the
required permission to permissions.RequireScope(ScopeTokensRefresh) so the
middleware accepts refresh tokens; update the invocation referencing
ScopeTokensManage to ScopeTokensRefresh next to h.postRefreshToken.
In `@internal/sms-gateway/jwt/jwt.go`:
- Around line 15-19: The Service interface method signatures are too long and
fail golines; reformat each signature in the Service interface (GenerateToken,
GenerateTokenPair, RefreshTokenPair, ParseToken) by breaking parameter lists and
return types across lines (e.g., place ctx and first parameter on the first
line, remaining params each on their own line and the return type on its own
line) so each line stays within gofmt/golines limits; update GenerateToken,
GenerateTokenPair (accessTTL, refreshTTL), RefreshTokenPair, and ParseToken
declarations accordingly to satisfy the linter without changing types or
semantics.
In `@internal/sms-gateway/jwt/service.go`:
- Around line 160-163: Refresh loses original API scopes because
RefreshTokenPair builds the new access token from the refresh token's claims
(which only contains []string{ScopeRefresh}); fix by preserving original scopes
in the refresh token and using them when creating the new access token: update
GenerateTokenPair to embed the original scopes into the refresh token's claims
(e.g., add a field like OriginalScopes or reuse claims.Scopes when calling
newClaims for the refresh token), then in RefreshTokenPair read those preserved
scopes from the refresh claims, filter out ScopeRefresh, and pass that filtered
slice into newClaims when creating nextAccessClaims (keep using TokenUseAccess
and the existing TTLs).
---
Nitpick comments:
In `@internal/sms-gateway/handlers/thirdparty/auth.go`:
- Around line 160-162: The switch case that currently groups multiple jwt error
checks in one line should be split into separate case lines for each condition
to satisfy the linter; replace the single combined case using errors.Is(err,
jwt.ErrInvalidToken), errors.Is(err, jwt.ErrTokenRevoked), errors.Is(err,
jwt.ErrInvalidTokenUse), errors.Is(err, jwt.ErrTokenReplay) with individual case
entries that each return fiber.ErrUnauthorized (refer to the switch handling of
err in auth.go and the jwt error symbols jwt.ErrInvalidToken,
jwt.ErrTokenRevoked, jwt.ErrInvalidTokenUse, jwt.ErrTokenReplay).
In `@internal/sms-gateway/jwt/config_test.go`:
- Line 1: Change the test package from "jwt" to the external package "jwt_test"
in config_test.go so tests exercise only the public API; update all references
to the type/values to use the exported package qualifier (e.g., replace Config
with jwt.Config and any other unqualified symbols), and add an import for the
jwt package if not already present.
- Around line 8-45: Add additional subtests to TestConfigValidate that call
Config.Validate to cover the missing/invalid-field branches: include cases for
empty Secret, Secret shorter than 32 bytes, AccessTTL zero and negative,
RefreshTTL zero and negative, and missing Issuer (if Validate checks it). For
each case (referencing the Config struct fields Secret, AccessTTL, RefreshTTL,
Issuer and the method Validate), set wantErr=true and assert an error is
returned; keep the existing valid case expecting no error. Ensure each case has
a descriptive name so test output pinpoints the failing validation branch.
In `@internal/sms-gateway/jwt/disabled.go`:
- Around line 20-23: The GenerateTokenPair method on type disabled currently has
a long single-line signature; reformat the function signature to comply with the
linter by breaking the parameter list across multiple lines (one parameter per
line or grouped logically), keeping the same parameter names and types and the
same return values, and preserving the body that returns nil, ErrDisabled;
update the signature for func (d *disabled) GenerateTokenPair(...) to span
multiple lines so it is more readable and linter-friendly.
In `@internal/sms-gateway/jwt/repository.go`:
- Around line 50-60: Repository.GetByID currently returns (nil, nil) when a
record isn't found and exposes an unexported tokenModel from an exported method;
change it to return a sentinel error (e.g., declare ErrTokenNotFound) or return
gorm.ErrRecordNotFound instead of nil error so callers can distinguish "not
found", and update the method signature/visibility accordingly — either make
GetByID unexported (getByID) if tokenModel stays unexported, or export
tokenModel (TokenModel) if GetByID must remain exported; ensure the error wrap
uses fmt.Errorf(...: %w) or returns the sentinel directly when errors.Is(err,
gorm.ErrRecordNotFound).
- Around line 62-109: The Transaction call in Repository.RotateRefreshToken is
returned directly and must be wrapped for context; assign the result of
r.db.WithContext(ctx).Transaction(...) to a variable (e.g., txErr) and if
non-nil return fmt.Errorf("rotate refresh token: %w", txErr) so the outer error
adds context for wrapcheck, keeping the internal transaction logic unchanged.
In `@internal/sms-gateway/jwt/service.go`:
- Around line 173-177: The err variable in the RotateRefreshToken call is
shadowing an outer err; change the short-declaration (:=) to an assignment to
the existing err so it reuses the earlier declaration. Specifically, after
building nextRefreshModel and nextAccessModel with newTokenModel (using
claims.ID and claims.UserID), call s.tokens.RotateRefreshToken(ctx, claims.ID,
nextRefreshModel, nextAccessModel) and assign its result to the previously
declared err (use = not :=) and handle the error as before to avoid shadowing.
- Around line 70-131: The GenerateTokenPair method in service has high cognitive
complexity; extract the validation and token generation/insertion logic into
small helpers: add validateTokenPairParams(userID, scopes, accessTTL,
refreshTTL) to return the parameter error checks, and add a helper like
buildAndStoreToken(ctx, userID, scopes, use TokenUse, ttl time.Duration) (or
createTokenWithInsert) that creates claims via s.newClaims, signs them with
s.sign, and inserts the token via s.tokens.Insert (propagate errors). Replace
the inline checks and repeated sign/insert blocks in GenerateTokenPair with
calls to validateTokenPairParams and two calls to the build/store helper, then
assemble TokenPairInfo from the returned token infos to reduce complexity.
- Around line 253-255: The code is misusing RegisteredClaims.Audience to store
parentJTI; update the Claims struct in jwt.go to add a custom field (e.g.,
ParentJTI *string `json:"parent_jti,omitempty"`) and change newClaims (where you
currently set claims.RegisteredClaims.Audience = []string{*parentJTI}) to
instead assign the pointer to claims.ParentJTI. Also update any code that reads
parentJTI from RegisteredClaims.Audience to read from Claims.ParentJTI and keep
json/tag naming consistent.
- Around line 103-104: The two long assignment lines creating accessClaims and
refreshClaims exceed the line-length linter; break the arguments across multiple
lines when calling s.newClaims so each parameter is on its own (or grouped) line
for readability—e.g., format the s.newClaims calls for accessClaims and
refreshClaims by placing userID, scopes/[]string{ScopeRefresh},
TokenUseAccess/TokenUseRefresh, the now.Add(min(...)) expression, and nil on
separate lines; ensure you keep the same identifiers (accessClaims,
refreshClaims, s.newClaims, TokenUseAccess, TokenUseRefresh, ScopeRefresh,
accessTTL, refreshTTL, s.config.AccessTTL, s.config.RefreshTTL) and maintain the
same argument order and semantics.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
configs/config.example.ymldocs/jwt-refresh-token-plan.mdinternal/config/config.gointernal/config/module.gointernal/sms-gateway/handlers/middlewares/jwtauth/jwtauth.gointernal/sms-gateway/handlers/thirdparty/auth.gointernal/sms-gateway/handlers/thirdparty/permissions.gointernal/sms-gateway/jwt/config.gointernal/sms-gateway/jwt/config_test.gointernal/sms-gateway/jwt/disabled.gointernal/sms-gateway/jwt/errors.gointernal/sms-gateway/jwt/jwt.gointernal/sms-gateway/jwt/models.gointernal/sms-gateway/jwt/repository.gointernal/sms-gateway/jwt/service.gointernal/sms-gateway/models/migrations/mysql/20260301090000_tokens_refresh_support.sql
🤖 Pull request artifacts
|
capcom6
force-pushed
the
codex/plan-refresh-token-support-for-jwt-auth
branch
from
8e23ee5 to
d958c2b
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
internal/config/module.go (1)
138-141: Consider emitting a deprecation warning when falling back tojwt.ttl.The fallback is useful, but logging when it is used would make migration progress visible and prevent stale config from lingering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/module.go` around lines 138 - 141, When computing accessTTL currently by checking cfg.JWT.AccessTTL and falling back to cfg.JWT.TTL, add a deprecation/logging statement to notify when the fallback is used: detect when cfg.JWT.AccessTTL == 0 and before assigning accessTTL = cfg.JWT.TTL call your logger (e.g., processLogger or the existing logger in scope) to emit a deprecation warning mentioning "jwt.access_ttl is deprecated; using jwt.ttl as fallback" and include both values for visibility; ensure the message only logs when the fallback path (cfg.JWT.AccessTTL == 0) is taken so migration progress is visible.internal/sms-gateway/handlers/middlewares/permissions/permissions.go (1)
21-30: GuardHasScopeagainst nil options.
HasScopedereferencesopts.exactdirectly. Since the function is exported, a directnilcaller will panic.🛡️ Suggested defensive fix
func HasScope(c *fiber.Ctx, scope string, opts *options) bool { + if opts == nil { + opts = &options{} + } scopes, ok := c.Locals(localsScopes).([]string) if !ok { return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/handlers/middlewares/permissions/permissions.go` around lines 21 - 30, The HasScope function currently dereferences opts.exact without checking opts for nil, which can panic when called with a nil options pointer; update HasScope to first check if opts is nil (or default a local exact boolean) and treat nil as opts.exact == false, then use that safe boolean in the slices.ContainsFunc comparison so the function (and uses of localsScopes/ScopeAll) never dereferences a nil opts.api/requests.http (1)
223-228: Use the just-issued refresh token in this flow.This refresh call uses dotenv
{{refreshToken}}while revoke uses{{issueToken...id}}, so the sequence can target different token sessions and hide rotation issues during manual validation.🔧 Suggested update
POST {{baseUrl}}/3rdparty/v1/auth/token/refresh HTTP/1.1 -Authorization: Bearer {{refreshToken}} +Authorization: Bearer {{issueToken.response.body.$.refreshToken}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/requests.http` around lines 223 - 228, The refresh request is using the static {{refreshToken}} instead of the token returned by the prior issueToken call, so swap the hardcoded variable for the just-issued refresh token from the issueToken response and keep the same identifier for the revoke step; specifically, update the Authorization header in the POST /3rdparty/v1/auth/token/refresh to use the issueToken response value (e.g., {{issueToken.response.body.$.refresh_token}} or the appropriate field returned by issueToken), and ensure the subsequent `@jti` assignment and DELETE /3rdparty/v1/auth/token/{{jti}} use that same issued-token variable (currently referenced as issueToken.response.body.$.id) so both calls target the same session.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/sms-gateway/handlers/thirdparty/auth.go`:
- Around line 83-89: The build error comes from populating a non-existent
RefreshToken field on smsgateway.TokenResponse; update the response struct
literals in this file (the return statements that construct
smsgateway.TokenResponse) to remove the RefreshToken entry and only set the
valid fields: ID (use pair.Access.ID), TokenType ("Bearer"), AccessToken
(pair.Access.Token), and ExpiresAt (pair.Access.ExpiresAt); do the same for the
second occurrence referenced around the later return so no RefreshToken
references remain.
In `@internal/sms-gateway/jwt/jwt.go`:
- Line 33: Fix the JSON tag typo on the OriginalScopes field: change the struct
tag from `json:"orginal_scopes"` to `json:"original_scopes"` so tokens use the
correct claim key; update any code that marshals/unmarshals JWTs or relies on
the misspelled claim (search for OriginalScopes usage) and run tests/regen any
fixtures to ensure consumers accept the corrected claim name before release.
In `@internal/sms-gateway/jwt/models.go`:
- Around line 20-26: newTokenModel currently only maps ID and ExpiresAt so
persisted rows lack token usage and rotation lineage; update the constructor
newTokenModel to also map the TokenInfo fields that represent token use (e.g.,
TokenInfo.Type or TokenInfo.Use → tokenModel.Type/Use or IsRefresh) and rotation
metadata (e.g., TokenInfo.ParentID/RotatedFrom/RotationID →
tokenModel.ParentID/RotatedFrom) and any timestamp like IssuedAt/CreatedAt so
tokenModel can enforce and observe refresh-token rotation and access vs refresh
semantics.
In `@internal/sms-gateway/jwt/repository.go`:
- Around line 50-54: In RotateRefreshToken, add upfront nil checks for the input
pointers nextRefresh and nextAccess before starting the DB transaction: validate
that nextRefresh and nextAccess are non-nil and return a clear validation error
if either is nil so the function fails fast and avoids downstream DB create
errors; update the beginning of Repository.RotateRefreshToken to perform these
checks and return early with an appropriate error if validation fails.
- Around line 57-79: The rotation transaction currently locks/updates by id only
and doesn't ensure the row is a refresh token nor record the replacement JTI;
change the SELECT lock to include token type (e.g., add Where("id = ? AND
token_type = ?", currentJTI, "refresh") in tx.Clauses(...).First(¤t)) and
make the UPDATE also match token_type (e.g., Where("id = ? AND token_type = ?",
current.ID, "refresh")) and set both "revoked_at" and a persisted replacement
field (e.g., "replaced_by" or "replacement_jti") to the new token JTI; also
add/ensure the tokenModel has the ReplacedBy/ReplacementJTI field so the DB
enforces refresh-token semantics and records linkage for audit/replay
protection.
In `@internal/sms-gateway/jwt/service.go`:
- Around line 113-119: The two separate inserts using s.tokens.Insert with
newTokenModel(userID, tokenInfo.Access) and newTokenModel(userID,
tokenInfo.Refresh) can leave a partial commit if the second insert fails; wrap
these operations in an atomic transaction (use the tokens repository transaction
API like Begin/Commit/Rollback or a provided WithTx/context-transaction helper)
so both inserts happen under one transaction, or alternatively perform the
refresh insert first then the access insert and roll back (delete) the first if
the second fails; update the method performing these calls to use the
transaction and ensure Commit on success and Rollback on any insert error so no
orphaned token rows remain.
- Line 233: The revoke TODO indicates only access tokens (by JTI) are revoked
while refresh tokens remain valid; update the revoke flow in service.go so that
RevokeToken (or the existing revoke method) also locates and invalidates
associated refresh credentials: lookup refresh tokens by parent JTI/session ID
via the TokenRepository/RefreshTokenRepo (or add methods like
FindRefreshByParentJTI and InvalidateRefreshTokensByParentJTI), mark them
revoked/expired in storage, and persist the change so refresh can't be used to
continue the session; add/adjust unit tests for RevokeToken to assert both
access and refresh tokens become invalid and handle repository errors.
- Around line 157-161: The call to parsedClaims.Scopes[0] can panic when
parsedClaims.Scopes is empty; update the code path that calls generatePair (the
section using parsedClaims.UserID, parsedClaims.OriginalScopes,
parsedClaims.Scopes[0], s.config.AccessTTL) to first validate that
len(parsedClaims.Scopes) > 0 and if not return ErrInvalidToken (or the service's
token-invalid error) so the function fails closed instead of panicking.
- Around line 144-146: The parseErr branch currently returns a wrapped parse
error which prevents errors.Is from matching jwt.ErrInvalidToken; change the
error return to consistently return the sentinel ErrInvalidToken (or
jwt.ErrInvalidToken) instead of fmt.Errorf("failed to parse token: %w",
parseErr) in the token-parsing function where parseErr is checked so handlers
can detect invalid tokens via errors.Is(err, ErrInvalidToken).
---
Nitpick comments:
In `@api/requests.http`:
- Around line 223-228: The refresh request is using the static {{refreshToken}}
instead of the token returned by the prior issueToken call, so swap the
hardcoded variable for the just-issued refresh token from the issueToken
response and keep the same identifier for the revoke step; specifically, update
the Authorization header in the POST /3rdparty/v1/auth/token/refresh to use the
issueToken response value (e.g., {{issueToken.response.body.$.refresh_token}} or
the appropriate field returned by issueToken), and ensure the subsequent `@jti`
assignment and DELETE /3rdparty/v1/auth/token/{{jti}} use that same issued-token
variable (currently referenced as issueToken.response.body.$.id) so both calls
target the same session.
In `@internal/config/module.go`:
- Around line 138-141: When computing accessTTL currently by checking
cfg.JWT.AccessTTL and falling back to cfg.JWT.TTL, add a deprecation/logging
statement to notify when the fallback is used: detect when cfg.JWT.AccessTTL ==
0 and before assigning accessTTL = cfg.JWT.TTL call your logger (e.g.,
processLogger or the existing logger in scope) to emit a deprecation warning
mentioning "jwt.access_ttl is deprecated; using jwt.ttl as fallback" and include
both values for visibility; ensure the message only logs when the fallback path
(cfg.JWT.AccessTTL == 0) is taken so migration progress is visible.
In `@internal/sms-gateway/handlers/middlewares/permissions/permissions.go`:
- Around line 21-30: The HasScope function currently dereferences opts.exact
without checking opts for nil, which can panic when called with a nil options
pointer; update HasScope to first check if opts is nil (or default a local exact
boolean) and treat nil as opts.exact == false, then use that safe boolean in the
slices.ContainsFunc comparison so the function (and uses of
localsScopes/ScopeAll) never dereferences a nil opts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
api/requests.httpconfigs/config.example.ymlinternal/config/config.gointernal/config/module.gointernal/sms-gateway/handlers/middlewares/jwtauth/jwtauth.gointernal/sms-gateway/handlers/middlewares/permissions/options.gointernal/sms-gateway/handlers/middlewares/permissions/permissions.gointernal/sms-gateway/handlers/thirdparty/auth.gointernal/sms-gateway/handlers/thirdparty/permissions.gointernal/sms-gateway/jwt/config.gointernal/sms-gateway/jwt/config_test.gointernal/sms-gateway/jwt/disabled.gointernal/sms-gateway/jwt/errors.gointernal/sms-gateway/jwt/jwt.gointernal/sms-gateway/jwt/models.gointernal/sms-gateway/jwt/repository.gointernal/sms-gateway/jwt/service.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/sms-gateway/handlers/thirdparty/permissions.go
- internal/sms-gateway/jwt/config_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/sms-gateway/jwt/repository.go (1)
81-84:⚠️ Potential issue | 🟠 MajorPersist replacement metadata during rotation update.
Lines 81-84 only set
revoked_at; the rotate path should also persist replacement linkage/usage timestamp for replay auditability.🧩 Suggested update
- if err := tx.Model((*tokenModel)(nil)).Where("id = ?", current.ID).Updates(map[string]any{ - "revoked_at": gorm.Expr("NOW()"), + if err := tx.Model((*tokenModel)(nil)). + Where("id = ? AND token_use = ?", current.ID, refreshToken). + Updates(map[string]any{ + "revoked_at": gorm.Expr("NOW()"), + "replaced_by_jti": nextRefresh.ID, + "last_used_at": gorm.Expr("NOW()"), }).Error; err != nil { return fmt.Errorf("can't mark refresh token as replaced: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/repository.go` around lines 81 - 84, The update only sets revoked_at; include persistent replacement linkage and timestamp in the same rotation update by extending the Updates map in the tx.Model((*tokenModel)(nil)).Where("id = ?", current.ID).Updates(...) call to also set the replacement token ID and a replaced_at/replacement_used_at timestamp (e.g. "replacement_id": replacement.ID and "replaced_at": gorm.Expr("NOW()")) so the tokenModel row records which token replaced it and when for replay auditability.
🧹 Nitpick comments (2)
internal/sms-gateway/jwt/config.go (1)
43-45: Consider validatingOptions.RefreshScopeto fail fast on misconfiguration.Line 44 allows an empty refresh scope; validating it early avoids hard-to-debug runtime auth behavior.
♻️ Proposed hardening
import ( "fmt" + "strings" "time" ) @@ type Options struct { RefreshScope string } + +func (o Options) Validate() error { + if strings.TrimSpace(o.RefreshScope) == "" { + return fmt.Errorf("%w: refresh scope is required", ErrInvalidConfig) + } + return nil +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/config.go` around lines 43 - 45, Options.RefreshScope is currently allowed to be empty which can cause silent auth failures; add validation to fail fast by implementing a validation step (e.g., func (o *Options) Validate() error or a constructor NewOptions(...)) that checks RefreshScope != "" and returns a descriptive error if empty, and call this validator from wherever JWT config/options are constructed (e.g., the JWT config constructor or initializer) so misconfiguration surfaces at startup.internal/sms-gateway/jwt/service.go (1)
57-96: Consider passingnowintonewClaimsto ensure consistent timestamps.
generatePaircapturesnow := time.Now()at line 78 for expiration calculation, butnewClaims(line 243) callstime.Now()again internally forIssuedAt. Under load, this could cause minor clock drift between the base time for expiration and the recorded issuance time.♻️ Suggested fix: pass timestamp to helpers
-func (s *service) newClaims(userID string, scopes []string, expiresAt time.Time) *Claims { - now := time.Now() +func (s *service) newClaims(userID string, scopes []string, now time.Time, expiresAt time.Time) *Claims { claims := &Claims{ RegisteredClaims: jwt.RegisteredClaims{ ID: s.idFactory(),Then update callers:
- accessClaims := s.newClaims(userID, scopes, now.Add(min(accessTTL, s.config.AccessTTL))) - refreshClaims := s.newRefreshClaims(userID, s.options.RefreshScope, scopes, now.Add(s.config.RefreshTTL)) + accessClaims := s.newClaims(userID, scopes, now, now.Add(min(accessTTL, s.config.AccessTTL))) + refreshClaims := s.newRefreshClaims(userID, s.options.RefreshScope, scopes, now, now.Add(s.config.RefreshTTL))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 57 - 96, The generatePair function captures now := time.Now() but newClaims/newRefreshClaims call time.Now() internally, causing inconsistent IssuedAt vs ExpiresAt; modify newClaims and newRefreshClaims to accept a time.Time parameter (e.g., issuedAt) and use that instead of calling time.Now(), then update generatePair to pass now into s.newClaims(...) and s.newRefreshClaims(...), and update any other callers of newClaims/newRefreshClaims to supply a timestamp or use time.Now() at the call site to preserve behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@internal/sms-gateway/models/migrations/mysql/20260303021154_refresh_tokens.sql`:
- Line 8: Remove the stray standalone `---` line in the migration file
20260303021154_refresh_tokens.sql (it is not valid SQL or a Goose directive) so
the migration will run; if you intended a Goose directive use the proper format
like `-- +goose Down`/`-- +goose Up`, otherwise delete the `---` line entirely
to avoid MySQL parsing errors.
---
Duplicate comments:
In `@internal/sms-gateway/jwt/repository.go`:
- Around line 81-84: The update only sets revoked_at; include persistent
replacement linkage and timestamp in the same rotation update by extending the
Updates map in the tx.Model((*tokenModel)(nil)).Where("id = ?",
current.ID).Updates(...) call to also set the replacement token ID and a
replaced_at/replacement_used_at timestamp (e.g. "replacement_id": replacement.ID
and "replaced_at": gorm.Expr("NOW()")) so the tokenModel row records which token
replaced it and when for replay auditability.
---
Nitpick comments:
In `@internal/sms-gateway/jwt/config.go`:
- Around line 43-45: Options.RefreshScope is currently allowed to be empty which
can cause silent auth failures; add validation to fail fast by implementing a
validation step (e.g., func (o *Options) Validate() error or a constructor
NewOptions(...)) that checks RefreshScope != "" and returns a descriptive error
if empty, and call this validator from wherever JWT config/options are
constructed (e.g., the JWT config constructor or initializer) so
misconfiguration surfaces at startup.
In `@internal/sms-gateway/jwt/service.go`:
- Around line 57-96: The generatePair function captures now := time.Now() but
newClaims/newRefreshClaims call time.Now() internally, causing inconsistent
IssuedAt vs ExpiresAt; modify newClaims and newRefreshClaims to accept a
time.Time parameter (e.g., issuedAt) and use that instead of calling time.Now(),
then update generatePair to pass now into s.newClaims(...) and
s.newRefreshClaims(...), and update any other callers of
newClaims/newRefreshClaims to supply a timestamp or use time.Now() at the call
site to preserve behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
api/requests.httpgo.modinternal/sms-gateway/handlers/middlewares/permissions/options.gointernal/sms-gateway/handlers/middlewares/permissions/permissions.gointernal/sms-gateway/handlers/thirdparty/auth.gointernal/sms-gateway/handlers/thirdparty/module.gointernal/sms-gateway/jwt/config.gointernal/sms-gateway/jwt/disabled.gointernal/sms-gateway/jwt/jwt.gointernal/sms-gateway/jwt/models.gointernal/sms-gateway/jwt/module.gointernal/sms-gateway/jwt/repository.gointernal/sms-gateway/jwt/service.gointernal/sms-gateway/models/migrations/mysql/20260303021154_refresh_tokens.sql
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/sms-gateway/handlers/middlewares/permissions/options.go
- internal/sms-gateway/handlers/thirdparty/auth.go
- internal/sms-gateway/handlers/middlewares/permissions/permissions.go
internal/sms-gateway/models/migrations/mysql/20260303021154_refresh_tokens.sql
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/sms-gateway/jwt/service.go (1)
27-31:⚠️ Potential issue | 🟠 MajorValidate
optionsduring service initialization.
New(...)validatesconfigbut notoptions. An emptyRefreshScopecan slip through and create malformed refresh-token claims.🔧 Proposed fix
func New(config Config, options Options, tokens *Repository, metrics *Metrics) (Service, error) { if err := config.Validate(); err != nil { return nil, err } + if err := options.Validate(); err != nil { + return nil, err + } if tokens == nil { return nil, fmt.Errorf("%w: revoked storage is required", ErrInitFailed) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 27 - 31, New currently validates config but not the provided Options, allowing an empty RefreshScope to pass; update New to validate options (e.g., call an Options.Validate() method or explicitly check that options.RefreshScope is non-empty) and return an error if validation fails so malformed refresh-token claims cannot be created; reference the New function, the Options type and its RefreshScope field (or an Options.Validate helper) when adding the check before proceeding to create the Service.
♻️ Duplicate comments (2)
internal/sms-gateway/jwt/service.go (2)
192-194:⚠️ Potential issue | 🟠 MajorNormalize parse failures to
ErrInvalidTokeninParseToken.At Line 193, returning a generic wrapped parse error can bypass
errors.Is(err, ErrInvalidToken)handling and misclassify auth failures.🔧 Proposed fix
if parseErr != nil { - err = fmt.Errorf("failed to parse token: %w", parseErr) + err = fmt.Errorf("%w: %v", ErrInvalidToken, parseErr) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` around lines 192 - 194, In ParseToken, replace the current generic wrapped parse error handling so parse failures are normalized to ErrInvalidToken; when parseErr != nil set err to a wrapped ErrInvalidToken (e.g., fmt.Errorf("%w: %v", ErrInvalidToken, parseErr)) or assign ErrInvalidToken directly so that callers using errors.Is(err, ErrInvalidToken) will match; update the branch in ParseToken that currently sets err = fmt.Errorf("failed to parse token: %w", parseErr) to the normalized ErrInvalidToken form.
230-230:⚠️ Potential issue | 🟠 MajorRefresh-token revocation gap is still open.
Line 230 still leaves refresh credentials active after access-token revocation, which weakens revoke semantics.
If you want, I can draft a concrete follow-up issue with a minimal repo/query/update plan for parent/child refresh-chain invalidation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/jwt/service.go` at line 230, Access-token revocation currently does not touch refresh credentials (see the inline TODO "revoke refresh tokens too"), leaving refresh tokens usable after access revocation; find the code path where access tokens are revoked (e.g. the method that implements revoke/access-token behavior in service.go) and extend it to also revoke refresh tokens by calling or adding a function such as RevokeRefreshToken(s) / InvalidateRefreshTokenChain that: locates stored refresh token entries in the RefreshTokenStore or RefreshTokenRepository, marks them revoked (or deletes them), and recursively invalidates child/descendant refresh tokens or bumps the user's refresh token version field so an entire family is invalidated; ensure to log the action and add tests for parent/child refresh-chain invalidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/sms-gateway/jwt/service.go`:
- Around line 27-31: New currently validates config but not the provided
Options, allowing an empty RefreshScope to pass; update New to validate options
(e.g., call an Options.Validate() method or explicitly check that
options.RefreshScope is non-empty) and return an error if validation fails so
malformed refresh-token claims cannot be created; reference the New function,
the Options type and its RefreshScope field (or an Options.Validate helper) when
adding the check before proceeding to create the Service.
---
Duplicate comments:
In `@internal/sms-gateway/jwt/service.go`:
- Around line 192-194: In ParseToken, replace the current generic wrapped parse
error handling so parse failures are normalized to ErrInvalidToken; when
parseErr != nil set err to a wrapped ErrInvalidToken (e.g., fmt.Errorf("%w: %v",
ErrInvalidToken, parseErr)) or assign ErrInvalidToken directly so that callers
using errors.Is(err, ErrInvalidToken) will match; update the branch in
ParseToken that currently sets err = fmt.Errorf("failed to parse token: %w",
parseErr) to the normalized ErrInvalidToken form.
- Line 230: Access-token revocation currently does not touch refresh credentials
(see the inline TODO "revoke refresh tokens too"), leaving refresh tokens usable
after access revocation; find the code path where access tokens are revoked
(e.g. the method that implements revoke/access-token behavior in service.go) and
extend it to also revoke refresh tokens by calling or adding a function such as
RevokeRefreshToken(s) / InvalidateRefreshTokenChain that: locates stored refresh
token entries in the RefreshTokenStore or RefreshTokenRepository, marks them
revoked (or deletes them), and recursively invalidates child/descendant refresh
tokens or bumps the user's refresh token version field so an entire family is
invalidated; ensure to log the action and add tests for parent/child
refresh-chain invalidation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
jwt.access_ttlandjwt.refresh_ttland wire them throughinternal/configintojwt.Configwith validation thatrefresh_ttl > access_ttland positive values.TokenUseinClaims,TokenPairInfo, new service methodsGenerateTokenPairandRefreshTokenPair, and helpersnewClaims,sign, andparseTokenininternal/sms-gateway/jwt/service.go.tokensmodel withtoken_use,parent_jti,replaced_by_jti, andlast_used_at, add repository operationsGetByIDandRotateRefreshTokenand a DB migration SQL file to update the schema.POST /3rdparty/v1/auth/token/refresh, addtokens:refreshscope constant, update JWT middleware to reject non-access tokens, extend error set with refresh-specific errors, and implement disabled-service stubs for the new methods.Testing
internal/sms-gateway/jwt/config_test.goto validate config rules and rango test ./internal/sms-gateway/jwt, which passed.Codex Task
Summary by CodeRabbit
New Features
Authentication
API
Permissions
Database