Skip to content

Commit

Permalink
all: refactor, fixed tests, incorporated feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Aeneas Rekkas committed Jan 2, 2016
1 parent 480af91 commit 9e59df2
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 131 deletions.
13 changes: 12 additions & 1 deletion .travis.yml
Expand Up @@ -9,7 +9,17 @@ env:
language: go

go:
- 1.2
- 1.3
- 1.4
- 1.5
- tip

matrix:
allow_failures:
- go: tip
- go: 1.2
- go: 1.3

install:
- go get golang.org/x/tools/cmd/vet
Expand All @@ -21,4 +31,5 @@ install:

script:
- go vet -x ./...
- ./coverage --coveralls
- ./coverage --coveralls

38 changes: 17 additions & 21 deletions README.md
Expand Up @@ -77,20 +77,22 @@ c. get hold of the token encryption secret.

A database leak or (exclusively) the knowledge of the token encrpytion secret are not enough to maliciously obtain or create a valid token. Tokens and credentials can
however still be stolen by man-in-the-middle attacks, by malicious or vulnerable clients and other attack vectors.
* Issuance
1. The key is hashed using BCrypt (variable) and used as `<signature>`.
2. The client is presented with `<key>.<signature>`.
3. The signature is encrypted and stored in the database using AES (variable).
* Validation
1. The client presents `<key>.<signature>`.
2. It is validated if <key> matches <signature> using BCrypt (variable).
3. The signature is encrypted using AES (variable).
4. The encrypted signature is looked up in the database, failing validation if no such row exists.
5. They key is considered valid and is now subject for other validations, like audience, redirect or state matching.

**Issuance**
1. The key is hashed using BCrypt (variable) and used as `<signature>`.
2. The client is presented and entrusted with `<key>.<signature>` which can act for example as an access token or an authorize code.
3. The signature is encrypted and stored in the database using AES (variable).

**Validation**
1. The client presents `<key>.<signature>`.
2. It is validated if <key> matches <signature> using BCrypt (variable).
3. The signature is encrypted using AES (variable).
4. The encrypted signature is looked up in the database, failing validation if no such row exists.
5. They key is considered valid and is now subject for other validations, like audience, redirect or state matching.

#### Encrypt credentials at rest

Credentials (tokens, passwords and secrets) are always encrypted at rest.
Credentials (token signatures, passwords and secrets) are always encrypted at rest.

#### Implement peer reviewed IETF Standards

Expand Down Expand Up @@ -128,8 +130,7 @@ var r *http.Request // we're assuming that we are inside a http.Handler
var rw http.ResponseWriter // we're assuming that we are inside a http.Handler

