feat: add support for urn:ietf:params:oauth:grant-type:jwt-bearer grant type RFC 7523� (#560)

Closes #546
Closes #305

Co-authored-by: Vladimir Kalugin <v.p.kalugin@tinkoff.ru>
Co-authored-by: i.seliverstov <i.seliverstov@tinkoff.ru>

Author: 3 people

.circleci/config.yml

@@ -3,7 +3,7 @@ version: 2.1
 orbs:
   changelog: ory/changelog@0.1.4
   nancy: ory/nancy@0.0.13
-  golangci: ory/golangci@0.0.9
+  golangci: ory/golangci@0.0.11
 
 jobs:
   test:

access_request_handler.go

@@ -57,7 +57,6 @@ import (
 //   client MUST authenticate with the authorization server as described
 //   in Section 3.2.1.
 func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session Session) (AccessRequester, error) {
-	var err error
 	accessRequest := NewAccessRequest(session)
 
 	if r.Method != "POST" {
@@ -80,18 +79,34 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
 		return accessRequest, errorsx.WithStack(ErrInvalidRequest.WithHint("Request parameter 'grant_type' is missing"))
 	}
 
-	client, err := f.AuthenticateClient(ctx, r, r.PostForm)
-	if err != nil {
-		return accessRequest, err
+	client, clientErr := f.AuthenticateClient(ctx, r, r.PostForm)
+	if clientErr == nil {
+		accessRequest.Client = client
 	}
-	accessRequest.Client = client
 
 	var found = false
 	for _, loader := range f.TokenEndpointHandlers {
+		// Is the loader responsible for handling the request?
+		if !loader.CanHandleTokenEndpointRequest(accessRequest) {
+			continue
+		}
+
+		// The handler **is** responsible!
+
+		// Is the client supplied in the request? If not can this handler skip client auth?
+		if !loader.CanSkipClientAuth(accessRequest) && clientErr != nil {
+			// No client and handler can not skip client auth -> error.
+			return accessRequest, clientErr
+		}
+
+		// All good.
 		if err := loader.HandleTokenEndpointRequest(ctx, accessRequest); err == nil {
 			found = true
 		} else if errors.Is(err, ErrUnknownRequest) {
-			// do nothing
+			// This is a duplicate because it should already have been handled by
+			// `loader.CanHandleTokenEndpointRequest(accessRequest)` but let's keep it for sanity.
+			//
+			continue
 		} else if err != nil {
 			return accessRequest, err
 		}

access_request_handler_test.go

@@ -42,6 +42,8 @@ func TestNewAccessRequest(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	store := internal.NewMockStorage(ctrl)
 	handler := internal.NewMockTokenEndpointHandler(ctrl)
+	handler.EXPECT().CanHandleTokenEndpointRequest(gomock.Any()).Return(true).AnyTimes()
+	handler.EXPECT().CanSkipClientAuth(gomock.Any()).Return(false).AnyTimes()
 	hasher := internal.NewMockHasher(ctrl)
 	defer ctrl.Finish()
 
@@ -94,6 +96,7 @@ func TestNewAccessRequest(t *testing.T) {
 			mock: func() {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(nil, errors.New(""))
 			},
+			handlers: TokenEndpointHandlers{handler},
 		},
 		{
 			header: http.Header{
@@ -118,6 +121,7 @@ func TestNewAccessRequest(t *testing.T) {
 			mock: func() {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(nil, errors.New(""))
 			},
+			handlers: TokenEndpointHandlers{handler},
 		},
 		{
 			header: http.Header{
@@ -134,6 +138,7 @@ func TestNewAccessRequest(t *testing.T) {
 				client.Secret = []byte("foo")
 				hasher.EXPECT().Compare(context.TODO(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(errors.New(""))
 			},
+			handlers: TokenEndpointHandlers{handler},
 		},
 		{
 			header: http.Header{
@@ -221,6 +226,239 @@ func TestNewAccessRequest(t *testing.T) {
 	}
 }
 
+func TestNewAccessRequestWithoutClientAuth(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	store := internal.NewMockStorage(ctrl)
+	handler := internal.NewMockTokenEndpointHandler(ctrl)
+	handler.EXPECT().CanHandleTokenEndpointRequest(gomock.Any()).Return(true).AnyTimes()
+	handler.EXPECT().CanSkipClientAuth(gomock.Any()).Return(true).AnyTimes()
+	hasher := internal.NewMockHasher(ctrl)
+	defer ctrl.Finish()
+
+	client := &DefaultClient{}
+	anotherClient := &DefaultClient{ID: "another"}
+	fosite := &Fosite{Store: store, Hasher: hasher, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}
+	for k, c := range []struct {
+		header    http.Header
+		form      url.Values
+		mock      func()
+		method    string
+		expectErr error
+		expect    *AccessRequest
+		handlers  TokenEndpointHandlers
+	}{
+		// No grant type -> error
+		{
+			form: url.Values{},
+			mock: func() {
+				store.EXPECT().GetClient(gomock.Any(), gomock.Any()).Times(0)
+			},
+			method:    "POST",
+			expectErr: ErrInvalidRequest,
+		},
+		// No registered handlers -> error
+		{
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				store.EXPECT().GetClient(gomock.Any(), gomock.Any()).Times(0)
+			},
+			method:    "POST",
+			expectErr: ErrInvalidRequest,
+			handlers:  TokenEndpointHandlers{},
+		},
+		// Handler can skip client auth and ignores missing client.
+		{
+			header: http.Header{
+				"Authorization": {basicAuth("foo", "bar")},
+			},
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				// despite error from storage, we should success, because client auth is not required
+				store.EXPECT().GetClient(gomock.Any(), "foo").Return(nil, errors.New("no client")).Times(1)
+				handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+			},
+			method: "POST",
+			expect: &AccessRequest{
+				GrantTypes: Arguments{"foo"},
+				Request: Request{
+					Client: client,
+				},
+			},
+			handlers: TokenEndpointHandlers{handler},
+		},
+		// Should pass if no auth is set in the header and can skip!
+		{
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+			},
+			method: "POST",
+			expect: &AccessRequest{
+				GrantTypes: Arguments{"foo"},
+				Request: Request{
+					Client: client,
+				},
+			},
+			handlers: TokenEndpointHandlers{handler},
+		},
+		// Should also pass if client auth is set!
+		{
+			header: http.Header{
+				"Authorization": {basicAuth("foo", "bar")},
+			},
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				store.EXPECT().GetClient(gomock.Any(), "foo").Return(anotherClient, nil).Times(1)
+				hasher.EXPECT().Compare(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
+				handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+			},
+			method: "POST",
+			expect: &AccessRequest{
+				GrantTypes: Arguments{"foo"},
+				Request: Request{
+					Client: anotherClient,
+				},
+			},
+			handlers: TokenEndpointHandlers{handler},
+		},
+	} {
+		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+			r := &http.Request{
+				Header:   c.header,
+				PostForm: c.form,
+				Form:     c.form,
+				Method:   c.method,
+			}
+			c.mock()
+			ctx := NewContext()
+			fosite.TokenEndpointHandlers = c.handlers
+			ar, err := fosite.NewAccessRequest(ctx, r, new(DefaultSession))
+
+			if c.expectErr != nil {
+				assert.EqualError(t, err, c.expectErr.Error())
+			} else {
+				require.NoError(t, err)
+				AssertObjectKeysEqual(t, c.expect, ar, "GrantTypes", "Client")
+				assert.NotNil(t, ar.GetRequestedAt())
+			}
+		})
+	}
+}
+
+// In this test case one handler requires client auth and another handler not.
+func TestNewAccessRequestWithMixedClientAuth(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	store := internal.NewMockStorage(ctrl)
+
+	handlerWithClientAuth := internal.NewMockTokenEndpointHandler(ctrl)
+	handlerWithClientAuth.EXPECT().CanHandleTokenEndpointRequest(gomock.Any()).Return(true).AnyTimes()
+	handlerWithClientAuth.EXPECT().CanSkipClientAuth(gomock.Any()).Return(false).AnyTimes()
+
+	handlerWithoutClientAuth := internal.NewMockTokenEndpointHandler(ctrl)
+	handlerWithoutClientAuth.EXPECT().CanHandleTokenEndpointRequest(gomock.Any()).Return(true).AnyTimes()
+	handlerWithoutClientAuth.EXPECT().CanSkipClientAuth(gomock.Any()).Return(true).AnyTimes()
+
+	hasher := internal.NewMockHasher(ctrl)
+	defer ctrl.Finish()
+
+	client := &DefaultClient{}
+	fosite := &Fosite{Store: store, Hasher: hasher, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}
+	for k, c := range []struct {
+		header    http.Header
+		form      url.Values
+		mock      func()
+		method    string
+		expectErr error
+		expect    *AccessRequest
+		handlers  TokenEndpointHandlers
+	}{
+		{
+			header: http.Header{
+				"Authorization": {basicAuth("foo", "bar")},
+			},
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
+				client.Public = false
+				client.Secret = []byte("foo")
+				hasher.EXPECT().Compare(context.TODO(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(errors.New("hash err"))
+				handlerWithoutClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+			},
+			method:    "POST",
+			expectErr: ErrInvalidClient,
+			handlers:  TokenEndpointHandlers{handlerWithoutClientAuth, handlerWithClientAuth},
+		},
+		{
+			header: http.Header{
+				"Authorization": {basicAuth("foo", "bar")},
+			},
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
+				client.Public = false
+				client.Secret = []byte("foo")
+				hasher.EXPECT().Compare(context.TODO(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
+				handlerWithoutClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+				handlerWithClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+			},
+			method: "POST",
+			expect: &AccessRequest{
+				GrantTypes: Arguments{"foo"},
+				Request: Request{
+					Client: client,
+				},
+			},
+			handlers: TokenEndpointHandlers{handlerWithoutClientAuth, handlerWithClientAuth},
+		},
+		{
+			header: http.Header{},
+			form: url.Values{
+				"grant_type": {"foo"},
+			},
+			mock: func() {
+				store.EXPECT().GetClient(gomock.Any(), gomock.Any()).Times(0)
+				handlerWithoutClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
+			},
+			method:    "POST",
+			expectErr: ErrInvalidRequest,
+			handlers:  TokenEndpointHandlers{handlerWithoutClientAuth, handlerWithClientAuth},
+		},
+	} {
+		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+			r := &http.Request{
+				Header:   c.header,
+				PostForm: c.form,
+				Form:     c.form,
+				Method:   c.method,
+			}
+			c.mock()
+			ctx := NewContext()
+			fosite.TokenEndpointHandlers = c.handlers
+			ar, err := fosite.NewAccessRequest(ctx, r, new(DefaultSession))
+
+			if c.expectErr != nil {
+				assert.EqualError(t, err, c.expectErr.Error())
+			} else {
+				require.NoError(t, err)
+				AssertObjectKeysEqual(t, c.expect, ar, "GrantTypes", "Client")
+				assert.NotNil(t, ar.GetRequestedAt())
+			}
+		})
+	}
+}
+
 func basicAuth(username, password string) string {
 	return "Basic " + base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", username, password)))
 }

compose/compose.go

@@ -111,6 +111,7 @@ func ComposeAllEnabled(config *Config, storage interface{}, secret []byte, key *
 		OAuth2ClientCredentialsGrantFactory,
 		OAuth2RefreshTokenGrantFactory,
 		OAuth2ResourceOwnerPasswordCredentialsFactory,
+		RFC7523AssertionGrantFactory,
 
 		OpenIDConnectExplicitFactory,
 		OpenIDConnectImplicitFactory,

compose/compose_rfc7523.go

@@ -0,0 +1,26 @@
+package compose
+
+import (
+	"github.com/ory/fosite/handler/oauth2"
+	"github.com/ory/fosite/handler/rfc7523"
+)
+
+// RFC7523AssertionGrantFactory creates an OAuth2 Authorize JWT Grant (using JWTs as Authorization Grants) handler
+// and registers an access token, refresh token and authorize code validator.
+func RFC7523AssertionGrantFactory(config *Config, storage interface{}, strategy interface{}) interface{} {
+	return &rfc7523.Handler{
+		Storage:                  storage.(rfc7523.RFC7523KeyStorage),
+		ScopeStrategy:            config.GetScopeStrategy(),
+		AudienceMatchingStrategy: config.GetAudienceStrategy(),
+		TokenURL:                 config.TokenURL,
+		SkipClientAuth:           config.GrantTypeJWTBearerCanSkipClientAuth,
+		JWTIDOptional:            config.GrantTypeJWTBearerIDOptional,
+		JWTIssuedDateOptional:    config.GrantTypeJWTBearerIssuedDateOptional,
+		JWTMaxDuration:           config.GetJWTMaxDuration(),
+		HandleHelper: &oauth2.HandleHelper{
+			AccessTokenStrategy: strategy.(oauth2.AccessTokenStrategy),
+			AccessTokenStorage:  storage.(oauth2.AccessTokenStorage),
+			AccessTokenLifespan: config.GetAccessTokenLifespan(),
+		},
+	}
+}

compose/config.go

@@ -99,6 +99,18 @@ type Config struct {
 	// UseLegacyErrorFormat controls whether the legacy error format (with `error_debug`, `error_hint`, ...)
 	// should be used or not.
 	UseLegacyErrorFormat bool
+
+	// GrantTypeJWTBearerCanSkipClientAuth indicates, if client authentication can be skipped, when using jwt as assertion.
+	GrantTypeJWTBearerCanSkipClientAuth bool
+
+	// GrantTypeJWTBearerIDOptional indicates, if jti (JWT ID) claim required or not in JWT.
+	GrantTypeJWTBearerIDOptional bool
+
+	// GrantTypeJWTBearerIssuedDateOptional indicates, if "iat" (issued at) claim required or not in JWT.
+	GrantTypeJWTBearerIssuedDateOptional bool
+
+	// GrantTypeJWTBearerMaxDuration sets the maximum time after JWT issued date, during which the JWT is considered valid.
+	GrantTypeJWTBearerMaxDuration time.Duration
 }
 
 // GetScopeStrategy returns the scope strategy to be used. Defaults to glob scope strategy.
@@ -198,3 +210,14 @@ func (c *Config) GetMinParameterEntropy() int {
 		return c.MinParameterEntropy
 	}
 }
+
+// GetJWTMaxDuration specified the maximum amount of allowed `exp` time for a JWT. It compares
+// the time with the JWT's `exp` time if the JWT time is larger, will cause the JWT to be invalid.
+//
+// Defaults to a day.
+func (c *Config) GetJWTMaxDuration() time.Duration {
+	if c.GrantTypeJWTBearerMaxDuration == 0 {
+		return time.Hour * 24
+	}
+	return c.GrantTypeJWTBearerMaxDuration
+}