From dab4940ace9996c2384078e4bd338f0de7538e7b Mon Sep 17 00:00:00 2001
From: Hein <hein.puth@gmail.com>
Date: Thu, 27 Aug 2026 21:50:57 +0200
Subject: [PATCH] fix(security): DatabaseAuthenticator.RefreshToken surfaces
 rotated refresh token and expiry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 pkg/security/providers.go      | 20 ++++++++++++++--
 pkg/security/providers_test.go | 43 ++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/pkg/security/providers.go b/pkg/security/providers.go
index b0c11bb..57266eb 100644
--- a/pkg/security/providers.go
+++ b/pkg/security/providers.go
@@ -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 &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
 		User:      &userCtx,
 		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
diff --git a/pkg/security/providers_test.go b/pkg/security/providers_test.go
index b44ccf3..685d097 100644
--- a/pkg/security/providers_test.go
+++ b/pkg/security/providers_test.go
@@ -793,6 +793,49 @@ func TestDatabaseAuthenticatorRefreshToken(t *testing.T) {
 			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) {