Skip to content

Commit

Permalink
Adds ability to catch non-conform OIDC authorizations
Browse files Browse the repository at this point in the history 
Fosite is now capable of detecting authorization flows that
are not conformant with the OpenID Connect spec.
  • Loading branch information
arekkas committed Dec 10, 2017
1 parent 4c7e4e5 commit 81f77cd
Show file tree
Hide file tree
Showing 7 changed files with 354 additions and 37 deletions.
18 changes: 18 additions & 0 deletions errors.go
View file
Expand Up @@ -159,9 +159,27 @@ var (
Description: "Token was not issued to the client making the revokation request",
Code: http.StatusBadRequest,
}
ErrLoginRequired = &RFC6749Error{
Name: errLoginRequired,
Description: "The Authorization Server requires End-User authentication",
Code: http.StatusBadRequest,
}
ErrInteractionRequired = &RFC6749Error{
Description: "The Authorization Server requires End-User interaction of some form to proceed",
Name: errInteractionRequired,
Code: http.StatusBadRequest,
}
ErrConsentRequired = &RFC6749Error{
Description: "The Authorization Server requires End-User consent",
Name: errConsentRequired,
Code: http.StatusBadRequest,
}
)

const (
errConsentRequired = "consent_required"
errInteractionRequired = "interaction_required"
errLoginRequired = "login_required"
errRequestUnauthorizedName = "request_unauthorized"
errRequestForbidden = "request_forbidden"
errInvalidRequestName = "invalid_request"
Expand Down
68 changes: 63 additions & 5 deletions handler/openid/strategy_jwt.go
View file
Expand Up @@ -18,6 +18,10 @@ import (
"context"
"time"

"fmt"
"strconv"

jwtgo "github.com/dgrijalva/jwt-go"
"github.com/mohae/deepcopy"
"github.com/ory/fosite"
"github.com/ory/fosite/token/jwt"
Expand Down Expand Up @@ -45,7 +49,9 @@ type DefaultSession struct {

func NewDefaultSession() *DefaultSession {
return &DefaultSession{
Claims: &jwt.IDTokenClaims{},
Claims: &jwt.IDTokenClaims{
RequestedAt: time.Now().UTC(),
},
Headers: &jwt.Headers{},
}
}
Expand Down Expand Up @@ -123,14 +129,66 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req
}

claims := sess.IDTokenClaims()
if requester.GetRequestForm().Get("max_age") != "" && (claims.AuthTime.IsZero() || claims.AuthTime.After(time.Now().UTC())) {
return "", errors.New("Failed to generate id token because authentication time claim is required when max_age is set and can not be in the future")
}

if claims.Subject == "" {
return "", errors.New("Failed to generate id token because subject is an empty string")
}

if requester.GetRequestForm().Get("grant_type") != "refresh_token" {
maxAge, err := strconv.ParseInt(requester.GetRequestForm().Get("max_age"), 10, 64)
if err != nil {
maxAge = 0
}

if maxAge > 0 {
if claims.AuthTime.IsZero() || claims.AuthTime.After(time.Now()) {
return "", errors.New("Failed to generate id token because authentication time claim is required when max_age is set and can not be in the future")
} else if claims.AuthTime.Add(time.Second * time.Duration(maxAge)).Before(time.Now()) {
return "", errors.WithStack(fosite.ErrLoginRequired.WithDebug("Failed to generate id token because authentication time does not satisfy max_age time"))
}
}

prompt := requester.GetRequestForm().Get("prompt")
if prompt != "" {
if claims.AuthTime.IsZero() || claims.AuthTime.After(time.Now()) {
return "", errors.New("Unable to determine validity of prompt parameter because auth_time is missing in id token claims")
}
}

switch prompt {
case "none":
if claims.AuthTime.After(claims.RequestedAt) {
return "", errors.WithStack(fosite.ErrLoginRequired.WithDebug("Failed to generate id token because prompt was set to \"none\" but auth_time happened after the authorization request was registered, indicating that the user was logged in during this request which is not allowed"))
}
break
case "login":
if claims.AuthTime.Before(claims.RequestedAt) {
return "", errors.WithStack(fosite.ErrLoginRequired.WithDebug("Failed to generate id token because prompt was set to \"login\" but auth_time happened before the authorization request was registered, indicating that the user was not re-authenticated which is forbidden"))
}
break
}

// If acr_values was requested but no acr value was provided in the ID token, fall back to level 0 which means least
// confidence in authentication.
if requester.GetRequestForm().Get("acr_values") != "" && claims.AuthenticationContextClassReference == "" {
claims.AuthenticationContextClassReference = "0"
}

if tokenHintString := requester.GetRequestForm().Get("id_token_hint"); tokenHintString != "" {
tokenHint, err := h.RS256JWTStrategy.Decode(tokenHintString)
if err != nil {
return "", errors.WithStack(fosite.ErrInvalidRequest.WithDebug(fmt.Sprintf("Unable to decode id token from id_token_hint parameter because %s", err.Error())))
}

if hintClaims, ok := tokenHint.Claims.(jwtgo.MapClaims); !ok {
return "", errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Unable to decode id token from id_token_hint to *jwt.StandardClaims"))
} else if hintSub, _ := hintClaims["sub"].(string); hintSub == "" {
return "", errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Provided id token from id_token_hint does not have a subject"))
} else if hintSub != claims.Subject {
return "", errors.WithStack(fosite.ErrLoginRequired.WithDebug(fmt.Sprintf("Subject from authorization mismatches id token subject from id_token_hint")))
}
}
}

if claims.ExpiresAt.IsZero() {
claims.ExpiresAt = time.Now().UTC().Add(h.Expiry)
}
Expand Down
164 changes: 156 additions & 8 deletions handler/openid/strategy_jwt_test.go
View file
Expand Up @@ -18,6 +18,8 @@ import (
"testing"
"time"

"fmt"

"github.com/ory/fosite"
"github.com/ory/fosite/token/jwt"
"github.com/stretchr/testify/assert"
Expand All @@ -26,8 +28,9 @@ import (
func TestJWTStrategy_GenerateIDToken(t *testing.T) {
var req *fosite.AccessRequest
for k, c := range []struct {
setup func()
expectErr bool
description string
setup func()
expectErr bool
}{
{
setup: func() {
Expand Down Expand Up @@ -102,12 +105,157 @@ func TestJWTStrategy_GenerateIDToken(t *testing.T) {
},
expectErr: false,
},
{
description: "should pass because max_age was requested and auth_time happened after initial request time",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().UTC(),
},
Headers: &jwt.Headers{},
})
req.Form.Set("max_age", "60")
},
expectErr: false,
},
{
description: "should fail because max_age was requested and auth_time has expired",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().Add(-time.Hour).UTC(),
},
Headers: &jwt.Headers{},
})
req.Form.Set("max_age", "60")
},
expectErr: true,
},
{
description: "should fail because prompt=none was requested and auth_time indicates fresh login",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
req.Form.Set("prompt", "none")
},
expectErr: true,
},
{
description: "should pass because prompt=none was requested and auth_time indicates fresh login but grant type is refresh_token",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
req.Form.Set("prompt", "none")
req.Form.Set("grant_type", "refresh_token")
},
expectErr: false,
},
{
description: "should pass because prompt=none was requested and auth_time indicates old login",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().Add(-time.Hour).UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
req.Form.Set("prompt", "none")
},
expectErr: false,
},
{
description: "should pass because prompt=login was requested and auth_time indicates fresh login",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
req.Form.Set("prompt", "login")
},
expectErr: false,
},
{
description: "should fail because prompt=login was requested and auth_time indicates old login",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().Add(-time.Hour).UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
req.Form.Set("prompt", "login")
},
expectErr: true,
},
{
description: "should pass because id_token_hint subject matches subject from claims",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().Add(-time.Hour).UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
token, _ := j.GenerateIDToken(nil, fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
},
Headers: &jwt.Headers{},
}))
req.Form.Set("id_token_hint", token)
},
expectErr: false,
},
{
description: "should fail because id_token_hint subject does not match subject from claims",
setup: func() {
req = fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{
Subject: "peter",
AuthTime: time.Now().Add(-time.Hour).UTC(),
RequestedAt: time.Now().Add(-time.Minute),
},
Headers: &jwt.Headers{},
})
token, _ := j.GenerateIDToken(nil, fosite.NewAccessRequest(&DefaultSession{
Claims: &jwt.IDTokenClaims{Subject: "alice"}, Headers: &jwt.Headers{},
}))
req.Form.Set("id_token_hint", token)
},
expectErr: true,
},
} {
c.setup()
token, err := j.GenerateIDToken(nil, req)
assert.Equal(t, c.expectErr, err != nil, "%d: %s", k, err)
if !c.expectErr {
assert.NotEmpty(t, token)
}
t.Run(fmt.Sprintf("case=%d/description=%s", k, c.description), func(t *testing.T) {
c.setup()
token, err := j.GenerateIDToken(nil, req)
assert.Equal(t, c.expectErr, err != nil, "%d: %s", k, err)
if !c.expectErr {
assert.NotEmpty(t, token)
}
})
}
}

0 comments on commit 81f77cd

Please sign in to comment.