feat: add jwks access type to external token handler #24681

ryan-hanchett · 2024-05-07T22:08:37Z

Hey, I just made a Pull Request!

Re-opening from #24679 after I got into some trouble with git.

This pull request builds on BEP-07 to expand the ExternalTokenHandler to support a configured JWKS auth strategy. This allows external callers to use 3rd party authentication via JWKS, enabling greater flexibility in providing ways to auth with Backstage via signed tokens.

A potential area of improvement for future PRs would be to move the TokenFactory used in the tests into the backend-test-utils plugin, since it is used with near-identical patterns in the jwks.test.ts file, as well as plugins/auth-node/src/identity/DefaultIdentityClient.test.ts, and plugins/auth-node/src/identity/IdentityClient.test.ts.

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)
All your commits have a Signed-off-by line in the message. (more info)

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

backstage-goalie · 2024-05-07T22:09:04Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/backend-app-api	packages/backend-app-api	patch	`v0.7.3`

Rugvip

Thank you! 🎉

Couple of early thoughts

packages/backend-app-api/src/services/implementations/auth/external/jwks.ts

freben · 2024-05-08T15:29:48Z

@Rugvip a thought about naming/targeting - should it be

named oidc or openid
expect a BASE auth server url instead of directly to jwks
look up /.well-known/openid-configuration and the JWKS uri from there automatically

That's sort of the actual use case being targeted right? Feels like an end user would appreciate the framing of that, and knowing how to ask for and insert the common auth server url rather than the more technical implementation dependant JWKS?

Rugvip · 2024-05-08T15:33:00Z

@freben hmm, JWKS endpoints are used for more than just OIDC. I'm thinking that we consider that as an addition, but as a base feature set I think the existing structure makes sense?

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

…config Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

ryan-hanchett · 2024-05-08T16:55:36Z

@freben I like the idea of calculating the jwks endpoint based on the openid-configuration. Perhaps we could add an additional config where users could provide the base server or the direct JWKS url, or we could add an additional token handler so we have both jwks and oidc to give users flexibility but keep them separate.

Happy to do that as a follow-up PR or to make the changes here, whatever makes the most sense.

freben · 2024-05-08T17:04:13Z

Yep so if the given url doesn't contain ".well-known", do A, otherwise do B. There's possibilities for future smarts as additions to the basic feature as followups

Rugvip · 2024-05-08T17:54:29Z

Prolly need to be a bit more explicit though, JWKS endpoints don't always contain .well-known, e.g. https://login.microsoftonline.com/common/discovery/v2.0/keys

freben · 2024-05-08T18:01:25Z

Alright, makes sense then to keep this one explicit, thanks!

ryan-hanchett · 2024-05-09T15:52:19Z

Any other changes/improvements ya'll want me to make on this existing PR?

ryan-hanchett · 2024-05-17T16:01:11Z

Was the merge-after-release added for ^1.27.0? Or is there another release we're waiting for that I can communicate back to my team?

freben · 2024-05-17T16:19:11Z

It was, yes, we've just been busy.

ryan-hanchett · 2024-05-17T16:27:16Z

Understood, no worries! Thanks for getting back to me so quickly!

docs/auth/service-to-service-auth.md

freben · 2024-05-20T14:37:55Z

packages/backend-app-api/src/services/implementations/auth/external/jwks.ts

+    const issuers = options.getOptionalStringArray('issuers');
+    const audiences = options.getOptionalStringArray('audiences');
+    const subjectPrefix = options.getOptionalString('subjectPrefix');
+    const url = new URL(options.getString('url'));


Could you add all of this to the config.d.ts file in this package? It's important to keep the config schema in check :)

I always forget about the config.d.ts 🤦

Yes I can update it with the new fields

freben · 2024-05-20T14:41:01Z

packages/backend-app-api/src/services/implementations/auth/external/jwks.ts

