Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Refresh token expiration window #3770

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions driver/config/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ const (
KeyExcludeNotBeforeClaim = "oauth2.exclude_not_before_claim"
KeyAllowedTopLevelClaims = "oauth2.allowed_top_level_claims"
KeyMirrorTopLevelClaims = "oauth2.mirror_top_level_claims"
KeyRefreshTokenRotationGracePeriod = "oauth2.refresh_token_rotation.grace_period" // #nosec G101
KeyOAuth2GrantJWTIDOptional = "oauth2.grant.jwt.jti_optional"
KeyOAuth2GrantJWTIssuedDateOptional = "oauth2.grant.jwt.iat_optional"
KeyOAuth2GrantJWTMaxDuration = "oauth2.grant.jwt.max_ttl"
Expand Down Expand Up @@ -649,3 +650,11 @@ func (p *DefaultProvider) cookieSuffix(ctx context.Context, key string) string {

return p.getProvider(ctx).String(key) + suffix
}

func (p *DefaultProvider) RefreshTokenRotationGracePeriod() time.Duration {
var duration = p.p.DurationF(KeyRefreshTokenRotationGracePeriod, 0)
if duration > time.Hour {
return time.Hour
}
return p.p.DurationF(KeyRefreshTokenRotationGracePeriod, 0)
}
7 changes: 7 additions & 0 deletions driver/config/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,13 @@ func TestViperProviderValidates(t *testing.T) {
assert.Equal(t, "random_salt", c.SubjectIdentifierAlgorithmSalt(ctx))
assert.Equal(t, []string{"whatever"}, c.DefaultClientScope(ctx))

// refresh
assert.Equal(t, time.Duration(0), c.RefreshTokenRotationGracePeriod())
require.NoError(t, c.Set(ctx, KeyRefreshTokenRotationGracePeriod, "1s"))
assert.Equal(t, time.Second, c.RefreshTokenRotationGracePeriod())
require.NoError(t, c.Set(ctx, KeyRefreshTokenRotationGracePeriod, "2h"))
assert.Equal(t, time.Hour, c.RefreshTokenRotationGracePeriod())

// urls
assert.Equal(t, urlx.ParseOrPanic("https://issuer"), c.IssuerURL(ctx))
assert.Equal(t, urlx.ParseOrPanic("https://public/"), c.PublicURL(ctx))
Expand Down
13 changes: 13 additions & 0 deletions internal/config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,19 @@ oauth2:
session:
# store encrypted data in database, default true
encrypt_at_rest: true
## refresh_token_rotation
# By default Refresh Tokens are rotated and invalidated with each use. See https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.13.2 for more details
refresh_token_rotation:
#
## grace_period
#
# Set the grace period for a refresh token to allow it to be used for the duration of this configuration after its
# first use. New refresh tokens will continue to be issued.
#
# Examples:
# - 5s
# - 1m
grace_period: 0s

# The secrets section configures secrets used for encryption and signing of several systems. All secrets can be rotated,
# for more information on this topic navigate to:
Expand Down
58 changes: 58 additions & 0 deletions oauth2/fosite_store_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@
t.Run(fmt.Sprintf("case=testHelperDeleteAccessTokens/db=%s", k), testHelperDeleteAccessTokens(store))
t.Run(fmt.Sprintf("case=testHelperRevokeAccessToken/db=%s", k), testHelperRevokeAccessToken(store))
t.Run(fmt.Sprintf("case=testFositeJWTBearerGrantStorage/db=%s", k), testFositeJWTBearerGrantStorage(store))
t.Run(fmt.Sprintf("case=testHelperRevokeRefreshTokenMaybeGracePeriod/db=%s", k), testHelperRevokeRefreshTokenMaybeGracePeriod(store))
}

func testHelperRequestIDMultiples(m InternalRegistry, _ string) func(t *testing.T) {
Expand Down Expand Up @@ -552,6 +553,63 @@
}
}

