mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: ensure OAuth2 refresh tokens outlive access tokens (#19769)
This commit is contained in:
Generated
+4
@@ -16424,6 +16424,10 @@ const docTemplate = `{
|
||||
},
|
||||
"max_token_lifetime": {
|
||||
"type": "integer"
|
||||
},
|
||||
"refresh_default_duration": {
|
||||
"description": "RefreshDefaultDuration is the default lifetime for OAuth2 refresh tokens.\nThis should generally be longer than access token lifetimes to allow\nrefreshing after access token expiry.",
|
||||
"type": "integer"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
Generated
+4
@@ -14945,6 +14945,10 @@
|
||||
},
|
||||
"max_token_lifetime": {
|
||||
"type": "integer"
|
||||
},
|
||||
"refresh_default_duration": {
|
||||
"description": "RefreshDefaultDuration is the default lifetime for OAuth2 refresh tokens.\nThis should generally be longer than access token lifetimes to allow\nrefreshing after access token expiry.",
|
||||
"type": "integer"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbauthz"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtime"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider"
|
||||
@@ -27,6 +28,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
"github.com/coder/serpent"
|
||||
)
|
||||
|
||||
func TestOAuth2ProviderApps(t *testing.T) {
|
||||
@@ -1184,6 +1186,71 @@ func TestOAuth2ProviderCrossResourceAudienceValidation(t *testing.T) {
|
||||
// For now, this verifies the basic token flow works correctly
|
||||
}
|
||||
|
||||
// TestOAuth2RefreshExpiryOutlivesAccess verifies that refresh token expiry is
|
||||
// greater than the provisioned access token (API key) expiry per configuration.
|
||||
func TestOAuth2RefreshExpiryOutlivesAccess(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Set explicit lifetimes to make comparison deterministic.
|
||||
db, pubsub := dbtestutil.NewDB(t)
|
||||
dv := coderdtest.DeploymentValues(t, func(d *codersdk.DeploymentValues) {
|
||||
d.Sessions.DefaultDuration = serpent.Duration(1 * time.Hour)
|
||||
d.Sessions.RefreshDefaultDuration = serpent.Duration(48 * time.Hour)
|
||||
})
|
||||
ownerClient := coderdtest.New(t, &coderdtest.Options{
|
||||
Database: db,
|
||||
Pubsub: pubsub,
|
||||
DeploymentValues: dv,
|
||||
})
|
||||
_ = coderdtest.CreateFirstUser(t, ownerClient)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// Create app and secret
|
||||
// Keep suffix short to satisfy name validation (<=32 chars, alnum + hyphens).
|
||||
apps := generateApps(ctx, t, ownerClient, "ref-exp")
|
||||
//nolint:gocritic // Owner permission required for app secret creation
|
||||
secret, err := ownerClient.PostOAuth2ProviderAppSecret(ctx, apps.Default.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg := &oauth2.Config{
|
||||
ClientID: apps.Default.ID.String(),
|
||||
ClientSecret: secret.ClientSecretFull,
|
||||
Endpoint: oauth2.Endpoint{
|
||||
AuthURL: apps.Default.Endpoints.Authorization,
|
||||
DeviceAuthURL: apps.Default.Endpoints.DeviceAuth,
|
||||
TokenURL: apps.Default.Endpoints.Token,
|
||||
AuthStyle: oauth2.AuthStyleInParams,
|
||||
},
|
||||
RedirectURL: apps.Default.CallbackURL,
|
||||
Scopes: []string{},
|
||||
}
|
||||
|
||||
// Authorization and token exchange
|
||||
code, err := authorizationFlow(ctx, ownerClient, cfg)
|
||||
require.NoError(t, err)
|
||||
tok, err := cfg.Exchange(ctx, code)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, tok.AccessToken)
|
||||
require.NotEmpty(t, tok.RefreshToken)
|
||||
|
||||
// Parse refresh token prefix (coder_<prefix>_<secret>)
|
||||
parts := strings.Split(tok.RefreshToken, "_")
|
||||
require.Len(t, parts, 3)
|
||||
prefix := parts[1]
|
||||
|
||||
// Look up refresh token row and associated API key
|
||||
dbToken, err := db.GetOAuth2ProviderAppTokenByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(prefix))
|
||||
require.NoError(t, err)
|
||||
apiKey, err := db.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), dbToken.APIKeyID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Assert refresh token expiry is strictly after access token expiry
|
||||
require.Truef(t, dbToken.ExpiresAt.After(apiKey.ExpiresAt),
|
||||
"expected refresh expiry %s to be after access expiry %s",
|
||||
dbToken.ExpiresAt, apiKey.ExpiresAt,
|
||||
)
|
||||
}
|
||||
|
||||
// customTokenExchange performs a custom OAuth2 token exchange with support for resource parameter
|
||||
// This is needed because golang.org/x/oauth2 doesn't support custom parameters in token requests
|
||||
func customTokenExchange(ctx context.Context, baseURL, clientID, clientSecret, code, redirectURI, resource string) (*oauth2.Token, error) {
|
||||
|
||||
@@ -92,9 +92,8 @@ func extractTokenParams(r *http.Request, callbackURL *url.URL) (tokenParams, []c
|
||||
}
|
||||
|
||||
// Tokens
|
||||
// TODO: the sessions lifetime config passed is for coder api tokens.
|
||||
// Should there be a separate config for oauth2 tokens? They are related,
|
||||
// but they are not the same.
|
||||
// Uses Sessions.DefaultDuration for access token (API key) TTL and
|
||||
// Sessions.RefreshDefaultDuration for refresh token TTL.
|
||||
func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerFunc {
|
||||
return func(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
@@ -280,6 +279,13 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
|
||||
}
|
||||
|
||||
// Do the actual token exchange in the database.
|
||||
// Determine refresh token expiry independently from the access token.
|
||||
refreshLifetime := lifetimes.RefreshDefaultDuration.Value()
|
||||
if refreshLifetime == 0 {
|
||||
refreshLifetime = lifetimes.DefaultDuration.Value()
|
||||
}
|
||||
refreshExpiresAt := dbtime.Now().Add(refreshLifetime)
|
||||
|
||||
err = db.InTx(func(tx database.Store) error {
|
||||
ctx := dbauthz.As(ctx, actor)
|
||||
err = tx.DeleteOAuth2ProviderAppCodeByID(ctx, dbCode.ID)
|
||||
@@ -307,7 +313,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
|
||||
_, err = tx.InsertOAuth2ProviderAppToken(ctx, database.InsertOAuth2ProviderAppTokenParams{
|
||||
ID: uuid.New(),
|
||||
CreatedAt: dbtime.Now(),
|
||||
ExpiresAt: key.ExpiresAt,
|
||||
ExpiresAt: refreshExpiresAt,
|
||||
HashPrefix: []byte(refreshToken.Prefix),
|
||||
RefreshHash: []byte(refreshToken.Hashed),
|
||||
AppSecretID: dbSecret.ID,
|
||||
@@ -401,6 +407,13 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
|
||||
}
|
||||
|
||||
// Replace the token.
|
||||
// Determine refresh token expiry independently from the access token.
|
||||
refreshLifetime := lifetimes.RefreshDefaultDuration.Value()
|
||||
if refreshLifetime == 0 {
|
||||
refreshLifetime = lifetimes.DefaultDuration.Value()
|
||||
}
|
||||
refreshExpiresAt := dbtime.Now().Add(refreshLifetime)
|
||||
|
||||
err = db.InTx(func(tx database.Store) error {
|
||||
ctx := dbauthz.As(ctx, actor)
|
||||
err = tx.DeleteAPIKeyByID(ctx, prevKey.ID) // This cascades to the token.
|
||||
@@ -416,7 +429,7 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
|
||||
_, err = tx.InsertOAuth2ProviderAppToken(ctx, database.InsertOAuth2ProviderAppTokenParams{
|
||||
ID: uuid.New(),
|
||||
CreatedAt: dbtime.Now(),
|
||||
ExpiresAt: key.ExpiresAt,
|
||||
ExpiresAt: refreshExpiresAt,
|
||||
HashPrefix: []byte(refreshToken.Prefix),
|
||||
RefreshHash: []byte(refreshToken.Hashed),
|
||||
AppSecretID: dbToken.AppSecretID,
|
||||
|
||||
Reference in New Issue
Block a user