Skip to content

Commit

Permalink
add refresh_token grace period
Browse files Browse the repository at this point in the history
add migration for used on refresh token

add refresh token grace period

replace migration with ory dev CLI cmd

create migrations with command:
ory dev pop migration create persistence/sql/migrations/ add_refresh_token_used_flag

use existing GetRefreshtTokenSession

make FositeStorer include oauth2.TokenRevocationStorage

WIP mabye grace period tests

use test run instead of named functions

add documentation for example config

add grace period to internal config

prettier --write

Update persistence/sql/persister_oauth2.go

Co-authored-by: hackerman <[email protected]>

move refresh token rotation to proper parent

remove unneeded file

make encryption of session more obvious

rename used to in_grace_period

when deactivating a refresh token, in_grace_period should be false

add testing the refresh token store when grace period is configured

use reflection to control configuring the persister during tests

cchore: format

fix linting errors

fix: add max lifetime

fix: move migration to latest

remove reflection
  • Loading branch information
bill-robbins-ss authored and StarAurryon committed May 25, 2024
1 parent af0c64f commit b55a602
Show file tree
Hide file tree
Showing 9 changed files with 199 additions and 8 deletions.
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
14 changes: 14 additions & 0 deletions internal/config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,20 @@ 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 @@ func TestHelperRunner(t *testing.T, store InternalRegistry, k string) {
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 testHelperRevokeAccessToken(x InternalRegistry) func(t *testing.T) {
}
}

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

0 comments on commit b55a602

Please sign in to comment.