Skip to content

Commit

Permalink
all: enforce use of scopes
Browse files Browse the repository at this point in the history
Close #14
  • Loading branch information
Aeneas Rekkas committed Jan 11, 2016
1 parent 49aa920 commit 73b61a9
Show file tree
Hide file tree
Showing 15 changed files with 230 additions and 37 deletions.
32 changes: 12 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ Please be aware that Fosite only secures your server side security. You still ne
your tokens safe, prevent CSRF attacks and much more. If you need any help or advice feel free to contact our security
staff through [our website](https://ory.am/)!

## Security first or encouraging security by enforcing it

We have given [OAuth 2.0 Threat Model and Security Considerations](https://tools.ietf.org/html/rfc6819#section-5.1.5.3)
We have given the [OAuth 2.0 Threat Model and Security Considerations](https://tools.ietf.org/html/rfc6819#section-5.1.5.3)
a very close look and included everything we thought was in the scope of this framework. Here is a complete list
of things we implemented in Fosite:

Expand All @@ -97,11 +95,14 @@ Not implemented yet:
cryptography per default instead of HMAC-SHA (but support both).

Additionally, we added these safeguards:
* Enforcing random states: Without a random-looking state the request will fail.
* Advanced Token Validation: Tokens are layouted as `<key>.<signature>` where `<signature>` is created using HMAC-SHA256, a global secret
* **Enforcing random states:** Without a random-looking state the request will fail.
* **Advanced Token Validation:** Tokens are layouted as `<key>.<signature>` where `<signature>` is created using HMAC-SHA256, a global secret
and the client's secret. Read more about this workflow in the [proposal](https://github.com/ory-am/fosite/issues/11).
This is what a token can look like:
`/tgBeUhWlAT8tM8Bhmnx+Amf8rOYOUhrDi3pGzmjP7c=.BiV/Yhma+5moTP46anxMT6cWW8gz5R5vpC9RbpwSDdM=`
* **Enforging scopes:** By default, you always need to include the `fosite` scope or fosite will not execute the request
properly. Obviously, you can change the scope to `basic` or `core` but be aware that you *must use scopes* if you use
OAuth2.

It is strongly encouraged to use the handlers shipped with Fosite as the follow specs.

Expand All @@ -120,22 +121,13 @@ The following list documents which sections of the RFCs we reviewed for each act
* [OAuth 2.0 Threat Model and Security Considerations](https://tools.ietf.org/html/rfc6819)
* ...

####


#### Encrypt credentials at rest

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

#### Implement peer reviewed IETF Standards

Fosite implements [rfc6749](https://tools.ietf.org/html/rfc6749) and enforces countermeasures suggested in [rfc6819](https://tools.ietf.org/html/rfc6819).

### Provide extensibility and interoperability
## A word on extensibility

... because OAuth2 is an extensible and flexible **framework**. Fosite let's you register new response types, new grant
types and new response key value pares. This is useful, if you want to provide OpenID Connect on top of your
OAuth2 stack. Or custom assertions, what ever you like and as long as it is secure. ;)
Fosite is extensible ... because OAuth2 is an extensible and flexible **framework**.
Fosite let's you register custom token and authorize endpoint handlers with the security that the requests
have been validated against the OAuth2 specs beforehand.
You can easily extend Fosite's capabilities. For example, if you want to provide OpenID Connect on top of your
OAuth2 stack, that's no problem. Or custom assertions, what ever you like and as long as it is secure. ;)

## Usage

Expand Down
30 changes: 30 additions & 0 deletions access_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ type AccessRequester interface {
// GetRequestedAt returns the time the request was created.
GetRequestedAt() time.Time

// GetScopes returns the request's scopes.
GetScopes() Arguments

// SetScopes sets the request's scopes.
SetScopes(Arguments)

// GetGrantScopes returns all granted scopes.
GetGrantedScopes() Arguments

// GrantScope marks a request's scope as granted.
GrantScope(string)

// SetGrantTypeHandled marks a grant type as handled indicating that the response type is supported.
SetGrantTypeHandled(string)

Expand All @@ -27,6 +39,8 @@ type AccessRequest struct {
HandledGrantType []string
RequestedAt time.Time
Client client.Client
Scopes Arguments
GrantedScopes []string
}

func NewAccessRequest() *AccessRequest {
Expand Down Expand Up @@ -55,3 +69,19 @@ func (a *AccessRequest) GetRequestedAt() time.Time {
func (a *AccessRequest) GetClient() client.Client {
return a.Client
}

func (a *AccessRequest) GetScopes() Arguments {
return a.Scopes
}

func (a *AccessRequest) SetScopes(s Arguments) {
a.Scopes = s
}

func (a *AccessRequest) GetGrantedScopes() Arguments {
return Arguments(a.GrantedScopes)
}

func (a *AccessRequest) GrantScope(scope string) {
a.GrantedScopes = append(a.GrantedScopes, scope)
}
17 changes: 12 additions & 5 deletions access_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/go-errors/errors"
"golang.org/x/net/context"
"net/http"
"strings"
)

// Implements
Expand Down Expand Up @@ -31,14 +32,14 @@ import (
// credentials (or assigned other authentication requirements), the
// client MUST authenticate with the authorization server as described
// in Section 3.2.1.
func (c *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session interface{}) (AccessRequester, error) {
func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session interface{}) (AccessRequester, error) {
ar := NewAccessRequest()
if r.Method != "POST" {
return ar, errors.New(ErrInvalidRequest)
}

if c.RequiredScope == "" {
c.RequiredScope = DefaultRequiredScopeName
if f.RequiredScope == "" {
f.RequiredScope = DefaultRequiredScopeName
}

if err := r.ParseForm(); err != nil {
Expand All @@ -49,6 +50,7 @@ func (c *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
return ar, errors.New("Session must not be nil")
}

ar.Scopes = removeEmpty(strings.Split(r.Form.Get("scope"), " "))
ar.GrantType = r.Form.Get("grant_type")
if ar.GrantType == "" {
return ar, errors.New(ErrInvalidRequest)
Expand All @@ -59,7 +61,7 @@ func (c *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
return ar, errors.New(ErrInvalidRequest)
}

client, err := c.Store.GetClient(clientID)
client, err := f.Store.GetClient(clientID)
if err != nil {
return ar, errors.New(ErrInvalidClient)
}
Expand All @@ -70,7 +72,7 @@ func (c *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
}
ar.Client = client

for _, loader := range c.TokenEndpointHandlers {
for _, loader := range f.TokenEndpointHandlers {
if err := loader.HandleTokenEndpointRequest(ctx, r, ar, session); err != nil {
return ar, err
}
Expand All @@ -80,5 +82,10 @@ func (c *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
return ar, errors.New(ErrUnsupportedGrantType)
}

if !ar.GetScopes().Has(f.RequiredScope) {
return ar, errors.New(ErrInvalidScope)
}

ar.GrantScope(f.RequiredScope)
return ar, nil
}
20 changes: 20 additions & 0 deletions access_request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,26 @@ func TestNewAccessRequest(t *testing.T) {
client.EXPECT().CompareSecretWith(gomock.Eq([]byte("bar"))).Return(true)
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester, _ interface{}) {
a.SetGrantTypeHandled("foo")
a.SetScopes([]string{"asdfasdf"})
}).Return(nil)
},
handlers: TokenEndpointHandlers{handler},
expectErr: ErrInvalidScope,
},
{
header: http.Header{
"Authorization": {basicAuth("foo", "bar")},
},
method: "POST",
form: url.Values{
"grant_type": {"foo"},
},
mock: func() {
store.EXPECT().GetClient(gomock.Eq("foo")).Return(client, nil)
client.EXPECT().CompareSecretWith(gomock.Eq([]byte("bar"))).Return(true)
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester, _ interface{}) {
a.SetGrantTypeHandled("foo")
a.SetScopes([]string{DefaultRequiredScopeName})
}).Return(nil)
},
handlers: TokenEndpointHandlers{handler},
Expand Down
2 changes: 2 additions & 0 deletions access_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ func TestAccessRequest(t *testing.T) {
ar := NewAccessRequest()
ar.GrantType = "foobar"
ar.Client = &client.SecureClient{}
ar.GrantScope("foo")
assert.True(t, ar.GetGrantedScopes().Has("foo"))
assert.NotNil(t, ar.GetRequestedAt())
assert.Equal(t, ar.GrantType, ar.GetGrantType())
assert.Equal(t, ar.Client, ar.GetClient())
Expand Down
26 changes: 25 additions & 1 deletion authorize_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,21 @@ type AuthorizeRequester interface {
// IsRedirectURIValid returns false if the redirect is not rfc-conform (i.e. missing client, not on white list,
// or malformed)
IsRedirectURIValid() bool

// SetScopes sets the request's scopes.
SetScopes(Arguments)

// GetGrantScopes returns all granted scopes.
GetGrantedScopes() Arguments
}

func NewAuthorizeRequest() *AuthorizeRequest {
return &AuthorizeRequest{}
return &AuthorizeRequest{
ResponseTypes: Arguments{},
Scopes: Arguments{},
HandledResponseTypes: Arguments{},
GrantedScopes: []string{},
}
}

// AuthorizeRequest is an implementation of AuthorizeRequester
Expand All @@ -51,6 +62,7 @@ type AuthorizeRequest struct {
State string
RequestedAt time.Time
HandledResponseTypes Arguments
GrantedScopes []string
}

func (d *AuthorizeRequest) IsRedirectURIValid() bool {
Expand Down Expand Up @@ -107,3 +119,15 @@ func (d *AuthorizeRequest) DidHandleAllResponseTypes() bool {

return len(d.ResponseTypes) > 0
}

func (a *AuthorizeRequest) GetGrantedScopes() Arguments {
return Arguments(a.GrantedScopes)
}

func (a *AuthorizeRequest) GrantScope(scope string) {
a.GrantedScopes = append(a.GrantedScopes, scope)
}

func (a *AuthorizeRequest) SetScopes(s Arguments) {
a.Scopes = s
}
5 changes: 5 additions & 0 deletions authorize_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,10 @@ func (c *Fosite) NewAuthorizeRequest(_ context.Context, r *http.Request) (Author
// Remove empty items from arrays
request.Scopes = removeEmpty(strings.Split(r.Form.Get("scope"), " "))

if !request.Scopes.Has(c.RequiredScope) {
return request, errors.New(ErrInvalidScope)
}
request.GrantScope(c.RequiredScope)

return request, nil
}
36 changes: 26 additions & 10 deletions authorize_request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,10 @@ func TestNewAuthorizeRequest(t *testing.T) {
desc: "short state",
conf: &Fosite{Store: store},
query: url.Values{
"redirect_uri": []string{"http://foo.bar/cb"},
"client_id": []string{"1234"},
"response_type": []string{"code"},
"state": []string{"short"},
"redirect_uri": {"http://foo.bar/cb"},
"client_id": {"1234"},
"response_type": {"code"},
"state": {"short"},
},
expectedError: ErrInvalidState,
mock: func() {
Expand All @@ -137,11 +137,27 @@ func TestNewAuthorizeRequest(t *testing.T) {
desc: "should pass",
conf: &Fosite{Store: store},
query: url.Values{
"redirect_uri": []string{"http://foo.bar/cb"},
"client_id": []string{"1234"},
"response_type": []string{"code token"},
"state": []string{"strong-state"},
"scope": []string{"foo bar"},
"redirect_uri": {"http://foo.bar/cb"},
"client_id": {"1234"},
"response_type": {"code token"},
"state": {"strong-state"},
"scope": {"foo bar"},
},
mock: func() {
store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}}, nil)
},
expectedError: ErrInvalidScope,
},
/* success case */
{
desc: "should pass",
conf: &Fosite{Store: store},
query: url.Values{
"redirect_uri": {"http://foo.bar/cb"},
"client_id": {"1234"},
"response_type": {"code token"},
"state": {"strong-state"},
"scope": {DefaultRequiredScopeName + " foo bar"},
},
mock: func() {
store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}}, nil)
Expand All @@ -151,7 +167,7 @@ func TestNewAuthorizeRequest(t *testing.T) {
Client: &SecureClient{RedirectURIs: []string{"http://foo.bar/cb"}},
ResponseTypes: []string{"code", "token"},
State: "strong-state",
Scopes: []string{"foo", "bar"},
Scopes: []string{DefaultRequiredScopeName, "foo", "bar"},
},
},
} {
Expand Down
2 changes: 2 additions & 0 deletions authorize_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,7 @@ func TestAuthorizeRequest(t *testing.T) {
assert.Equal(t, c.ar.Scopes, c.ar.GetScopes(), "%d", k)
assert.Equal(t, c.ar.State, c.ar.GetState(), "%d", k)
assert.Equal(t, c.isRedirValid, c.ar.IsRedirectURIValid(), "%d", k)
c.ar.GrantScope("foo")
assert.True(t, c.ar.GetGrantedScopes().Has("foo"))
}
}
3 changes: 3 additions & 0 deletions handler/authorize/explicit/explicit_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ func (c *AuthorizeExplicitEndpointHandler) HandleTokenEndpointRequest(_ context.
return errors.New(ErrServerError)
}

// Override scopes
request.SetScopes(ar.GetScopes())

// The authorization server MUST ensure that the authorization code was issued to the authenticated
// confidential client, or if the client is public, ensure that the
// code was issued to "client_id" in the request,
Expand Down
10 changes: 10 additions & 0 deletions handler/authorize/explicit/explicit_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ func TestHandleTokenEndpointRequest(t *testing.T) {
chgen.EXPECT().ValidateChallenge(gomock.Any(), gomock.Any()).Return(nil)
store.EXPECT().GetAuthorizeCodeSession(gomock.Any(), gomock.Any()).Return(authreq, nil)

authreq.EXPECT().GetScopes().Return([]string{})
areq.EXPECT().SetScopes(gomock.Any())
authreq.EXPECT().GetClient().Return(&client.SecureClient{ID: "bar"})
},
expectErr: fosite.ErrInvalidRequest,
Expand All @@ -244,6 +246,8 @@ func TestHandleTokenEndpointRequest(t *testing.T) {
sess.RequestRedirectURI = "request-redir"
}).Return(authreq, nil)

authreq.EXPECT().GetScopes().Return([]string{})
areq.EXPECT().SetScopes(gomock.Any())
authreq.EXPECT().GetClient().Return(&client.SecureClient{ID: "foo"})
},
expectErr: fosite.ErrInvalidRequest,
Expand All @@ -263,6 +267,8 @@ func TestHandleTokenEndpointRequest(t *testing.T) {
sess.RequestRedirectURI = "request-redir"
}).Return(authreq, nil)

authreq.EXPECT().GetScopes().Return([]string{})
areq.EXPECT().SetScopes(gomock.Any())
authreq.EXPECT().GetClient().Return(&client.SecureClient{ID: "foo"})
authreq.EXPECT().GetRequestedAt().Return(time.Now().Add(-time.Hour))
},
Expand All @@ -281,6 +287,8 @@ func TestHandleTokenEndpointRequest(t *testing.T) {
chgen.EXPECT().ValidateChallenge(gomock.Any(), gomock.Any()).Return(nil)
store.EXPECT().GetAuthorizeCodeSession(gomock.Any(), gomock.Any()).Return(authreq, nil)

authreq.EXPECT().GetScopes().Return([]string{})
areq.EXPECT().SetScopes(gomock.Any())
authreq.EXPECT().GetClient().Return(&client.SecureClient{ID: "foo"})
authreq.EXPECT().GetRequestedAt().Return(time.Now().Add(-time.Hour))
},
Expand All @@ -298,6 +306,8 @@ func TestHandleTokenEndpointRequest(t *testing.T) {
chgen.EXPECT().ValidateChallenge(gomock.Any(), gomock.Any()).Return(nil)
store.EXPECT().GetAuthorizeCodeSession(gomock.Any(), gomock.Any()).Return(authreq, nil)

authreq.EXPECT().GetScopes().Return([]string{})
areq.EXPECT().SetScopes(gomock.Any())
authreq.EXPECT().GetClient().Return(&client.SecureClient{ID: "foo"})
authreq.EXPECT().GetRequestedAt().Return(time.Now().Add(time.Hour))
areq.EXPECT().SetGrantTypeHandled("authorization_code")
Expand Down
Loading

0 comments on commit 73b61a9

Please sign in to comment.