Skip to content

Commit

Permalink
Allow encrypted client secrets
Browse files Browse the repository at this point in the history
  • Loading branch information
liggitt committed Dec 8, 2015
1 parent d47a3d4 commit a379063
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 3 deletions.
18 changes: 15 additions & 3 deletions access.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,22 @@ func getClient(auth *BasicAuth, storage Storage, w *Response) Client {
w.SetError(E_UNAUTHORIZED_CLIENT, "")
return nil
}
if client.GetSecret() != auth.Password {
w.SetError(E_UNAUTHORIZED_CLIENT, "")
return nil

switch client := client.(type) {
case ClientSecretMatcher:
// Prefer the more secure method of giving the secret to the client for comparison
if !client.ClientSecretMatches(auth.Password) {
w.SetError(E_UNAUTHORIZED_CLIENT, "")
return nil
}
default:
// Fallback to the less secure method of extracting the plain text secret from the client for comparison
if client.GetSecret() != auth.Password {
w.SetError(E_UNAUTHORIZED_CLIENT, "")
return nil
}
}

if client.GetRedirectUri() == "" {
w.SetError(E_UNAUTHORIZED_CLIENT, "")
return nil
Expand Down
97 changes: 97 additions & 0 deletions access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,100 @@ func TestExtraScopes(t *testing.T) {
}

}

// clientWithoutMatcher just implements the base Client interface
type clientWithoutMatcher struct {
Id string
Secret string
RedirectUri string
}

func (c *clientWithoutMatcher) GetId() string { return c.Id }
func (c *clientWithoutMatcher) GetSecret() string { return c.Secret }
func (c *clientWithoutMatcher) GetRedirectUri() string { return c.RedirectUri }
func (c *clientWithoutMatcher) GetUserData() interface{} { return nil }

func TestGetClientWithoutMatcher(t *testing.T) {
myclient := &clientWithoutMatcher{
Id: "myclient",
Secret: "myclientsecret",
RedirectUri: "http://www.example.com",
}
storage := &TestingStorage{clients: map[string]Client{myclient.Id: myclient}}

// Ensure bad secret fails
{
auth := &BasicAuth{
Username: "myclient",
Password: "invalidsecret",
}
w := &Response{}
client := getClient(auth, storage, w)
if client != nil {
t.Errorf("Expected error, got client: %v", client)
}
}

// Ensure good secret works
{
auth := &BasicAuth{
Username: "myclient",
Password: "myclientsecret",
}
w := &Response{}
client := getClient(auth, storage, w)
if client != myclient {
t.Errorf("Expected client, got nil with response: %v", w)
}
}
}

// clientWithMatcher implements the base Client interface and the ClientSecretMatcher interface
type clientWithMatcher struct {
Id string
Secret string
RedirectUri string
}

func (c *clientWithMatcher) GetId() string { return c.Id }
func (c *clientWithMatcher) GetSecret() string { panic("called GetSecret"); return "" }
func (c *clientWithMatcher) GetRedirectUri() string { return c.RedirectUri }
func (c *clientWithMatcher) GetUserData() interface{} { return nil }
func (c *clientWithMatcher) ClientSecretMatches(secret string) bool {
return secret == c.Secret
}

func TestGetClientSecretMatcher(t *testing.T) {
myclient := &clientWithMatcher{
Id: "myclient",
Secret: "myclientsecret",
RedirectUri: "http://www.example.com",
}
storage := &TestingStorage{clients: map[string]Client{myclient.Id: myclient}}

// Ensure bad secret fails, but does not panic (doesn't call GetSecret)
{
auth := &BasicAuth{
Username: "myclient",
Password: "invalidsecret",
}
w := &Response{}
client := getClient(auth, storage, w)
if client != nil {
t.Errorf("Expected error, got client: %v", client)
}
}

// Ensure good secret works, but does not panic (doesn't call GetSecret)
{
auth := &BasicAuth{
Username: "myclient",
Password: "myclientsecret",
}
w := &Response{}
client := getClient(auth, storage, w)
if client != myclient {
t.Errorf("Expected client, got nil with response: %v", w)
}
}
}
13 changes: 13 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ type Client interface {
GetUserData() interface{}
}

// ClientSecretMatcher is an optional interface clients can implement
// which allows them to be the one to determine if a secret matches.
// If a Client implements ClientSecretMatcher, the framework will never call GetSecret
type ClientSecretMatcher interface {
// SecretMatches returns true if the given secret matches
ClientSecretMatches(secret string) bool
}

// DefaultClient stores all data in struct variables
type DefaultClient struct {
Id string
Expand All @@ -39,6 +47,11 @@ func (d *DefaultClient) GetUserData() interface{} {
return d.UserData
}

// Implement the ClientSecretMatcher interface
func (d *DefaultClient) ClientSecretMatches(secret string) bool {
return d.Secret == secret
}

func (d *DefaultClient) CopyFrom(client Client) {
d.Id = client.GetId()
d.Secret = client.GetSecret()
Expand Down

0 comments on commit a379063

Please sign in to comment.