mirror of
https://github.com/bitechdev/ResolveSpec.git
synced 2026-08-28 03:52:35 +00:00
fix(security): DatabaseAuthenticator.RefreshToken surfaces rotated refresh token and expiry
RefreshToken() hardcoded LoginResponse{Token: userCtx.SessionID, ExpiresIn: 24h}
and silently discarded anything else resolvespec_refresh_token returned. An
implementation that issues its own independent, rotating refresh token (not
just reusing the session/access token as its own refresh token) has nowhere
else to put the new refresh token and real access-token expiry than
UserContext.Claims, since UserContext has no dedicated fields for either.
Now reads claims.refresh_token/claims.expires_in when present and surfaces
them into LoginResponse.RefreshToken/ExpiresIn. Implementations that don't
set these claims keep today's behavior unchanged (empty RefreshToken, 24h
ExpiresIn default) — purely additive, no breaking change.
This commit is contained in:
@@ -551,11 +551,27 @@ func (a *DatabaseAuthenticator) RefreshToken(ctx context.Context, refreshToken s
|
|||||||
return nil, fmt.Errorf("failed to parse user context: %w", err)
|
return nil, fmt.Errorf("failed to parse user context: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return &LoginResponse{
|
// A resolvespec_refresh_token implementation that issues its own rotating
|
||||||
|
// refresh token (independent of the access/session token) returns it
|
||||||
|
// under claims.refresh_token, since UserContext has no dedicated field
|
||||||
|
// for it. Surface that into LoginResponse.RefreshToken so callers don't
|
||||||
|
// need to reach into User.Claims themselves. claims.expires_in
|
||||||
|
// (seconds) similarly overrides the default access-token ExpiresIn when
|
||||||
|
// the procedure provides a real value. Implementations that don't set
|
||||||
|
// these claims keep today's behavior unchanged (empty RefreshToken,
|
||||||
|
// 24h ExpiresIn default).
|
||||||
|
resp := &LoginResponse{
|
||||||
Token: userCtx.SessionID, // New session token from stored procedure
|
Token: userCtx.SessionID, // New session token from stored procedure
|
||||||
User: &userCtx,
|
User: &userCtx,
|
||||||
ExpiresIn: int64(24 * time.Hour.Seconds()),
|
ExpiresIn: int64(24 * time.Hour.Seconds()),
|
||||||
}, nil
|
}
|
||||||
|
if refreshToken, ok := userCtx.Claims["refresh_token"].(string); ok && refreshToken != "" {
|
||||||
|
resp.RefreshToken = refreshToken
|
||||||
|
}
|
||||||
|
if expiresIn, ok := userCtx.Claims["expires_in"].(float64); ok && expiresIn > 0 {
|
||||||
|
resp.ExpiresIn = int64(expiresIn)
|
||||||
|
}
|
||||||
|
return resp, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// JWTAuthenticator provides JWT token-based authentication
|
// JWTAuthenticator provides JWT token-based authentication
|
||||||
|
|||||||
@@ -793,6 +793,49 @@ func TestDatabaseAuthenticatorRefreshToken(t *testing.T) {
|
|||||||
t.Errorf("unfulfilled expectations: %v", err)
|
t.Errorf("unfulfilled expectations: %v", err)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// A resolvespec_refresh_token implementation that rotates its own
|
||||||
|
// independent refresh token (not just reusing the session/access token)
|
||||||
|
// has nowhere else to put the new refresh token and real access-token
|
||||||
|
// expiry than under UserContext.Claims, since UserContext has no
|
||||||
|
// dedicated fields for either. RefreshToken must surface those claims
|
||||||
|
// keys into LoginResponse.RefreshToken/ExpiresIn rather than silently
|
||||||
|
// dropping them (see the "successful token refresh" case above, which
|
||||||
|
// covers an implementation that has no independent refresh token at all
|
||||||
|
// and gets the 24h default instead).
|
||||||
|
t.Run("surfaces rotated refresh token and expiry from claims", func(t *testing.T) {
|
||||||
|
refreshToken := "refresh-token-abc"
|
||||||
|
|
||||||
|
sessionRows := sqlmock.NewRows([]string{"p_success", "p_error", "p_user"}).
|
||||||
|
AddRow(true, nil, `{"user_id":1,"user_name":"testuser"}`)
|
||||||
|
mock.ExpectQuery(`SELECT p_success, p_error, p_user::text FROM resolvespec_session`).
|
||||||
|
WithArgs(refreshToken, "refresh").
|
||||||
|
WillReturnRows(sessionRows)
|
||||||
|
|
||||||
|
refreshRows := sqlmock.NewRows([]string{"p_success", "p_error", "p_user"}).
|
||||||
|
AddRow(true, nil, `{"user_id":1,"user_name":"testuser","session_id":"new-access-789","claims":{"refresh_token":"new-refresh-def","expires_in":900}}`)
|
||||||
|
mock.ExpectQuery(`SELECT p_success, p_error, p_user::text FROM resolvespec_refresh_token`).
|
||||||
|
WithArgs(refreshToken, sqlmock.AnyArg()).
|
||||||
|
WillReturnRows(refreshRows)
|
||||||
|
|
||||||
|
resp, err := auth.RefreshToken(ctx, refreshToken)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected no error, got %v", err)
|
||||||
|
}
|
||||||
|
if resp.Token != "new-access-789" {
|
||||||
|
t.Errorf("expected token new-access-789, got %s", resp.Token)
|
||||||
|
}
|
||||||
|
if resp.RefreshToken != "new-refresh-def" {
|
||||||
|
t.Errorf("expected rotated refresh token new-refresh-def, got %q", resp.RefreshToken)
|
||||||
|
}
|
||||||
|
if resp.ExpiresIn != 900 {
|
||||||
|
t.Errorf("expected ExpiresIn 900 from claims, got %d", resp.ExpiresIn)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := mock.ExpectationsWereMet(); err != nil {
|
||||||
|
t.Errorf("unfulfilled expectations: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestDatabaseAuthenticatorReconnectsClosedDBPaths(t *testing.T) {
|
func TestDatabaseAuthenticatorReconnectsClosedDBPaths(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user