openid: Improves prompt, max_age and id_token_hint validation (#268)

This patch improves the OIDC prompt, max_age, and id_token_hint
validation.

HISTORY.md

@@ -5,6 +5,8 @@ bumps (`0.1.0` -> `0.2.0`).
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 
 
+- [0.19.0](#0190)
+- [0.18.0](#0180)
 - [0.17.0](#0170)
 - [0.16.0](#0160)
 - [0.15.0](#0150)
@@ -41,6 +43,16 @@ bumps (`0.1.0` -> `0.2.0`).
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
+## 0.19.0
+
+This release improves the OpenID Connect vaildation strategy which now properly handles `prompt`, `max_age`, and `id_token_hint`
+at the `/oauth2/auth` endpoint instead of the `/oauth2/token` endpoint.
+
+To achieve this, the `OpenIDConnectRequestValidator` has been modified and now requires a `jwt.JWTStrategy` (implemented by,
+for example `jwt.RS256JWTStrategy`).
+
+The compose package has been updated accordingly. You should not expect any major breaking changes from this release.
+
 ## 0.18.0
 
 This release allows the introspection handler to return the token type (e.g. `access_token`, `refresh_token`) of the

compose/compose.go

@@ -25,6 +25,7 @@ import (
 	"crypto/rsa"
 
 	"github.com/ory/fosite"
+	"github.com/ory/fosite/token/jwt"
 )
 
 type Factory func(config *Config, storage interface{}, strategy interface{}) interface{}
@@ -93,6 +94,9 @@ func ComposeAllEnabled(config *Config, storage interface{}, secret []byte, key *
 		&CommonStrategy{
 			CoreStrategy:               NewOAuth2HMACStrategy(config, secret),
 			OpenIDConnectTokenStrategy: NewOpenIDConnectStrategy(key),
+			JWTStrategy: &jwt.RS256JWTStrategy{
+				PrivateKey: key,
+			},
 		},
 		nil,
 

compose/compose_openid.go

@@ -24,6 +24,7 @@ package compose
 import (
 	"github.com/ory/fosite/handler/oauth2"
 	"github.com/ory/fosite/handler/openid"
+	"github.com/ory/fosite/token/jwt"
 )
 
 // OpenIDConnectExplicitFactory creates an OpenID Connect explicit ("authorize code flow") grant handler.
@@ -35,7 +36,7 @@ func OpenIDConnectExplicitFactory(config *Config, storage interface{}, strategy
 		IDTokenHandleHelper: &openid.IDTokenHandleHelper{
 			IDTokenStrategy: strategy.(openid.OpenIDConnectTokenStrategy),
 		},
-		OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(config.AllowedPromptValues),
+		OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(config.AllowedPromptValues, strategy.(jwt.JWTStrategy)),
 	}
 }
 
@@ -64,7 +65,7 @@ func OpenIDConnectImplicitFactory(config *Config, storage interface{}, strategy
 		IDTokenHandleHelper: &openid.IDTokenHandleHelper{
 			IDTokenStrategy: strategy.(openid.OpenIDConnectTokenStrategy),
 		},
-		OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(config.AllowedPromptValues),
+		OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(config.AllowedPromptValues, strategy.(jwt.JWTStrategy)),
 	}
 }
 
@@ -90,6 +91,6 @@ func OpenIDConnectHybridFactory(config *Config, storage interface{}, strategy in
 		IDTokenHandleHelper: &openid.IDTokenHandleHelper{
 			IDTokenStrategy: strategy.(openid.OpenIDConnectTokenStrategy),
 		},
-		OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(config.AllowedPromptValues),
+		OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(config.AllowedPromptValues, strategy.(jwt.JWTStrategy)),
 	}
 }

compose/compose_strategy.go

@@ -33,6 +33,7 @@ import (
 type CommonStrategy struct {
 	oauth2.CoreStrategy
 	openid.OpenIDConnectTokenStrategy
+	jwt.JWTStrategy
 }
 
 func NewOAuth2HMACStrategy(config *Config, secret []byte) *oauth2.HMACSHAStrategy {

handler/openid/flow_explicit_auth_test.go

@@ -48,12 +48,16 @@ func TestExplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 
 	areq := fosite.NewAuthorizeRequest()
 
+	session := NewDefaultSession()
+	session.Claims.Subject = "foo"
+	areq.Session = session
+
 	h := &OpenIDConnectExplicitHandler{
 		OpenIDConnectRequestStorage: store,
 		IDTokenHandleHelper: &IDTokenHandleHelper{
 			IDTokenStrategy: j,
 		},
-		OpenIDConnectRequestValidator: NewOpenIDConnectRequestValidator(nil),
+		OpenIDConnectRequestValidator: NewOpenIDConnectRequestValidator(nil, j.RS256JWTStrategy),
 	}
 	for k, c := range []struct {
 		description string

handler/openid/flow_hybrid.go

@@ -60,15 +60,15 @@ func (c *OpenIDConnectHybridHandler) HandleAuthorizeEndpointRequest(ctx context.
 		return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use the id_token response type"))
 	}
 
-	if err := c.OpenIDConnectRequestValidator.ValidatePrompt(ar); err != nil {
-		return err
-	}
-
 	sess, ok := ar.GetSession().(Session)
 	if !ok {
 		return errors.WithStack(ErrInvalidSession)
 	}
 
+	if err := c.OpenIDConnectRequestValidator.ValidatePrompt(ar); err != nil {
+		return err
+	}
+
 	client := ar.GetClient()
 	for _, scope := range ar.GetRequestedScopes() {
 		if !c.ScopeStrategy(client.GetScopes(), scope) {

handler/openid/flow_hybrid_test.go

@@ -93,8 +93,9 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 			IDTokenStrategy: idStrategy,
 		},
 		ScopeStrategy:                 fosite.HierarchicScopeStrategy,
-		OpenIDConnectRequestValidator: NewOpenIDConnectRequestValidator(nil),
+		OpenIDConnectRequestValidator: NewOpenIDConnectRequestValidator(nil, j.RS256JWTStrategy),
 	}
+
 	for k, c := range []struct {
 		description string
 		setup       func()
@@ -133,12 +134,12 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 					ResponseTypes: fosite.Arguments{"token", "code", "id_token"},
 					Scopes:        []string{"openid"},
 				}
-				areq.Session = &defaultSession{
+				areq.Session = &DefaultSession{
 					Claims: &jwt.IDTokenClaims{
 						Subject: "peter",
 					},
-					Headers:        &jwt.Headers{},
-					DefaultSession: new(fosite.DefaultSession),
+					Headers: &jwt.Headers{},
+					Subject: "peter",
 				}
 			},
 			expectErr: fosite.ErrInvalidGrant,

handler/openid/flow_implicit.go

@@ -54,10 +54,6 @@ func (c *OpenIDConnectImplicitHandler) HandleAuthorizeEndpointRequest(ctx contex
 		return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use implicit grant type"))
 	}
 
-	if err := c.OpenIDConnectRequestValidator.ValidatePrompt(ar); err != nil {
-		return err
-	}
-
 	if ar.GetResponseTypes().Exact("id_token") && !ar.GetClient().GetResponseTypes().Has("id_token") {
 		return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use response type id_token"))
 	} else if ar.GetResponseTypes().Matches("token", "id_token") && !ar.GetClient().GetResponseTypes().Has("token", "id_token") {
@@ -76,6 +72,10 @@ func (c *OpenIDConnectImplicitHandler) HandleAuthorizeEndpointRequest(ctx contex
 		return errors.WithStack(ErrInvalidSession)
 	}
 
+	if err := c.OpenIDConnectRequestValidator.ValidatePrompt(ar); err != nil {
+		return err
+	}
+
 	claims := sess.IDTokenClaims()
 	if ar.GetResponseTypes().Has("token") {
 		if err := c.AuthorizeImplicitGrantTypeHandler.IssueImplicitAccessToken(ctx, ar, resp); err != nil {

handler/openid/flow_implicit_test.go

@@ -53,8 +53,9 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 			IDTokenStrategy: idStrategy,
 		},
 		ScopeStrategy:                 fosite.HierarchicScopeStrategy,
-		OpenIDConnectRequestValidator: NewOpenIDConnectRequestValidator(nil),
+		OpenIDConnectRequestValidator: NewOpenIDConnectRequestValidator(nil, j.RS256JWTStrategy),
 	}
+
 	for k, c := range []struct {
 		description string
 		setup       func()
@@ -132,6 +133,7 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 						Subject: "peter",
 					},
 					Headers: &jwt.Headers{},
+					Subject: "peter",
 				}
 				areq.Form.Add("nonce", "some-random-foo-nonce-wow")
 			},

handler/openid/strategy_jwt.go

@@ -132,12 +132,12 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req
 
 	sess, ok := requester.GetSession().(Session)
 	if !ok {
-		return "", errors.New("Failed to generate id token because session must be of type fosite/handler/openid.Session")
+		return "", errors.WithStack(fosite.ErrServerError.WithDebug("Failed to generate id token because session must be of type fosite/handler/openid.Session"))
 	}
 
 	claims := sess.IDTokenClaims()
 	if claims.Subject == "" {
-		return "", errors.New("Failed to generate id token because subject is an empty string")
+		return "", errors.WithStack(fosite.ErrServerError.WithDebug("Failed to generate id token because subject is an empty string"))
 	}
 
 	if requester.GetRequestForm().Get("grant_type") != "refresh_token" {
@@ -148,28 +148,28 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req
 
 		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")
+				return "", errors.WithStack(fosite.ErrServerError.WithDebug("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"))
+				return "", errors.WithStack(fosite.ErrServerError.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")
+				return "", errors.WithStack(fosite.ErrServerError.WithDebug("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"))
+				return "", errors.WithStack(fosite.ErrServerError.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"))
+				return "", errors.WithStack(fosite.ErrServerError.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
 		}
@@ -183,15 +183,15 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req
 		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())))
+				return "", errors.WithStack(fosite.ErrServerError.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"))
+				return "", errors.WithStack(fosite.ErrServerError.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"))
+				return "", errors.WithStack(fosite.ErrServerError.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")))
+				return "", errors.WithStack(fosite.ErrServerError.WithDebug(fmt.Sprintf("Subject from authorization mismatches id token subject from id_token_hint")))
 			}
 		}
 	}

handler/openid/validator.go

@@ -24,23 +24,30 @@ package openid
 import (
 	"fmt"
 
+	"strconv"
+	"time"
+
+	jwtgo "github.com/dgrijalva/jwt-go"
 	"github.com/ory/fosite"
+	"github.com/ory/fosite/token/jwt"
 	"github.com/ory/go-convenience/stringslice"
 	"github.com/ory/go-convenience/stringsx"
 	"github.com/pkg/errors"
 )
 
 type OpenIDConnectRequestValidator struct {
 	AllowedPrompt []string
+	Strategy      jwt.JWTStrategy
 }
 
-func NewOpenIDConnectRequestValidator(prompt []string) *OpenIDConnectRequestValidator {
+func NewOpenIDConnectRequestValidator(prompt []string, strategy jwt.JWTStrategy) *OpenIDConnectRequestValidator {
 	if len(prompt) == 0 {
 		prompt = []string{"login", "none", "consent", "select_account"}
 	}
 
 	return &OpenIDConnectRequestValidator{
 		AllowedPrompt: prompt,
+		Strategy:      strategy,
 	}
 }
 
@@ -85,6 +92,63 @@ func (v *OpenIDConnectRequestValidator) ValidatePrompt(req fosite.AuthorizeReque
 		return errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Parameter prompt was set to none, but contains other values as well which is not allowed"))
 	}
 
+	maxAge, err := strconv.ParseInt(req.GetRequestForm().Get("max_age"), 10, 64)
+	if err != nil {
+		maxAge = 0
+	}
+
+	session, ok := req.GetSession().(Session)
+	if !ok {
+		return errors.WithStack(fosite.ErrServerError.WithDebug("Failed to validate OpenID Connect request because session is not of type fosite/handler/openid.Session"))
+	}
+
+	claims := session.IDTokenClaims()
+	if claims.Subject == "" {
+		return errors.WithStack(fosite.ErrServerError.WithDebug("Failed to validate OpenID Connect request because session subject is empty"))
+	}
+
+	if maxAge > 0 {
+		if claims.AuthTime.IsZero() || claims.AuthTime.After(time.Now()) {
+			return errors.WithStack(fosite.ErrServerError.WithDebug("Failed to validate OpenID Connect request 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 validate OpenID Connect request because authentication time does not satisfy max_age time"))
+		}
+	}
+
+	if stringslice.Has(prompt, "none") {
+		if claims.AuthTime.IsZero() {
+			return errors.WithStack(fosite.ErrServerError.WithDebug("Failed to validate OpenID Connect request because because auth_time is missing from session"))
+		}
+		if claims.AuthTime.After(claims.RequestedAt) {
+			return errors.WithStack(fosite.ErrLoginRequired.WithDebug("Failed to validate OpenID Connect request 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"))
+		}
+	}
+
+	if stringslice.Has(prompt, "login") {
+		if claims.AuthTime.Before(claims.RequestedAt) {
+			return errors.WithStack(fosite.ErrLoginRequired.WithDebug("Failed to validate OpenID Connect request 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"))
+		}
+	}
+
+	idTokenHint := req.GetRequestForm().Get("id_token_hint")
+	if idTokenHint == "" {
+
+		return nil
+	}
+
+	tokenHint, err := v.Strategy.Decode(idTokenHint)
+	if err != nil {
+		return errors.WithStack(fosite.ErrInvalidRequest.WithDebug(fmt.Sprintf("Failed to validate OpenID Connect request as decoding id token from id_token_hint parameter failed because %s", err.Error())))
+	}
+
+	if hintClaims, ok := tokenHint.Claims.(jwtgo.MapClaims); !ok {
+		return errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Failed to validate OpenID Connect request as decoding id token from id_token_hint to *jwt.StandardClaims failed"))
+	} else if hintSub, _ := hintClaims["sub"].(string); hintSub == "" {
+		return errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Failed to validate OpenID Connect request because provided id token from id_token_hint does not have a subject"))
+	} else if hintSub != claims.Subject || hintSub != session.GetSubject() {
+		return errors.WithStack(fosite.ErrLoginRequired.WithDebug(fmt.Sprintf("Failed to validate OpenID Connect request because subject from session does not subject from id_token_hint")))
+	}
+
 	return nil
 }