Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JWT/JWKS authentication with support for Auth0/Okta #912

Closed
burn2delete opened this issue Apr 25, 2022 · 11 comments · Fixed by #2348, #2500 or #2540
Closed

JWT/JWKS authentication with support for Auth0/Okta #912

burn2delete opened this issue Apr 25, 2022 · 11 comments · Fixed by #2348, #2500 or #2540
Assignees

Comments

@burn2delete
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently there is no built-in plugin to validate JWT's within the router that use JWKS. This means all security and JWT validation must happen on each subgraph. Moving this validation logic to the router can help reduce the load of subgraphs by rejecting invalid requests earlier, before they are allowed into the network.

Describe the solution you'd like
Ideally full support for Auth0/Okta via JWT/JWKS and restricted queries/mutations by roles/scopes.

Describe alternatives you've considered
Handle all authentication/authorization logic within subgraphs.

Additional context
Auth0/Okta are both widely used by developers, adding support to the Apollo Router will provide support for these solutions out-of-the-box.

@lleadbet
Copy link
Contributor

Problem

Early adopters of the router will often need to replicate existing custom logic from their apollo-gateway instances, and one piece that will be built across a number of teams will be AuthN-related.

Solution

In order to help accelerate users' development (or remove entirely), we should build support for validating JWKS-backed JWTs for those utilizing that standard as well as providing a reference implementation of an example of an asynchronous AuthN plugin similar to the existing jwt-auth example plugin.

Having either an out of the box solution, or one that is easily customized, will help users migrate to the more performant router and avoid duplicative work.

Why JWKS support?

JWKS, or JSON Web Key Sets, is a standard format for validating incoming JWTs without an explicitly defined shared secret. Given the standardization, it is possible to be used with a number of AuthN providers out of the box, including Auth0 and Okta.

Additionally, given the requirements when using JWKS around refetching, it would provide an example of authentication with asynchronous requirements.

Requirements

At the resolution of this issue, there should be a plugin that supports the following:

  • JWKS endpoint entry and validation
  • JWT validation against the provided JWKS
  • Ability to deny requests with invalid/expired JWTs
  • Configurable header location and prefixes, defaulting to Authorization and prefix Bearer
  • Support optional authorization, delegating to subgraphs for validation as needed

@andrew-kolesnikov
Copy link

andrew-kolesnikov commented Jun 14, 2022

Is someone working on this plugin already? If not, we're most likely going to have to implement this soon for our organization.

In our particular scenario, forwarding incoming Bearer tokens to be validated using JWKS downstream at the subgraph level isn't possible. The original JWT that the router receives must be validated using JWKS, but the router and subgraph are connected using a router-signed JWT with custom claims to send context over into the subgraph.

@bnjjj
Copy link
Contributor

bnjjj commented Jun 16, 2022

No one is currently working on it. If you have questions or need help feel free to ask :)

@lleadbet
Copy link
Contributor

@andrew-kolesnikov - apologies for the delay on this.

I have a rough example here: https://github.com/apollosolutions/jwks-router-plugin

Make sure to note that this isn't meant for production, but does provide an example implementation of JWKS support w/in the router.

@andrew-kolesnikov
Copy link

Thank you @lleadbet for posting this - it's been a while so we ended up writing our own plugin, but this is surely going to help a lot of folks out there!

@lleadbet
Copy link
Contributor

Glad to hear you got something built up! Let us know if you have any questions about plugins and such- always happy to help.

@andrew-kolesnikov
Copy link

Thanks @lleadbet! Our current challenge is porting query cost calculations from nodejs - we're looking for something like https://github.com/slicknode/graphql-query-complexity for the Rust router. That is something that I think could benefit a lot of folks too, so any suggestions would be much appreciated.

@lleadbet
Copy link
Contributor

@andrew-kolesnikov - do you mind filing a GH issue with more details on what you'd like to see? Might be useful to spin out that conversation outside the context of this specific plugin ask. :)

@prasek
Copy link
Contributor

prasek commented Sep 29, 2022

related community thread

@abernix
Copy link
Member

abernix commented Nov 22, 2022

Does anyone have any feedback on @lleadbet's solution here they'd be interested in sharing? We're curious about making this part of the Router directly, but would appreciate additional feedback to inform the final shape!

@andrew-kolesnikov
Copy link

andrew-kolesnikov commented Nov 22, 2022

@abernix That example is really close to the plugin I ended up deploying for my needs, pretty good starting point in case someone's looking to add this into their router setup too

@garypen garypen self-assigned this Jan 4, 2023
@abernix abernix removed the triage label Jan 13, 2023
garypen added a commit that referenced this issue Jan 18, 2023
fixes: #912

Tasks:
 - [x] Configuration (to be refined)
 - [x] JWKS lazy, deduplicated retrieval (net or disk)
 - [x] Header parsing
 - [x] Header name/prefix customization
 - [x] JWKS cooldown logic
 - [x] JWKS cache clearing
 - [x] key retention
 - [x] Validation
 - [x] Claim Validation
 - [x] Claim propagation via context
 - [x] Add a rhai example interacting with claims
 - [x] testing
 - [x] documentation

Co-authored-by: Stephen Barlow <stephen@apollographql.com>
This was referenced Jan 20, 2023
garypen added a commit that referenced this issue Jan 31, 2023
IMPORTANT: Merging this PR without resolving the "alg" question below.
The "alg" question will potentially be resolved by a follow-up PR.

