backstage / backstage

Backstage is an open framework for building developer portals
https://backstage.io/
Apache License 2.0
26.73k stars 5.52k forks source link

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

Closed ryan-hanchett closed 1 week ago

ryan-hanchett commented 3 weeks ago

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.

:heavy_check_mark: Checklist

backstage-goalie[bot] commented 3 weeks ago

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/backend-app-api packages/backend-app-api patch v0.7.3
freben commented 3 weeks ago

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

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 commented 3 weeks ago

@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?

ryan-hanchett commented 3 weeks ago

@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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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

ryan-hanchett commented 3 weeks ago

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

ryan-hanchett commented 2 weeks ago

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 commented 2 weeks ago

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

ryan-hanchett commented 2 weeks ago

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

github-actions[bot] commented 1 week ago

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[bot] commented 1 week ago

Uffizzi Cluster pr-24681 was deleted.

freben commented 1 week ago

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 :)

https://github.com/backstage/backstage/pull/24874