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: Add rotation support for client secrets #608

Merged
merged 1 commit into from
Jun 18, 2021
Merged
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
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
25 changes: 25 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
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