var store fosite.Storage // needs to be implemented or by using some library
config := fosite.NewDefaultConfig()
oauth := fosite.NewOAuth(config)
oauth := fosite.NewDefault()
authorizeRequest, err := oauth.NewAuthorizeRequest(r, store)
if err != nil {
oauth.RedirectError(rw, error)
Expand All @@ -146,11 +147,8 @@ if err != nil {
// session := oauth2.NewAuthorizeSession(123)
// session.SetExtra(extra interface{})

// persist that stuff in the database
// err = oauth2.PersistAuthorizeRequest(authorizeRequest, session) // sets e.g. session.Persistent = true

// finally, persist code in store and send response
// e.g: oauth2.WriteResponse(rw, response, session)
// e.g: oauth2.FinishAccessRequest(rw, response, session)
```

Because each component returns a different type, we can be (if safeguards are installed) quite sure, that the developer
Expand All @@ -160,11 +158,9 @@ implemented the work flow the right way:
2. do whatever you like
3. `HandleAuthorizeRequest(args...) *AuthorizeResponse`: Handle authorize request (check scopes and response types, hydrate response...)
4. do whatever you like
5. `oauth2.NewAuthorizeSession(*AuthorizeResponse) *AuthorizeSession`: A session
5. `oauth2.NewAuthorizeSession(*AuthorizeResponse) (*AuthorizeSession, error)`: A session
6. do whatever you like, e.g. `session.SetExtra(map[string]interface{"foo": "bar"})`
7. `oauth2.PersistAuthorizeRequest` persists the request in the database so the token endpoint can look up information
8. do whatever you like
9. `oauth2.WriteResponse(rw, response, session)` to write the response
9. `oauth2.FinishAccessRequest(rw, response, session)` to write and persist the response
10. done.

It is not clear yet how HandleAuthorizeRequest could be extensible. It might be possible to introduce an interface like AuthorizeStrategy
Expand Down
47 changes: 20 additions & 27 deletions auth/authorize.go → authorize.go
@@ -1,25 +1,23 @@
package auth
package fosite

import (
"net/http"
"github.com/go-errors/errors"
"github.com/ory-am/fosite/generator"
"golang.org/x/net/context"
"net/http"
"net/url"
"strings"
"golang.org/x/net/context"
. "github.com/ory-am/fosite"
"github.com/ory-am/fosite/generator"
)

// Authorize request information
type AuthorizeRequest struct {
Types []string
Types []string
Client Client
Scopes []string
Scopes []string
RedirectURI string
State string
Expiration int32
Code generator.AuthorizeCode
Config *Config
Expiration int32
Code *generator.Token
}

type ScopeStrategy interface {
Expand Down Expand Up @@ -125,47 +123,46 @@ func (c *Config) NewAuthorizeRequest(_ context.Context, r *http.Request, store S
return nil, errors.Wrap(ErrInvalidRequest, 0)
}

code, err := c.AuthorizeCodeGenerator.GenerateAuthorizeCode()
code, err := c.AuthorizeCodeGenerator.Generate()
if state == "" {
return nil, errors.Wrap(ErrServerError, 0)
}

scopes := removeEmpty(strings.Split(r.Form.Get("scope"), " "))
return &AuthorizeRequest{
Types: responseTypes,
Client: client,
Scopes: scopes,
State: state,
Expiration: c.Lifetime,
Types: responseTypes,
Client: client,
Scopes: scopes,
State: state,
Expiration: c.Lifetime,
RedirectURI: redirectURI,
Config: Config,
Code: code,
Code: code,
}, nil
}

func (c *Config) WriteAuthError(rw http.ResponseWriter, req *http.Request, err error) {
redirectURI, err := c.GetRedirectURI(req.Form)
if err != nil {
http.Error(rw, ErrInvalidRequest, http.StatusBadRequest)
http.Error(rw, errInvalidRequestName, http.StatusBadRequest)
return
}

client, err := c.Store.GetClient(req.Form.Get("client_id"))
if err != nil {
http.Error(rw, ErrInvalidClient, http.StatusBadRequest)
http.Error(rw, errInvalidRequestName, http.StatusBadRequest)
return
}

// * rfc6749 10.6. Authorization Code Redirection URI Manipulation
// * rfc6819 4.4.1.7. Threat: Authorization "code" Leakage through Counterfeit Client
if redirectURI, err = c.DoesClientWhiteListRedirect(redirectURI, client); err != nil {
http.Error(rw, ErrInvalidRequest, http.StatusBadRequest)
http.Error(rw, errInvalidRequestName, http.StatusBadRequest)
return
}

redir, err := url.Parse(redirectURI)
if err != nil {
http.Error(rw, ErrInvalidRequest, http.StatusBadRequest)
http.Error(rw, errInvalidRequestName, http.StatusBadRequest)
return
}

Expand All @@ -177,10 +174,6 @@ func (c *Config) WriteAuthError(rw http.ResponseWriter, req *http.Request, err e
rw.WriteHeader(http.StatusFound)
}

func (c *Config) PersistAndWriteAuthorizeCode(*AuthorizeRequest) {

}

func areResponseTypesValid(c *Config, responseTypes []string) bool {
if len(responseTypes) < 1 {
return false
Expand Down Expand Up @@ -210,4 +203,4 @@ func removeEmpty(args []string) (ret []string) {
}
}
return
}
}
15 changes: 7 additions & 8 deletions auth/authorize_test.go → authorize_test.go
@@ -1,27 +1,26 @@
package auth
package fosite

import (
"net/url"
"testing"
"github.com/stretchr/testify/assert"
. "github.com/ory-am/fosite"
"net/url"
"testing"
)

func TestGetRedirectURIAgainstRFC6749Section3(t *testing.T) {
cf := &Config{}
for _ , c := range []struct{
for _, c := range []struct {
in string
isError bool
expected string
} {
}{
{in: "", isError: true},
} {
values := url.Values{}
values.Set("redirect_uri", c)
values.Set("redirect_uri", c.in)
res, err := cf.GetRedirectURI(values)
assert.Equal(t, c.isError, err != nil)
if err == nil {
assert.Equal(t, c.expected, res)
}
}
}
}
2 changes: 1 addition & 1 deletion client.go
Expand Up @@ -28,7 +28,7 @@ func (c *SecureClient) GetID() string {
}

func (c *SecureClient) CompareSecretWith(secret string) bool {
return c.Hasher.Compare(c.Secret, secret) == nil
return c.Hasher.Compare([]byte(c.Secret), []byte(secret)) == nil
}

func (c *SecureClient) GetRedirectURIs() []string {
Expand Down
14 changes: 7 additions & 7 deletions client_test.go
Expand Up @@ -7,16 +7,16 @@ import (
)

func TestSecureClient(t *testing.T) {
hasher := hash.BCrypt{WorkFactor: 5}
secret, _ := hasher.Hash("foo")
hasher := &hash.BCrypt{WorkFactor: 5}
secret, _ := hasher.Hash([]byte("foo"))
sc := &SecureClient{
ID: "1",
Secret: secret,
ID: "1",
Secret: string(secret),
RedirectURIs: []string{"foo", "bar"},
Hasher: hasher,
Hasher: hasher,
}
assert.Equal(t, sc.ID, sc.GetID())
assert.Equal(t, sc.RedirectURIs, sc.RedirectURIs())
assert.Equal(t, sc.RedirectURIs, sc.GetRedirectURIs())
assert.True(t, sc.CompareSecretWith("foo"))
assert.False(t, sc.CompareSecretWith("bar"))
}
}
21 changes: 11 additions & 10 deletions config.go
@@ -1,21 +1,22 @@
package fosite

import "github.com/ory-am/fosite/generator"

type Config struct {
AllowedAuthorizeResponseTypes []string
AllowedTokenResponseTypes []string
Lifetime int32
Store Storage
Entropy int32
AuthorizeCodeGenerator generator.Generator
AllowedTokenResponseTypes []string
Lifetime int32
Store Storage
Entropy int32
AuthorizeCodeGenerator generator.Generator
}

func NewDefaultConfig() *Config {
return &Config{
AllowedAuthorizeResponseTypes: []string{"code", "token", "id_token"},
AllowedTokenResponseTypes: []string{},
Lifetime: 3600,
Entropy: 128,
AuthorizeCodeGenerator: generator.CryptoGenerator{},
AllowedTokenResponseTypes: []string{},
Lifetime: 3600,
Entropy: 128,
AuthorizeCodeGenerator: &generator.CryptoGenerator{},
}
}
}
24 changes: 11 additions & 13 deletions generator/crypto.go
Expand Up @@ -18,7 +18,7 @@ const minimumEntropy = 32

// GenerateAuthorizeCode generates a new authorize code or returns an error.
// This method implements rfc6819 Section 5.1.4.2.2: Use High Entropy for Secrets.
func (c *CryptoGenerator) GenerateAuthorizeCode() (*AuthorizeCode, error) {
func (c *CryptoGenerator) Generate() (*Token, error) {
if c.AuthCodeEntropy < minimumEntropy {
c.AuthCodeEntropy = minimumEntropy
}
Expand Down Expand Up @@ -48,30 +48,28 @@ func (c *CryptoGenerator) GenerateAuthorizeCode() (*AuthorizeCode, error) {
base64.RawStdEncoding.Encode(resultHash, hash)
resultHash = bytes.Trim(resultHash, "\x00")

return &AuthorizeCode{
return &Token{
Key: string(resultKey),
Signature: string(resultHash),
}, nil
}

// ValidateAuthorizeCodeSignature returns an AuthorizeCode, if the code argument is a valid authorize code
// and the signature matches the key.
func (c *CryptoGenerator) ValidateAuthorizeCode(code string) (ac *AuthorizeCode, err error) {
ac = new(AuthorizeCode)
ac.FromString(code)
if ac.Key == "" || ac.Signature == "" {
return nil, errors.New("Key and signature must both be not empty")
func (c *CryptoGenerator) ValidateSignature(t *Token) (err error) {
if t.Key == "" || t.Signature == "" {
return errors.New("Key and signature must both be not empty")
}

signature := make([]byte, base64.RawStdEncoding.DecodedLen(len(ac.Signature)))
if _, err := base64.RawStdEncoding.Decode(signature, []byte(ac.Signature)); err != nil {
return nil, err
signature := make([]byte, base64.RawStdEncoding.DecodedLen(len(t.Signature)))
if _, err := base64.RawStdEncoding.Decode(signature, []byte(t.Signature)); err != nil {
return err
}

if err := c.Hasher.Compare(signature, []byte(ac.Key)); err != nil {
if err := c.Hasher.Compare(signature, []byte(t.Key)); err != nil {
// Hash is invalid
return nil, err
return errors.New("Key and signature do not match")
}

return ac, nil
return nil
}

0 comments on commit 9e59df2

Please sign in to comment.