From 8e0e489a67a4e86c7173f494d7de7d07cbbe01b6 Mon Sep 17 00:00:00 2001 From: Marc Alvarez Date: Thu, 12 Jun 2025 15:52:06 -0600 Subject: [PATCH] fix: clarify refresh token parameter names and improve code readability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename misleading parameters in Storage interface: - refreshTokenID → refreshToken - newRefreshTokenID → newRefreshToken - Make bare returns explicit in token.go for better readability - Add documentation to clarify token creation flow The interface parameters were misleadingly named with 'ID' suffix when they actually contain the full token values. The example implementations already used the semantically correct names, creating inconsistency. This change aligns the interface with its implementations and prevents confusion. --- pkg/op/storage.go | 10 ++++++++-- pkg/op/token.go | 22 +++++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/pkg/op/storage.go b/pkg/op/storage.go index 35d7040..5c64989 100644 --- a/pkg/op/storage.go +++ b/pkg/op/storage.go @@ -41,8 +41,14 @@ type AuthStorage interface { // registered the refresh_token grant type in advance // // * TokenExchangeRequest as returned by ValidateTokenExchangeRequest - CreateAccessAndRefreshTokens(ctx context.Context, request TokenRequest, currentRefreshToken string) (accessTokenID string, newRefreshTokenID string, expiration time.Time, err error) - TokenRequestByRefreshToken(ctx context.Context, refreshTokenID string) (RefreshTokenRequest, error) + // + // CreateAccessAndRefreshToken creates both access and refresh tokens. + // The returned refresh token is the actual token value that will be passed + // directly to the client. The storage implementation is responsible for + // creating the complete refresh token (JWT or opaque format). For refresh tokens, + // in either format, the token itself serves as both the identifier and the credential. + CreateAccessAndRefreshTokens(ctx context.Context, request TokenRequest, currentRefreshToken string) (accessTokenID string, newRefreshToken string, expiration time.Time, err error) + TokenRequestByRefreshToken(ctx context.Context, refreshToken string) (RefreshTokenRequest, error) TerminateSession(ctx context.Context, userID string, clientID string) error diff --git a/pkg/op/token.go b/pkg/op/token.go index 1df9cc2..5d7217c 100644 --- a/pkg/op/token.go +++ b/pkg/op/token.go @@ -69,6 +69,13 @@ func CreateTokenResponse(ctx context.Context, request IDTokenRequest, client Cli }, nil } +// createTokens delegates token creation to the appropriate storage method based on +// the request type and requirements. It returns an access token ID and expiration +// in all cases, but the refresh token handling varies: +// - When needsRefreshToken() returns true: calls CreateAccessAndRefreshTokens, +// which returns both tokens. The newRefreshToken will contain the actual token value. +// - When needsRefreshToken() returns false: calls CreateAccessToken only. +// The newRefreshToken will be an empty string in this case. func createTokens(ctx context.Context, tokenRequest TokenRequest, storage Storage, refreshToken string, client AccessTokenClient) (id, newRefreshToken string, exp time.Time, err error) { ctx, span := tracer.Start(ctx, "createTokens") defer span.End() @@ -77,7 +84,7 @@ func createTokens(ctx context.Context, tokenRequest TokenRequest, storage Storag return storage.CreateAccessAndRefreshTokens(ctx, tokenRequest, refreshToken) } id, exp, err = storage.CreateAccessToken(ctx, tokenRequest) - return + return id, "", exp, err } func needsRefreshToken(tokenRequest TokenRequest, client AccessTokenClient) bool { @@ -95,6 +102,15 @@ func needsRefreshToken(tokenRequest TokenRequest, client AccessTokenClient) bool } } +// CreateAccessToken creates an access token and may return a refresh token from storage. +// This function always creates the access token using the ID returned from storage. +// The refresh token is obtained from the storage layer and passed through unchanged. +// Whether a refresh token is included depends on the request: +// - Authorization code flow with offline_access scope: returns refresh token +// - Refresh token grant (rotation): returns new refresh token +// - Client credentials, implicit flow: returns empty string +// +// The function returns both tokens to support all flows with a single signature. func CreateAccessToken(ctx context.Context, tokenRequest TokenRequest, accessTokenType AccessTokenType, creator TokenCreator, client AccessTokenClient, refreshToken string) (accessToken, newRefreshToken string, validity time.Duration, err error) { ctx, span := tracer.Start(ctx, "CreateAccessToken") defer span.End() @@ -110,12 +126,12 @@ func CreateAccessToken(ctx context.Context, tokenRequest TokenRequest, accessTok validity = exp.Add(clockSkew).Sub(time.Now().UTC()) if accessTokenType == AccessTokenTypeJWT { accessToken, err = CreateJWT(ctx, IssuerFromContext(ctx), tokenRequest, exp, id, client, creator.Storage()) - return + return accessToken, newRefreshToken, validity, err } _, span = tracer.Start(ctx, "CreateBearerToken") accessToken, err = CreateBearerToken(id, tokenRequest.GetSubject(), creator.Crypto()) span.End() - return + return accessToken, newRefreshToken, validity, err } func CreateBearerToken(tokenID, subject string, crypto Crypto) (string, error) {