Skip to content

Commit

Permalink
oauth2: Resolves various issues related to revokation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arekkas committed May 29, 2018
1 parent 29b394e commit 408e36e
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 33 deletions.
18 changes: 9 additions & 9 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -71,7 +75,7 @@

[[constraint]]
name = "github.com/ory/fosite"
version = "0.19.8"
version = "0.20.0"

[[constraint]]
name = "github.com/ory/graceful"
Expand Down Expand Up @@ -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"
Expand Down
30 changes: 24 additions & 6 deletions oauth2/fosite_store_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down
41 changes: 39 additions & 2 deletions oauth2/fosite_store_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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"),
},
},
},
}

Expand All @@ -158,6 +177,7 @@ var sqlParams = []string{
"form_data",
"session_data",
"subject",
"active",
}

type sqlData struct {
Expand All @@ -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"`
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 1 addition & 8 deletions oauth2/fosite_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
17 changes: 10 additions & 7 deletions oauth2/fosite_store_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 408e36e

Please sign in to comment.