func testHelperRevokeRefreshTokenMaybeGracePeriod(x InternalRegistry) func(t *testing.T) {

return func(t *testing.T) {
t.Run("Revokes refresh token when grace period not configured", func(t *testing.T) {
// SETUP
m := x.OAuth2Storage()
ctx := context.Background()

refreshTokenSession := fmt.Sprintf("refresh_token_%d", time.Now().Unix())
err := m.CreateRefreshTokenSession(ctx, refreshTokenSession, &defaultRequest)
assert.NoError(t, err, "precondition failed: could not create refresh token session")

// ACT
err = m.RevokeRefreshTokenMaybeGracePeriod(ctx, defaultRequest.GetID(), refreshTokenSession)
assert.NoError(t, err)

tmpSession := new(fosite.Session)
_, err = m.GetRefreshTokenSession(ctx, refreshTokenSession, *tmpSession)

// ASSERT
// a revoked refresh token returns an error when getting the token again
assert.Error(t, err)
assert.True(t, errors.Is(err, fosite.ErrInactiveToken))
})

t.Run("refresh token enters grace period when configured,", func(t *testing.T) {
ctx := context.Background()

// SETUP
x.Config().Set(ctx, "oauth2.refresh_token_rotation.grace_period", "1m")

Check failure on line 585 in oauth2/fosite_store_helpers.go

View workflow job for this annotation

GitHub Actions / Run tests and lints

Error return value of `(*github.com/ory/hydra/v2/driver/config.DefaultProvider).Set` is not checked (errcheck)

// always reset back to the default
defer x.Config().Set(ctx, "oauth2.refresh_token_rotation.grace_period", "0m")

Check failure on line 588 in oauth2/fosite_store_helpers.go

View workflow job for this annotation

GitHub Actions / Run tests and lints

Error return value of `(*github.com/ory/hydra/v2/driver/config.DefaultProvider).Set` is not checked (errcheck)

m := x.OAuth2Storage()

refreshTokenSession := fmt.Sprintf("refresh_token_%d_with_grace_period", time.Now().Unix())
err := m.CreateRefreshTokenSession(ctx, refreshTokenSession, &defaultRequest)
assert.NoError(t, err, "precondition failed: could not create refresh token session")

// ACT
err = m.RevokeRefreshTokenMaybeGracePeriod(ctx, defaultRequest.GetID(), refreshTokenSession)

assert.NoError(t, err)

tmpSession := new(fosite.Session)
_, err = m.GetRefreshTokenSession(ctx, refreshTokenSession, *tmpSession)

// ASSERT
// when grace period is configured the refresh token can be obtained within
// the grace period without error
assert.NoError(t, err)
})
}

}

