From 408e36ebcad8357178828fe52787166702a149c7 Mon Sep 17 00:00:00 2001 From: arekkas Date: Tue, 29 May 2018 08:59:31 +0200 Subject: [PATCH] oauth2: Resolves various issues related to revokation This patch properly tracks access and refresh tokens across requests and thus resolves several issues related to broken token revokation: * oauth2: Revoke previous and future access tokens when revoking a token - closes #884 * oauth2: Revoke access and refresh tokens when authorization code is used twice - closes #693 * oauth2: Revoke tokens when performing refreshing grant - closes #889 --- Gopkg.lock | 18 ++++++------- Gopkg.toml | 10 ++++++- oauth2/fosite_store_memory.go | 30 ++++++++++++++++----- oauth2/fosite_store_sql.go | 41 +++++++++++++++++++++++++++-- oauth2/fosite_store_test.go | 9 +------ oauth2/fosite_store_test_helpers.go | 17 +++++++----- 6 files changed, 92 insertions(+), 33 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index f4dc9959156..7bd2fda5ace 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -55,8 +55,8 @@ [[projects]] name = "github.com/dgrijalva/jwt-go" packages = ["."] - revision = "dbeaa9332f19a944acb5736b4456cfcc02140e29" - version = "v3.1.0" + revision = "06ea1031745cb8b3dab3f6a236daf2b0aa468b7e" + version = "v3.2.0" [[projects]] name = "github.com/docker/go-connections" @@ -292,11 +292,10 @@ "token/hmac", "token/jwt" ] - revision = "608bf5fff7f5f7fc0dde0b3aecd03534974ba982" - version = "v0.19.8" + revision = "72bff7f33ee8c3a4a8806cc266ca7299ff1785d4" + version = "v0.20.0" [[projects]] - branch = "master" name = "github.com/ory/go-convenience" packages = [ "mapx", @@ -304,7 +303,8 @@ "stringsx", "urlx" ] - revision = "cf3c3570c7b941a7180cdc036c8b06eebd95dcb1" + revision = "42cb17c3e4dc0d7d7672cfaffbdfe8f5deb494db" + version = "v0.0.2" [[projects]] name = "github.com/ory/graceful" @@ -336,13 +336,13 @@ version = "v0.0.1" [[projects]] - branch = "master" name = "github.com/ory/sqlcon" packages = [ ".", "dockertest" ] - revision = "2e99584773f66ac91fcf23af73d90a28df2709c9" + revision = "08e1c6762dd59c776a735acc134889f757574f66" + version = "v0.0.2" [[projects]] name = "github.com/pborman/uuid" @@ -622,6 +622,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "a532f2a7dec7d15d20b623c2d181f9c6bc6313aea2b39082552793a48ede5040" + inputs-digest = "3e6a4e67e6aa3e745f1a72ec9ec452b17cd26ace9a03f747dd261fd94f5f639e" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index a47df195dea..b3c6b5510cb 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -29,6 +29,10 @@ name = "github.com/go-sql-driver/mysql" version = "1.3.0" +[[constraint]] + name = "github.com/ory/sqlcon" + version = "0.0.2" + [[constraint]] name = "github.com/gorilla/context" version = "1.1.0" @@ -71,7 +75,7 @@ [[constraint]] name = "github.com/ory/fosite" - version = "0.19.8" + version = "0.20.0" [[constraint]] name = "github.com/ory/graceful" @@ -133,6 +137,10 @@ name = "github.com/urfave/negroni" version = "0.2.0" +[[constraint]] + name = "github.com/ory/go-convenience" + version = "0.0.2" + [[constraint]] branch = "master" name = "golang.org/x/oauth2" diff --git a/oauth2/fosite_store_memory.go b/oauth2/fosite_store_memory.go index 1e935147464..3c283cd9c7b 100644 --- a/oauth2/fosite_store_memory.go +++ b/oauth2/fosite_store_memory.go @@ -32,9 +32,10 @@ import ( func NewFositeMemoryStore(m client.Manager, ls time.Duration) *FositeMemoryStore { return &FositeMemoryStore{ - AuthorizeCodes: make(map[string]fosite.Requester), + AuthorizeCodes: make(map[string]authorizeCode), IDSessions: make(map[string]fosite.Requester), AccessTokens: make(map[string]fosite.Requester), + PKCES: make(map[string]fosite.Requester), RefreshTokens: make(map[string]fosite.Requester), AccessTokenLifespan: ls, Manager: m, @@ -44,7 +45,7 @@ func NewFositeMemoryStore(m client.Manager, ls time.Duration) *FositeMemoryStore type FositeMemoryStore struct { client.Manager - AuthorizeCodes map[string]fosite.Requester + AuthorizeCodes map[string]authorizeCode IDSessions map[string]fosite.Requester AccessTokens map[string]fosite.Requester RefreshTokens map[string]fosite.Requester @@ -54,6 +55,11 @@ type FositeMemoryStore struct { sync.RWMutex } +type authorizeCode struct { + active bool + fosite.Requester +} + func (s *FositeMemoryStore) CreateOpenIDConnectSession(_ context.Context, authorizeCode string, requester fosite.Requester) error { s.Lock() defer s.Unlock() @@ -81,24 +87,36 @@ func (s *FositeMemoryStore) DeleteOpenIDConnectSession(_ context.Context, author func (s *FositeMemoryStore) CreateAuthorizeCodeSession(_ context.Context, code string, req fosite.Requester) error { s.Lock() defer s.Unlock() - s.AuthorizeCodes[code] = req + s.AuthorizeCodes[code] = authorizeCode{active: true, Requester: req} return nil } func (s *FositeMemoryStore) GetAuthorizeCodeSession(_ context.Context, code string, _ fosite.Session) (fosite.Requester, error) { s.RLock() defer s.RUnlock() + rel, ok := s.AuthorizeCodes[code] if !ok { return nil, errors.Wrap(fosite.ErrNotFound, "") } - return rel, nil + + if !rel.active { + return rel.Requester, errors.WithStack(fosite.ErrInvalidatedAuthorizeCode) + } + + return rel.Requester, nil } -func (s *FositeMemoryStore) DeleteAuthorizeCodeSession(_ context.Context, code string) error { +func (s *FositeMemoryStore) InvalidateAuthorizeCodeSession(ctx context.Context, code string) error { s.Lock() defer s.Unlock() - delete(s.AuthorizeCodes, code) + + rel, ok := s.AuthorizeCodes[code] + if !ok { + return fosite.ErrNotFound + } + rel.active = false + s.AuthorizeCodes[code] = rel return nil } diff --git a/oauth2/fosite_store_sql.go b/oauth2/fosite_store_sql.go index f8961526ee4..0a2f95a74c5 100644 --- a/oauth2/fosite_store_sql.go +++ b/oauth2/fosite_store_sql.go @@ -81,6 +81,7 @@ func sqlSchemaUp(table string, id string) string { session_data text NOT NULL, subject varchar(255) NOT NULL )`, + "4": fmt.Sprintf("ALTER TABLE hydra_oauth2_%s ADD active BOOL NOT NULL DEFAULT TRUE", table), } return schemas[id] @@ -91,6 +92,7 @@ func sqlSchemaDown(table string, id string) string { "1": fmt.Sprintf(`DROP TABLE %s)`, table), "2": fmt.Sprintf("ALTER TABLE hydra_oauth2_%s DROP COLUMN subject", table), "3": "DROP TABLE hydra_oauth2_pkce", + "4": fmt.Sprintf("ALTER TABLE hydra_oauth2_%s DROP COLUMN active", table), } return schemas[id] @@ -145,6 +147,23 @@ var migrations = &migrate.MemoryMigrationSource{ sqlSchemaDown(sqlTablePKCE, "3"), }, }, + { + Id: "4", + Up: []string{ + sqlSchemaUp(sqlTableAccess, "4"), + sqlSchemaUp(sqlTableRefresh, "4"), + sqlSchemaUp(sqlTableCode, "4"), + sqlSchemaUp(sqlTableOpenID, "4"), + sqlSchemaUp(sqlTablePKCE, "4"), + }, + Down: []string{ + sqlSchemaDown(sqlTableAccess, "4"), + sqlSchemaDown(sqlTableRefresh, "4"), + sqlSchemaDown(sqlTableCode, "4"), + sqlSchemaDown(sqlTableOpenID, "4"), + sqlSchemaDown(sqlTablePKCE, "4"), + }, + }, }, } @@ -158,6 +177,7 @@ var sqlParams = []string{ "form_data", "session_data", "subject", + "active", } type sqlData struct { @@ -169,6 +189,7 @@ type sqlData struct { GrantedScopes string `db:"granted_scope"` Form string `db:"form_data"` Subject string `db:"subject"` + Active bool `db:"active"` Session []byte `db:"session_data"` } @@ -195,6 +216,7 @@ func sqlSchemaFromRequest(signature string, r fosite.Requester, logger logrus.Fi Form: r.GetRequestForm().Encode(), Session: session, Subject: subject, + Active: true, }, nil } @@ -254,6 +276,14 @@ func (s *FositeSQLStore) findSessionBySignature(signature string, session fosite return nil, errors.Wrap(fosite.ErrNotFound, "") } else if err != nil { return nil, errors.WithStack(err) + } else if !d.Active && table == sqlTableCode { + if r, err := d.toRequest(session, s.Manager, s.L); err != nil { + return nil, err + } else { + return r, errors.WithStack(fosite.ErrInvalidatedAuthorizeCode) + } + } else if !d.Active { + return nil, errors.WithStack(fosite.ErrInactiveToken) } return d.toRequest(session, s.Manager, s.L) @@ -295,8 +325,15 @@ func (s *FositeSQLStore) GetAuthorizeCodeSession(_ context.Context, signature st return s.findSessionBySignature(signature, session, sqlTableCode) } -func (s *FositeSQLStore) DeleteAuthorizeCodeSession(_ context.Context, signature string) error { - return s.deleteSession(signature, sqlTableCode) +func (s *FositeSQLStore) InvalidateAuthorizeCodeSession(ctx context.Context, signature string) error { + if _, err := s.DB.Exec(s.DB.Rebind(fmt.Sprintf( + "UPDATE hydra_oauth2_%s SET active=false WHERE signature=?", + sqlTableCode, + )), signature); err != nil { + return errors.WithStack(err) + } + + return nil } func (s *FositeSQLStore) CreateAccessTokenSession(_ context.Context, signature string, requester fosite.Requester) error { diff --git a/oauth2/fosite_store_test.go b/oauth2/fosite_store_test.go index 76b081c3c17..03e4425bb2f 100644 --- a/oauth2/fosite_store_test.go +++ b/oauth2/fosite_store_test.go @@ -46,14 +46,7 @@ var clientManager = &client.MemoryManager{ var databases = make(map[string]*sqlx.DB) func init() { - fositeStores["memory"] = &FositeMemoryStore{ - AuthorizeCodes: make(map[string]fosite.Requester), - IDSessions: make(map[string]fosite.Requester), - AccessTokens: make(map[string]fosite.Requester), - RefreshTokens: make(map[string]fosite.Requester), - AccessTokenLifespan: time.Hour, - PKCES: make(map[string]fosite.Requester), - } + fositeStores["memory"] = NewFositeMemoryStore(nil, time.Hour) } func TestMain(m *testing.M) { diff --git a/oauth2/fosite_store_test_helpers.go b/oauth2/fosite_store_test_helpers.go index 4562f645eca..09b74b69681 100644 --- a/oauth2/fosite_store_test_helpers.go +++ b/oauth2/fosite_store_test_helpers.go @@ -116,21 +116,24 @@ func TestHelperRevokeRefreshToken(m pkg.FositeStorer) func(t *testing.T) { func TestHelperCreateGetDeleteAuthorizeCodes(m pkg.FositeStorer) func(t *testing.T) { return func(t *testing.T) { ctx := context.Background() - _, err := m.GetAuthorizeCodeSession(ctx, "4321", &fosite.DefaultSession{}) - assert.NotNil(t, err) + res, err := m.GetAuthorizeCodeSession(ctx, "4321", &fosite.DefaultSession{}) + assert.Error(t, err) + assert.Nil(t, res) err = m.CreateAuthorizeCodeSession(ctx, "4321", &defaultRequest) require.NoError(t, err) - res, err := m.GetAuthorizeCodeSession(ctx, "4321", &fosite.DefaultSession{}) + res, err = m.GetAuthorizeCodeSession(ctx, "4321", &fosite.DefaultSession{}) require.NoError(t, err) AssertObjectKeysEqual(t, &defaultRequest, res, "Scopes", "GrantedScopes", "Form", "Session") - err = m.DeleteAuthorizeCodeSession(ctx, "4321") + err = m.InvalidateAuthorizeCodeSession(ctx, "4321") require.NoError(t, err) - _, err = m.GetAuthorizeCodeSession(ctx, "4321", &fosite.DefaultSession{}) - assert.NotNil(t, err) + res, err = m.GetAuthorizeCodeSession(ctx, "4321", &fosite.DefaultSession{}) + require.Error(t, err) + assert.EqualError(t, err, fosite.ErrInvalidatedAuthorizeCode.Error()) + assert.NotNil(t, res) } } @@ -227,7 +230,7 @@ func TestHelperFlushTokens(m pkg.FositeStorer, lifespan time.Duration) func(t *t _, err = m.GetAccessTokenSession(ctx, "flush-3", ds) require.NoError(t, err) - require.NoError(t, m.FlushInactiveAccessTokens(ctx, time.Now().Add(-(lifespan+time.Hour/2)))) + require.NoError(t, m.FlushInactiveAccessTokens(ctx, time.Now().Add(-(lifespan + time.Hour/2)))) _, err = m.GetAccessTokenSession(ctx, "flush-1", ds) require.NoError(t, err) _, err = m.GetAccessTokenSession(ctx, "flush-2", ds)