- Fix #912

This modifies the experimental JWT authentication support so that
multiple JWKS urls are now supported.

The implementation is draft; pending a review and feedback with product.

In addition to adding multiple JWKS we need to resolve the following
issue:

### Not all JWKS entries contain "alg".

Currently, if the router can't find "alg" in the JWKS. Then it will fail
the request. The JWKS spec notes that "alg" is optional, but if we don't
know what the "alg" is, then we can't decode the JWT.

Here are some alternatives:
-  Preserve the existing behaviour (i.e. fail if "alg" isn't specified)
- Look for "alg" in the JWT header and use that value if not found in
JWKSs
-  Allow "alg" (per IDP) to be specified in configuration either as
    -  A fallback or
    -  An override

Which of the above would users prefer?

**Checklist**

Complete the checklist (and note appropriate exceptions) before a final
PR is raised.

~- [ ] Changes are compatible[^1]~
- [x] Documentation[^2] completed
- [x] Performance impact assessed and acceptable
- Tests added and passing[^3]
    - [x] Unit Tests
    ~- [ ] Integration Tests~
    ~- [ ] Manual Tests~

**Exceptions**

The change is to experimental configuration and not compatible.

**Notes**

[^1]. It may be appropriate to bring upcoming changes to the attention
of other (impacted) groups. Please endeavour to do this before seeking
PR approval. The mechanism for doing this will vary considerably, so use
your judgement as to how and when to do this.
[^2]. Configuration is an important part of many changes. Where
applicable please try to document configuration examples.
[^3]. Tick whichever testing boxes are applicable. If you are adding
Manual Tests:
- please document the manual testing (extensively) in the Exceptions.
- please raise a separate issue to automate the test and label it (or
ask for it to be labeled) as `manual test`
garypen added a commit that referenced this issue Feb 9, 2023
- Fix #912

Many Identity Providers (IdP) don't specify "alg" in the keys of their
provided JWKSets. For example: Azure, Ping, etc...

This is fine as it isn't part of the specification. However, it provides
a challenge when it comes to JWT verification, because we need to know
which algorithm to use to verify the JWT.

This enhancement adds support for searching through the available
JWKSets in the router, trying to find the most suitable key for JWT
verification. Ideally, we will have a key which has both a matching
"kid" and an "alg" which we can use for verification. In the absence of
either or both of those, we may still be able to find a key which we can
use to verify our JWT as follows:
- If we find a key which has a matching "kid" and the correct family of
algorithm we will favour that first.
- If we find a key which has the correct family of algorithm, we will
favour that second.

**Checklist**

Complete the checklist (and note appropriate exceptions) before a final
PR is raised.

- [x] Changes are compatible[^1]
- [x] Documentation[^2] completed
- [x] Performance impact assessed and acceptable
- Tests added and passing[^3]
    - [x] Unit Tests
    ~- [ ] Integration Tests~
    ~- [ ] Manual Tests~

**Exceptions**

*Note any exceptions here*

**Notes**

[^1]. It may be appropriate to bring upcoming changes to the attention
of other (impacted) groups. Please endeavour to do this before seeking
PR approval. The mechanism for doing this will vary considerably, so use
your judgement as to how and when to do this.
[^2]. Configuration is an important part of many changes. Where
applicable please try to document configuration examples.
[^3]. Tick whichever testing boxes are applicable. If you are adding
Manual Tests:
- please document the manual testing (extensively) in the Exceptions.
- please raise a separate issue to automate the test and label it (or
ask for it to be labeled) as `manual test`
BrynCooke pushed a commit that referenced this issue Feb 10, 2023
- Fix #912

Many Identity Providers (IdP) don't specify "alg" in the keys of their
provided JWKSets. For example: Azure, Ping, etc...

This is fine as it isn't part of the specification. However, it provides
a challenge when it comes to JWT verification, because we need to know
which algorithm to use to verify the JWT.

This enhancement adds support for searching through the available
JWKSets in the router, trying to find the most suitable key for JWT
verification. Ideally, we will have a key which has both a matching
"kid" and an "alg" which we can use for verification. In the absence of
either or both of those, we may still be able to find a key which we can
use to verify our JWT as follows:
- If we find a key which has a matching "kid" and the correct family of
algorithm we will favour that first.
- If we find a key which has the correct family of algorithm, we will
favour that second.

**Checklist**

Complete the checklist (and note appropriate exceptions) before a final
PR is raised.

- [x] Changes are compatible[^1]
- [x] Documentation[^2] completed
- [x] Performance impact assessed and acceptable
- Tests added and passing[^3]
    - [x] Unit Tests
    ~- [ ] Integration Tests~
    ~- [ ] Manual Tests~

**Exceptions**

*Note any exceptions here*

**Notes**

[^1]. It may be appropriate to bring upcoming changes to the attention
of other (impacted) groups. Please endeavour to do this before seeking
PR approval. The mechanism for doing this will vary considerably, so use
your judgement as to how and when to do this.
[^2]. Configuration is an important part of many changes. Where
applicable please try to document configuration examples.
[^3]. Tick whichever testing boxes are applicable. If you are adding
Manual Tests:
- please document the manual testing (extensively) in the Exceptions.
- please raise a separate issue to automate the test and label it (or
ask for it to be labeled) as `manual test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment