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

Investigate switching to frontend auth with PKCE #2572

Closed
Tracked by #1985 ...
petertgiles opened this issue Apr 27, 2022 · 9 comments
Closed
Tracked by #1985 ...

Investigate switching to frontend auth with PKCE #2572

petertgiles opened this issue Apr 27, 2022 · 9 comments
Labels
debt Refactor or improve existing code. security Related to app security. spike Learn more about something we can't estimate yet.

Comments

@petertgiles
Copy link
Contributor

Security consultants have expressed concern that our current auth flow passes authentication tokens in the URI. This makes them vulnerable since they more visible, for example, in the browser history. We mitigate this by using very short-lived tokens. This way the window for attack is fairly limited. It seems that the accepted best solution for SPAs is to use the authorization grant flow with the PKCE extension.

Open questions: Is implementing this form of auth a large improvement over our current "backend helper" approach? Is it worth the large refactor?

The team has expressed a desire to leverage a library for this functionality. Patrick likes Auth0, Tristan found #2376 (comment) and Peter found #2376 (comment).

@petertgiles petertgiles added debt Refactor or improve existing code. security Related to app security. labels Apr 27, 2022
@patcon
Copy link
Contributor

patcon commented May 16, 2022

PKCE is already supported by the mock auth server that I've gotten working:
navikt/mock-oauth2-server#36

We'll be a step closer to using it when #2716 is merged

@petertgiles
Copy link
Contributor Author

While researching Azure B2C I found that Microsoft also recommends authorization code flow with PKCE for SPAs.
https://docs.microsoft.com/en-ca/azure/active-directory-b2c/tutorial-register-spa
They mention (but don't actually recommend) MSAL.js.

@patcon
Copy link
Contributor

patcon commented Jun 10, 2022

Ok, somehow I didn't get this into an issue when I was doing research last week, but here are the libraries that might be helpful:

package star contribs active? notes pkce?
bjerkio/oidc-react 189 19 react component. built on IdentityModel/oidc-client-js maybe yes!
--IdentityModel/oidc-client-js 2,300 67 JS lib. deprecated. yes
authts/react-oidc-context 123 6 React Component. yes
--authts/oidc-client-ts 327 81 JS lib. successor to IdentityModel/oidc-client-js yes
Swizec/useAuth 2,600 21 react component. has auth0 and netlify providers, but needs custom (good docs) no
openid/AppAuth-JS 841 8 JS lib. official one from openid org. core maintainer = google eng. yes
--AxaGuilDEv/react-oidc 316 34 react component. actively maintained by many ppl over a few years. built on openid/AppAuth-JS. implements service worker for security 🎉 yes
auth0/auth0-react 557 23 react component. for ref. service workers. built on auth0/auth0-spa-js yes
-- auth0/auth0-spa-js 680 58 JS lib. lots of cleverness and cross-browser accomodations. yes

AxaGuilDEv/react-oidc seems the best choice, as it has PKCE and also service workers. My understanding is that PKCE is only really secure with service workers, which ensure that the refresh token is never accessible to the main browser javascript thread.

@patcon
Copy link
Contributor

patcon commented Jun 10, 2022

useAuth is also interesting, and seems to have great docs and support, but we'd need to write our own provider:
https://useauth.dev/docs/auth-providers (This maybe isn't a bad thing, and maybe we could steal service worker code from the other library?)

@pamapa
Copy link

pamapa commented Jun 16, 2022

@patcon Do you know react-oidc-context? Might be also a good candidate for your list above...

@simenandre
Copy link

simenandre commented Jul 15, 2022

👋

bjerkio/oidc-react is maintained, albeit rather slow. We're working on bumping to v2, which should give you enough support going forward. It does support PKCE :)

PS: I'm a maintainer of oidc-react

@patcon
Copy link
Contributor

patcon commented Jul 19, 2022

Oh hey, thanks so much @pamapa and @cobraz!

I've updated the above table with your helpful feedback 🎉

(Do either of you feel this resource is useful enough to keep anywhere? Happy to open a PR if you think it's worth maintaining somewhere 😃 )

@simenandre
Copy link

(Do either of you feel this resource is useful enough to keep anywhere? Happy to open a PR if you think it's worth maintaining somewhere 😃 )

I would love to have a list in the readme of our package! 🎉 it would be awesome if you to open a pull request!

We can maintain it, and probably add more details on the differences! So it makes it easier for people!

Btw, have you evaluated using cookie-based authentication? For example with ORY Kratos?

@tristan-orourke tristan-orourke added the spike Learn more about something we can't estimate yet. label Nov 24, 2022
@tristan-orourke tristan-orourke changed the title Switch to frontend auth with PKCE Investigate switching to frontend auth with PKCE Nov 24, 2022
@tristan-orourke
Copy link
Member

I'm closing this as a successful spike. I support @patcon's recommendation to go with the library which uses service workers, as superior protection against XSRF attacks.

Now we will go ahead with implementing this in #4916.

@tristan-orourke tristan-orourke closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Refactor or improve existing code. security Related to app security. spike Learn more about something we can't estimate yet.
Projects
None yet
Development

No branches or pull requests

5 participants