Jwt client authn #291

madaster97 · 2021-10-24T21:29:21Z

Description

This PR adds JWT based client authentication to the library (building on Adam's initial implementation). All signing algorithms supported by jose@2.0.5 will be supported here (though upgrading jose versions is an option). This PR also adds a configurable JWKS endpoint, so that the public keys associated with the client can be distributed.

References

Following up on my issue for this enhancement.
GitHub Project where I documented the additions

Testing

There's an example JWT client that @adamjmcgrath already added. I've been using that to test, but I've been having issues getting it to work against the fixture provider. I think the run_example.js script is accidentally setting up a client_secret client instead of a jwt one. I've been testing against an example Openid Provider that supports private_key_jwt clients and it's worked so far.

This change adds test coverage for new/changed/fixed functionality

TBD on testing

[] Adding testing for the JWKS endpoint

Checklist

I have added documentation for new/changed functionality in this PR or in auth0.com/docs
All active GitHub checks for tests, formatting, and security are passing
The correct base branch is being used, if not master

adamjmcgrath · 2021-10-25T14:48:41Z

Thanks for your work @madaster97 - will take a look through this sometime this week

adamjmcgrath · 2021-11-19T10:28:09Z

Hi @madaster97 - sorry, I haven't forgotten this. It might be a little while before I get round to reviewing it

stale · 2022-03-02T09:47:01Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you have not received a response for our team (apologies for the delay) and this is still a blocker, please reply with additional information or just a ping. Thank you for your contribution! 🙇‍♂️

adamjmcgrath · 2022-03-02T10:49:09Z

Hi @madaster97 - we do have this in the Roadmap to look at at some point over the next few months, so will reopen this when we do

adamjmcgrath

@madaster97 - left some comments

Going to be working on this in a week or so, so happy to take this branch over if you're busy with something else.

adamjmcgrath · 2022-06-30T11:15:36Z

middleware/auth.js

@@ -58,6 +59,23 @@ const auth = function (params) {
    debug('logout handling route not applied');
  }

+  // jwks url route, configured with routes.jwksUrl


What's the benefit of publishing a jwksUri endpoint?

Also clientAssertionConfig.signingKey is a private key, so you don't want to make it publicly accessible

adamjmcgrath · 2022-06-30T11:16:58Z

examples/jwt-client-authn.js

@@ -13,9 +13,12 @@ app.use(
      response_type: 'code',
    },
    clientAuthMethod: 'private_key_jwt',
-    clientAssertionSigningKey: fs.readFileSync(
+    clientAssertionConfig: {


We don't need to specify the alg if we're not publishing a jwks uri

@adamjmcgrath - There are some cases where you need to explicitly provide an alg. For instance you can use an RSA private key with any of RS 256/384/512, and somewhere along the way openid-client or jose are defaulting to RS256 for an RSA private key.

I can think of two options to let callers specify their alg:

Keep the current setup, noting that we're passing the alg to openid-client here after parsing with jose's JWK module

Explicitly require a jwk and inspect the alg (maybe warning if it's missing and that we'll choose a default)

Note that openid-clilent won't throw an error for invalid algs until runtime. For example, if I try to specify ES384 but give an RSA private key I get the following, but not until I actually attempt a grant:

BadRequestError: no key found in client jwks to sign a client assertion with using alg ES384 at C:\Users\adams\source\node\express-openid-connect\middleware\auth.js:138:31 at processTicksAndRejections (node:internal/process/task_queues:96:5)

What do you think?

Hey @madaster97 - sorry for the delay responding, I was away last week

Your right, we should allow specifying the algorithm, but it should be optional

By default openid-client uses one of the token_endpoint_auth_signing_alg_values_supported if you don't specify - so I just need to figure out whether the default should be RS256 or undefined (let openid-client pick one from token_endpoint_auth_signing_alg_values_supported)

Also, can we make the config flat,eg clientAssertionSigningKey and clientAssertionSigningKeyAlg (like idTokenSigningAlg)

adamjmcgrath · 2022-06-30T11:17:07Z

end-to-end/fixture/oidc-provider.js

@@ -17,6 +17,7 @@ const config = {
  clients: [
    client,
    Object.assign({}, client, {
+      client_secret: undefined,


This shouldn't be necessary

adamjmcgrath · 2022-06-30T11:20:47Z

lib/client.js

+  }
+
+  if (config.clientAuthMethod == 'private_key_jwt') {
+    const configTokenAuthAlg = config.clientAssertionConfig.signingKey;


This would be signingAlg - but I would rather avoid requiring the user to specify this if we can

adamjmcgrath · 2022-06-30T11:26:25Z

lib/config.js

+        is: true,
+        then: Joi.string().invalid('none').messages({
+          'any.only':
+            '"clientAuthMethod" cannot be "none" when "response_type" includes "code"',


I want to default clientAuthMethod from clientAssertionSigningKey or clientSecret - so this should be

'either a "clientSecret" or "clientAuthMethod" is required for a "response_type" that includes "code"'

madaster97 · 2022-07-01T02:20:14Z

Thanks @adamjmcgrath - Yeah I'm looking into these comments. Right now just trying to get re-setup for testing

adamjmcgrath · 2022-07-12T11:22:39Z

lib/client.js

+        };
+      case 'client_secret_basic':
+      case 'client_secret_post':
+        return { client_secret: config.clientSecret };


Also , I think there's a use case for a client secret even when using private_key_jwt (when the ID Token signing alg is HS256) but I will confirm

adamjmcgrath · 2022-07-14T13:55:30Z

Thanks for your work on this @madaster97 - I'm going to continue it now on the jwt-client-authn branch

madaster97 · 2022-07-14T14:09:59Z

Thanks @adamjmcgrath - I didn't respond on your other comments but I agree with all but 1. Currently the jwks URL endpoint (which I agree we can take out) is explicitly only showing the public components of the key using toJWK(false):

        JWK.asKey(config.clientAssertionConfig.signingKey, {
          alg: config.clientAssertionConfig.signingAlg,
          use: 'sig',
        }).toJWK(false)

adamjmcgrath · 2022-07-14T15:06:42Z

toJWK(false)

Ah yep, gotcha, thanks

Apologies for taking ages reviewing then being in a rush to merge, this is just because of how our work is prioritised behind the scenes.

I'll add you to the PR when I raise the final one against master in case you want to take a look

madaster97 added 7 commits October 24, 2021 13:50

Split signing key into config object props

7df4ff6

Update example

be361c6

Add issuer support checks

6a6b4e8

Remove client secret for jwt client fixture

8d919eb

Edit config and unit tests for client secrets

9f52788

Add jwks endpoint with use/alg params

86da91b

Add jwksUrl to types and lint

da18174

madaster97 requested a review from a team as a code owner October 24, 2021 21:29

stale bot added the closed:stale Issue or PR has not seen activity recently label Mar 2, 2022

stale bot removed the closed:stale Issue or PR has not seen activity recently label Mar 2, 2022

adamjmcgrath closed this Mar 2, 2022

adamjmcgrath reopened this Jun 30, 2022

adamjmcgrath reviewed Jun 30, 2022

View reviewed changes

adamjmcgrath reviewed Jul 12, 2022

View reviewed changes

adamjmcgrath merged commit ead1608 into auth0:jwt-client-authn Jul 14, 2022

madaster97 deleted the jwt-client-authn branch July 15, 2022 00:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jwt client authn #291

Jwt client authn #291

madaster97 commented Oct 24, 2021 •

edited

Loading

adamjmcgrath commented Oct 25, 2021 •

edited

Loading

adamjmcgrath commented Nov 19, 2021

stale bot commented Mar 2, 2022

adamjmcgrath commented Mar 2, 2022

adamjmcgrath left a comment

adamjmcgrath Jun 30, 2022

adamjmcgrath Jun 30, 2022

madaster97 Jul 3, 2022

adamjmcgrath Jul 12, 2022

adamjmcgrath Jun 30, 2022

adamjmcgrath Jun 30, 2022

adamjmcgrath Jun 30, 2022

madaster97 commented Jul 1, 2022

adamjmcgrath Jul 12, 2022

adamjmcgrath commented Jul 14, 2022

madaster97 commented Jul 14, 2022

adamjmcgrath commented Jul 14, 2022 •

edited

Loading

Jwt client authn #291

Jwt client authn #291

Conversation

madaster97 commented Oct 24, 2021 • edited Loading

Description

References

Testing

TBD on testing

Checklist

adamjmcgrath commented Oct 25, 2021 • edited Loading

adamjmcgrath commented Nov 19, 2021

stale bot commented Mar 2, 2022

adamjmcgrath commented Mar 2, 2022

adamjmcgrath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madaster97 commented Jul 1, 2022

Choose a reason for hiding this comment

adamjmcgrath commented Jul 14, 2022

madaster97 commented Jul 14, 2022

adamjmcgrath commented Jul 14, 2022 • edited Loading

madaster97 commented Oct 24, 2021 •

edited

Loading

adamjmcgrath commented Oct 25, 2021 •

edited

Loading

adamjmcgrath commented Jul 14, 2022 •

edited

Loading