Skip to content

Commit

Permalink
feat: add client secret rotation support
Browse files Browse the repository at this point in the history
add new interface and implement it for
  • Loading branch information
rplnt committed Jun 9, 2021
1 parent 5d316ab commit b4baf9e
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 29 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ import "github.com/ory/fosite/compose"
import "github.com/ory/fosite/storage"

// This is the example storage that contains:
// * an OAuth2 Client with id "my-client" and secret "foobar" capable of all oauth2 and open id connect grant and response types.
// * an OAuth2 Client with id "my-client" and secrets "foobar" and "foobaz" capable of all oauth2 and open id connect grant and response types.
// * a User for the resource owner password credentials grant type with username "peter" and password "secret".
//
// You will most likely replace this with your own logic once you set up a real world application.
Expand Down
28 changes: 20 additions & 8 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ type Client interface {
GetAudience() Arguments
}

// ClientWithSecretRotation extends Client interface by a method providing a slice of rotated secrets.
type ClientWithSecretRotation interface {
Client
// GetRotatedHashes returns a slice of hashed secrets used for secrets rotation.
GetRotatedHashes() [][]byte
}

// OpenIDConnectClient represents a client capable of performing OpenID Connect requests.
type OpenIDConnectClient interface {
// GetRequestURIs is an array of request_uri values that are pre-registered by the RP for use at the OP. Servers MAY
Expand Down Expand Up @@ -88,14 +95,15 @@ type ResponseModeClient interface {

// DefaultClient is a simple default implementation of the Client interface.
type DefaultClient struct {
ID string `json:"id"`
Secret []byte `json:"client_secret,omitempty"`
RedirectURIs []string `json:"redirect_uris"`
GrantTypes []string `json:"grant_types"`
ResponseTypes []string `json:"response_types"`
Scopes []string `json:"scopes"`
Audience []string `json:"audience"`
Public bool `json:"public"`
ID string `json:"id"`
Secret []byte `json:"client_secret,omitempty"`
RotatedSecrets [][]byte `json:"rotated_secrets,omitempty"`
RedirectURIs []string `json:"redirect_uris"`
GrantTypes []string `json:"grant_types"`
ResponseTypes []string `json:"response_types"`
Scopes []string `json:"scopes"`
Audience []string `json:"audience"`
Public bool `json:"public"`
}

type DefaultOpenIDConnectClient struct {
Expand Down Expand Up @@ -133,6 +141,10 @@ func (c *DefaultClient) GetHashedSecret() []byte {
return c.Secret
}

func (c *DefaultClient) GetRotatedHashes() [][]byte {
return c.RotatedSecrets
}

func (c *DefaultClient) GetScopes() Arguments {
return c.Scopes
}
Expand Down
22 changes: 21 additions & 1 deletion client_authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,33 @@ func (f *Fosite) DefaultClientAuthenticationStrategy(ctx context.Context, r *htt
}

// Enforce client authentication
if err := f.Hasher.Compare(ctx, client.GetHashedSecret(), []byte(clientSecret)); err != nil {
if err := f.checkClientSecret(ctx, client, []byte(clientSecret)); err != nil {
return nil, errorsx.WithStack(ErrInvalidClient.WithWrap(err).WithDebug(err.Error()))
}

return client, nil
}

func (f *Fosite) checkClientSecret(ctx context.Context, client Client, clientSecret []byte) error {
var err error
err = f.Hasher.Compare(ctx, client.GetHashedSecret(), clientSecret)
if err == nil {
return nil
}
cc, ok := client.(ClientWithSecretRotation)
if !ok {
return err
}
for _, hash := range cc.GetRotatedHashes() {
err = f.Hasher.Compare(ctx, hash, clientSecret)
if err == nil {
return nil
}
}

return err
}

func findPublicKey(t *jwt.Token, set *jose.JSONWebKeySet, expectsRSAKey bool) (interface{}, error) {
keys := set.Keys
if len(keys) == 0 {
Expand Down
47 changes: 47 additions & 0 deletions client_authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,12 @@ func TestAuthenticateClient(t *testing.T) {
},
{
d: "should pass because client is confidential and id and secret match in post body",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: []byte("invalid_hash"), RotatedSecrets: [][]byte{barSecret}}, TokenEndpointAuthMethod: "client_secret_post"},
form: url.Values{"client_id": []string{"foo"}, "client_secret": []string{"bar"}},
r: new(http.Request),
},
{
d: "should pass because client is confidential and id and rotated secret match in post body",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: barSecret}, TokenEndpointAuthMethod: "client_secret_post"},
form: url.Values{"client_id": []string{"foo"}, "client_secret": []string{"bar"}},
r: new(http.Request),
Expand All @@ -207,6 +213,18 @@ func TestAuthenticateClient(t *testing.T) {
form: url.Values{},
r: &http.Request{Header: clientBasicAuthHeader("foo", "bar")},
},
{
d: "should pass because client is confidential and id and rotated secret match in header",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: []byte("invalid_hash"), RotatedSecrets: [][]byte{barSecret}}, TokenEndpointAuthMethod: "client_secret_basic"},
form: url.Values{},
r: &http.Request{Header: clientBasicAuthHeader("foo", "bar")},
},
{
d: "should pass because client is confidential and id and rotated secret match in header",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: []byte("invalid_hash"), RotatedSecrets: [][]byte{[]byte("invalid"), barSecret}}, TokenEndpointAuthMethod: "client_secret_basic"},
form: url.Values{},
r: &http.Request{Header: clientBasicAuthHeader("foo", "bar")},
},
{
d: "should fail because auth method is not client_secret_basic",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: barSecret}, TokenEndpointAuthMethod: "client_secret_post"},
Expand All @@ -221,6 +239,13 @@ func TestAuthenticateClient(t *testing.T) {
r: &http.Request{Header: clientBasicAuthHeader("foo", "baz")},
expectErr: ErrInvalidClient,
},
{
d: "should fail because client is confidential and neither secret nor rotated does match in header",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: barSecret, RotatedSecrets: [][]byte{barSecret}}, TokenEndpointAuthMethod: "client_secret_basic"},
form: url.Values{},
r: &http.Request{Header: clientBasicAuthHeader("foo", "baz")},
expectErr: ErrInvalidClient,
},
{
d: "should fail because client id is not encoded using application/x-www-form-urlencoded",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: barSecret}, TokenEndpointAuthMethod: "client_secret_basic"},
Expand Down Expand Up @@ -512,6 +537,28 @@ func TestAuthenticateClient(t *testing.T) {
}
}