+  async verifyToken(token: string) {
+    for (const entry of this.#entries) {
+      try {
+        const jwks = createRemoteJWKSet(entry.url);


Oh you don't want to create one of these for every single token verification. You'll lose all TTL based caching benefits and it'll hammer the remote key set relentlessly if this is in a high volume application :) You can create it in add above and keep it as part of the entry.

freben · 2024-05-20T14:57:55Z

packages/backend-app-api/src/services/implementations/auth/external/jwks.ts

+          return { subject: `external:${sub}` };
+        }
+      } catch {
+        continue;


Hm. As a future enhancement (no action now), we may want to look at what the actual error thrown was, so that we can discern between hard faults (the token was ok but we refused the aud), soft errors (there was a problem speaking to the jwks endpoint, log and continue), expected situations (some other jwks signed this token so let's move on silently) etc.

The legacy handler for example, checks if (e.code !== 'ERR_JWS_SIGNATURE_VERIFICATION_FAILED') and rethrows if that's the case, because that means that the token had actual hard errors like TTL expired etc and the caller wants that error thrown to them so they can log it or show it to the user.

But for now, feel free to leave as-is.

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

… step Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

freben

Looks good!

github-actions · 2024-05-22T13:29:06Z

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.28.0 release, scheduled for Tue, 18 Jun 2024.

github-actions · 2024-05-22T13:31:01Z

Uffizzi Cluster pr-24681 was deleted.

freben · 2024-05-22T14:46:14Z

kept working on this one a bit since we have a hackweek and i am poking around in the vicinity of that code right now :)

#24874

ryan-hanchett added 5 commits May 7, 2024 15:05

feat: first attempt at adding jwks-auth to external token handlers

03045fc

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

test: add unit tests for jwks access

e978bad

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

docs: expand on jwks docs

23dff40

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

chore: add changeset

398b82a

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

chore: clean up comments before opening PR

9b0db3f

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

ryan-hanchett requested review from a team as code owners May 7, 2024 22:08

ryan-hanchett requested review from benjdlambert and camilaibs May 7, 2024 22:08

github-actions bot added the documentation Improvements or additions to documentation label May 7, 2024

dforesman approved these changes May 8, 2024

View reviewed changes

Rugvip requested changes May 8, 2024

View reviewed changes

ryan-hanchett added 4 commits May 8, 2024 09:12

fix: rename uri to url

96e30c5

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

fix: default to undefined for algo, iss and aud fields if not set in …

8443332

…config Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

feat: add subjectPrefix config

e5ade53

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

test: fix tests, add new test for custom subject prefix

e54e0c4

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

ryan-hanchett requested a review from Rugvip May 8, 2024 17:20

Rugvip added the merge-after-release This is a bit too scary to merge until after the next release label May 13, 2024

freben reviewed May 21, 2024

View reviewed changes

freben and others added 3 commits May 21, 2024 09:23

Update docs/auth/service-to-service-auth.md

1afb8d4

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

fix: create JWKS as part of adding handler instead of in verification…

276da65

… step Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

fix: add missing config values to config.d.ts

922bddd

Signed-off-by: Ryan Hanchett <ryan.hanchett@invitae.com>

ryan-hanchett requested a review from freben May 21, 2024 17:05

Merge branch 'backstage:master' into feat/add-jwks-access-type-1

28be703

freben approved these changes May 22, 2024

View reviewed changes

freben merged commit 8ed28ff into backstage:master May 22, 2024
29 checks passed

freben mentioned this pull request May 22, 2024

update the jwks external auth to use singular nouns #24874

Merged

ryan-hanchett deleted the feat/add-jwks-access-type-1 branch May 22, 2024 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add jwks access type to external token handler #24681

feat: add jwks access type to external token handler #24681

ryan-hanchett commented May 7, 2024

backstage-goalie bot commented May 7, 2024 •

edited

Rugvip left a comment

freben commented May 8, 2024

Rugvip commented May 8, 2024

ryan-hanchett commented May 8, 2024

freben commented May 8, 2024 •

edited

Rugvip commented May 8, 2024

freben commented May 8, 2024

ryan-hanchett commented May 9, 2024

ryan-hanchett commented May 17, 2024

freben commented May 17, 2024

ryan-hanchett commented May 17, 2024

freben May 20, 2024

ryan-hanchett May 21, 2024

freben May 20, 2024

freben May 20, 2024

freben left a comment

github-actions bot commented May 22, 2024

github-actions bot commented May 22, 2024

freben commented May 22, 2024

feat: add jwks access type to external token handler #24681

feat: add jwks access type to external token handler #24681

Conversation

ryan-hanchett commented May 7, 2024

Hey, I just made a Pull Request!

✔️ Checklist

backstage-goalie bot commented May 7, 2024 • edited

Changed Packages

Rugvip left a comment

Choose a reason for hiding this comment

freben commented May 8, 2024

Rugvip commented May 8, 2024

ryan-hanchett commented May 8, 2024

freben commented May 8, 2024 • edited

Rugvip commented May 8, 2024

freben commented May 8, 2024

ryan-hanchett commented May 9, 2024

ryan-hanchett commented May 17, 2024

freben commented May 17, 2024

ryan-hanchett commented May 17, 2024

freben May 20, 2024

Choose a reason for hiding this comment

ryan-hanchett May 21, 2024

Choose a reason for hiding this comment

freben May 20, 2024

Choose a reason for hiding this comment

freben May 20, 2024

Choose a reason for hiding this comment

freben left a comment

Choose a reason for hiding this comment

github-actions bot commented May 22, 2024

github-actions bot commented May 22, 2024

freben commented May 22, 2024

backstage-goalie bot commented May 7, 2024 •

edited

freben commented May 8, 2024 •

edited