func testHelperCreateGetDeletePKCERequestSession(x InternalRegistry) func(t *testing.T) {
return func(t *testing.T) {
m := x.OAuth2Storage()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE hydra_oauth2_refresh DROP COLUMN in_grace_period;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE hydra_oauth2_refresh ADD in_grace_period bool DEFAULT false;
95 changes: 92 additions & 3 deletions persistence/sql/persister_oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"strings"
"time"

"github.com/gobuffalo/pop/v6"
"github.com/ory/hydra/v2/x"

"github.com/ory/x/sqlxx"
Expand Down Expand Up @@ -122,6 +123,24 @@ func (p *Persister) sqlSchemaFromRequest(ctx context.Context, signature string,
}, nil
}

func (p *Persister) marshalSession(ctx context.Context, session fosite.Session) ([]byte, error) {
sessionBytes, err := json.Marshal(session)
if err != nil {
return nil, err
}

if !p.config.EncryptSessionData(ctx) {
return sessionBytes, nil
}

ciphertext, err := p.r.KeyCipher().Encrypt(ctx, sessionBytes, nil)
if err != nil {
return nil, err
}

return []byte(ciphertext), nil
}

func (r *OAuth2RequestSQL) toRequest(ctx context.Context, session fosite.Session, p *Persister) (_ *fosite.Request, err error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.toRequest")
defer otelx.End(span, &err)
Expand Down Expand Up @@ -235,6 +254,30 @@ func (p *Persister) createSession(ctx context.Context, signature string, request
return nil
}

func (p *Persister) updateRefreshSession(ctx context.Context, requestId string, session fosite.Session, inGracePeriod bool) error {
_, ok := session.(*oauth2.Session)
if !ok && session != nil {
return errors.Errorf("expected session to be of type *oauth2.Session but got: %T", session)
}
sessionBytes, err := p.marshalSession(ctx, session)
if err != nil {
return err
}

updateSql := fmt.Sprintf("UPDATE %s SET session_data = ?, in_grace_period = ? WHERE request_id = ?",
OAuth2RequestSQL{Table: sqlTableRefresh}.TableName())

return p.Transaction(ctx, func(ctx context.Context, c *pop.Connection) error {
err := p.Connection(ctx).RawQuery(updateSql, sessionBytes, inGracePeriod, requestId).Exec()
if errors.Is(err, sql.ErrNoRows) {
return errorsx.WithStack(fosite.ErrNotFound)
} else if err != nil {
return sqlcon.HandleError(err)
}
return nil
})
}

func (p *Persister) findSessionBySignature(ctx context.Context, signature string, session fosite.Session, table tableName) (fosite.Requester, error) {
r := OAuth2RequestSQL{Table: table}
err := p.QueryWithNetwork(ctx).Where("signature = ?", signature).First(&r)
Expand Down Expand Up @@ -302,14 +345,31 @@ func (p *Persister) deactivateSessionByRequestID(ctx context.Context, id string,
return sqlcon.HandleError(
p.Connection(ctx).
RawQuery(
fmt.Sprintf("UPDATE %s SET active=false WHERE request_id=? AND nid = ? AND active=true", OAuth2RequestSQL{Table: table}.TableName()),
fmt.Sprintf("UPDATE %s SET active=false, in_grace_period=false WHERE request_id=? AND nid = ? AND active=true", OAuth2RequestSQL{Table: table}.TableName()),
id,
p.NetworkID(ctx),
).
Exec(),
)
}

func (p *Persister) getRefreshTokenGracePeriodStatusBySignature(ctx context.Context, signature string) (_ bool, err error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.getRefreshTokenGracePeriodStatusBySignature")
defer otelx.End(span, &err)

var inGracePeriod bool
return inGracePeriod, p.Transaction(ctx, func(ctx context.Context, c *pop.Connection) error {
query := fmt.Sprintf("SELECT in_grace_period FROM %s WHERE signature = ?", OAuth2RequestSQL{Table: sqlTableRefresh}.TableName())
err := p.Connection(ctx).RawQuery(query, signature).First(&inGracePeriod)
if errors.Is(err, sql.ErrNoRows) {
return errorsx.WithStack(fosite.ErrNotFound)
} else if err != nil {
return sqlcon.HandleError(err)
}
return err
})
}

func (p *Persister) CreateAuthorizeCodeSession(ctx context.Context, signature string, requester fosite.Requester) error {
return otelx.WithSpan(ctx, "persistence.sql.CreateAuthorizeCodeSession", func(ctx context.Context) error {
return p.createSession(ctx, signature, requester, sqlTableCode, requester.GetSession().GetExpiresAt(fosite.AuthorizeCode))
Expand Down Expand Up @@ -483,10 +543,39 @@ func (p *Persister) RevokeRefreshToken(ctx context.Context, id string) (err erro
return p.deactivateSessionByRequestID(ctx, id, sqlTableRefresh)
}

func (p *Persister) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, id string, _ string) (err error) {
func (p *Persister) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, id string, signature string) (err error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.RevokeRefreshTokenMaybeGracePeriod")
defer otelx.End(span, &err)
return p.deactivateSessionByRequestID(ctx, id, sqlTableRefresh)

gracePeriod := p.config.RefreshTokenRotationGracePeriod()
if gracePeriod <= 0 {
return p.deactivateSessionByRequestID(ctx, id, sqlTableRefresh)
}

var requester fosite.Requester
session := new(oauth2.Session)
if requester, err = p.GetRefreshTokenSession(ctx, signature, session); err != nil {
p.l.Errorf("signature: %s not found. grace period not applied", id)
return errors.WithStack(err)
}

var inGracePeriod bool
if inGracePeriod, err = p.getRefreshTokenGracePeriodStatusBySignature(ctx, signature); err != nil {
p.l.Errorf("signature: %s in_grace_period status not found. grace period not applied", id)
return errors.WithStack(err)
}

requesterSession := requester.GetSession()
if !inGracePeriod {
requesterSession.SetExpiresAt(fosite.RefreshToken, time.Now().UTC().Add(gracePeriod))
if err = p.updateRefreshSession(ctx, id, requesterSession, true); err != nil {
p.l.Errorf("failed to update session with signature: %s", id)
return errors.WithStack(err)
}
} else {
p.l.Tracef("request_id: %s is in the grace period", id)
}
return nil
}

func (p *Persister) RevokeAccessToken(ctx context.Context, id string) (err error) {
Expand Down
17 changes: 16 additions & 1 deletion spec/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -1121,9 +1121,24 @@
"$ref": "#/definitions/webhook_config"
}
]
},
"refresh_token_rotation": {
"type": "object",
"properties": {
"grace_period": {
"title": "Refresh Token Rotation Grace Period",
"description": "Configures how long a Refresh Token remains valid after it has been used. The maximum value is one hour.",
"default": "0h",
"allOf": [
{
"$ref": "#/definitions/duration"
}
]
}
}
}
},
}
},
"secrets": {
"type": "object",
"additionalProperties": false,
Expand Down
5 changes: 1 addition & 4 deletions x/fosite_storer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,13 @@ import (
type FositeStorer interface {
fosite.Storage
oauth2.CoreStorage
oauth2.TokenRevocationStorage
openid.OpenIDConnectRequestStorage
pkce.PKCERequestStorage
rfc7523.RFC7523KeyStorage
verifiable.NonceManager
oauth2.ResourceOwnerPasswordCredentialsGrantStorage

RevokeRefreshToken(ctx context.Context, requestID string) error

RevokeAccessToken(ctx context.Context, requestID string) error

// flush the access token requests from the database.
// no data will be deleted after the 'notAfter' timeframe.
FlushInactiveAccessTokens(ctx context.Context, notAfter time.Time, limit int, batchSize int) error
Expand Down
Loading