// func TestCheckClientSecret(t *testing.T) {
// hasher := &BCrypt{WorkFactor: 6}
// f := &Fosite{
// JWKSFetcherStrategy: NewDefaultJWKSFetcherStrategy(),
// Store: storage.NewMemoryStore(),
// Hasher: hasher,
// TokenURL: "token-url",
// }

// for k, tc := range []struct {
// d string
// client *DefaultOpenIDConnectClient
// secret []byte
// err error
// }{} {
// t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
// err := f.checkClientSecret(context.Background(), tc.client, tc.secret)
// assert.Equal(t, tc.err, err)
// })
// }
// }

func TestAuthenticateClientTwice(t *testing.T) {
const at = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"

Expand Down
16 changes: 10 additions & 6 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,19 @@ import (

func TestDefaultClient(t *testing.T) {
sc := &DefaultClient{
ID: "1",
Secret: []byte("foobar-"),
RedirectURIs: []string{"foo", "bar"},
ResponseTypes: []string{"foo", "bar"},
GrantTypes: []string{"foo", "bar"},
Scopes: []string{"fooscope"},
ID: "1",
Secret: []byte("foobar-"),
RotatedSecrets: [][]byte{[]byte("foobar-1"), []byte("foobar-2")},
RedirectURIs: []string{"foo", "bar"},
ResponseTypes: []string{"foo", "bar"},
GrantTypes: []string{"foo", "bar"},
Scopes: []string{"fooscope"},
}

assert.Equal(t, sc.ID, sc.GetID())
assert.Equal(t, sc.RedirectURIs, sc.GetRedirectURIs())
assert.Equal(t, sc.Secret, sc.GetHashedSecret())
assert.Equal(t, sc.RotatedSecrets, sc.GetRotatedHashes())
assert.EqualValues(t, sc.ResponseTypes, sc.GetResponseTypes())
assert.EqualValues(t, sc.GrantTypes, sc.GetGrantTypes())
assert.EqualValues(t, sc.Scopes, sc.GetScopes())
Expand All @@ -48,6 +50,8 @@ func TestDefaultClient(t *testing.T) {
sc.ResponseTypes = []string{}
assert.Equal(t, "code", sc.GetResponseTypes()[0])
assert.Equal(t, "authorization_code", sc.GetGrantTypes()[0])

var _ ClientWithSecretRotation = sc
}

func TestDefaultResponseModeClient_GetResponseMode(t *testing.T) {
Expand Down
14 changes: 14 additions & 0 deletions internal/client.go

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

2 changes: 1 addition & 1 deletion introspection_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (f *Fosite) NewIntrospectionRequest(ctx context.Context, r *http.Request, s
}

// Enforce client authentication
if err := f.Hasher.Compare(ctx, client.GetHashedSecret(), []byte(clientSecret)); err != nil {
if err := f.checkClientSecret(ctx, client, []byte(clientSecret)); err != nil {
return &IntrospectionResponse{Active: false}, errorsx.WithStack(ErrRequestUnauthorized.WithHint("OAuth 2.0 Client credentials are invalid."))
}
}
Expand Down
18 changes: 18 additions & 0 deletions introspection_request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,24 @@ func TestNewIntrospectionRequest(t *testing.T) {
},
isActive: true,
},
{
description: "should pass with basic auth if username and password not encoded",
setup: func() {
f.TokenIntrospectionHandlers = TokenIntrospectionHandlers{validator}
httpreq = &http.Request{
Method: "POST",
Header: http.Header{
//Basic Authorization with username=my-client and password=foobaz
"Authorization": []string{"Basic bXktY2xpZW50OmZvb2Jheg=="},
},
PostForm: url.Values{
"token": []string{"introspect-token"},
},
}
validator.EXPECT().IntrospectToken(ctx, "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
},
isActive: true,
},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
c.setup()
Expand Down
26 changes: 14 additions & 12 deletions storage/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,22 @@ func NewExampleStore() *MemoryStore {
IDSessions: make(map[string]fosite.Requester),
Clients: map[string]fosite.Client{
"my-client": &fosite.DefaultClient{
ID: "my-client",
Secret: []byte(`$2a$10$IxMdI6d.LIRZPpSfEwNoeu4rY3FhDREsxFJXikcgdRRAStxUlsuEO`), // = "foobar"
RedirectURIs: []string{"http://localhost:3846/callback"},
ResponseTypes: []string{"id_token", "code", "token", "id_token token", "code id_token", "code token", "code id_token token"},
GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"},
Scopes: []string{"fosite", "openid", "photos", "offline"},
ID: "my-client",
Secret: []byte(`$2a$10$IxMdI6d.LIRZPpSfEwNoeu4rY3FhDREsxFJXikcgdRRAStxUlsuEO`), // = "foobar"
RotatedSecrets: [][]byte{[]byte(`$2y$10$X51gLxUQJ.hGw1epgHTE5u0bt64xM0COU7K9iAp.OFg8p2pUd.1zC `)}, // = "foobaz",
RedirectURIs: []string{"http://localhost:3846/callback"},
ResponseTypes: []string{"id_token", "code", "token", "id_token token", "code id_token", "code token", "code id_token token"},
GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"},
Scopes: []string{"fosite", "openid", "photos", "offline"},
},
"encoded:client": &fosite.DefaultClient{
ID: "encoded:client",
Secret: []byte(`$2a$10$A7M8b65dSSKGHF0H2sNkn.9Z0hT8U1Nv6OWPV3teUUaczXkVkxuDS`), // = "encoded&password"
RedirectURIs: []string{"http://localhost:3846/callback"},
ResponseTypes: []string{"id_token", "code", "token", "id_token token", "code id_token", "code token", "code id_token token"},
GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"},
Scopes: []string{"fosite", "openid", "photos", "offline"},
ID: "encoded:client",
Secret: []byte(`$2a$10$A7M8b65dSSKGHF0H2sNkn.9Z0hT8U1Nv6OWPV3teUUaczXkVkxuDS`), // = "encoded&password"
RotatedSecrets: nil,
RedirectURIs: []string{"http://localhost:3846/callback"},
ResponseTypes: []string{"id_token", "code", "token", "id_token token", "code id_token", "code token", "code id_token token"},
GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"},
Scopes: []string{"fosite", "openid", "photos", "offline"},
},
},
Users: map[string]MemoryUserRelation{
Expand Down

0 comments on commit b4baf9e

Please sign in to comment.