p2-inc / keycloak-magic-link

Magic Link Authentication for Keycloak
Other
215 stars 41 forks source link

Magic link doesn't support PKCE #8

Open L-E opened 1 year ago

L-E commented 1 year ago

I was planning to the magic link extension with a standard Authorization Flow incl. PKCE (the use case is a mobile app logging in a user via email to access an API). In general everything works fine, but at the end I realised that it doesn't seem to work with PKCE, i.e., the code_verifier is ignored when sending it.

My assumption was that the magic-link extension doesn't really get involved in PKCE handling, but when I use something else (e.g., standard username+password) instead of the magic link it works fine.

Is this not possible when using the magic link or is it just a limitation/bug in the magic link extension... or am I doing something wrong?

Example request to initiate the flow: https://my-keycloak/realms/test/protocol/openid-connect/auth?client_id=my-client&response_type=code&scope=email openid&redirect_uri=https://my-redirect-url/&state=my-random-state&nonce=my-random-nonce&code_challenge=Uz9stxVbetoK4mdkE8LLzl7UyMIPra5DyIputr9FWCA&code_challenge_method=S256&login_hint=foo@example.com

When clicking the magic link in the email https://my-keycloak/realms/test/login-actions/action-token?key=eyJhbGciOiJIUzI1NiIsIn....&client_id=my-client I'm getting a redirect that only contains code and session_state, but not state or nonce.

Requesting the access_token using the code from the magic-link response with an INVALID code_verifier still gives me a valid access_token. Keycloak doesn't seem to expect a code_verifier and silently ignores it.

xgp commented 1 year ago

@L-E The current approach creates an entirely new login session, and it doesn't reuse the state/nonce values or carry the code_verifier to the action token. So currently a limitation. I can experiment with adding it. Thanks for submitting this, as I hadn't considered it before. If you want to take a look, the relevant code is in the MagicLinkAuthenticator and MagicLinkActionTokenHandler classes.

yordis commented 1 year ago

Any updates @xgp? Thanks in advanced.

xgp commented 1 year ago

@yordis Happy to accept PRs!

yordis commented 1 year ago

Believe me, I would do it by now if I even knew how to get started with Java 😭 ... I tried and I get lost in DX tools and whatnot, not counting the Keycloak abstraction on top of that 😢

sweeneytr commented 1 year ago

+1 to this, because state is an essential element in OAuth security this is unusable w/ the libraries we're using.

xgp commented 1 year ago

Agreed that this is important, as many use cases require PKCE. However, the problem in this implementation is that we use the action token to make it essentially stateless, whereas PKCE requires the server to store a value. I've looked at a few ideas, including storing the value as a temporary "credential" for the user. Any additional suggestions/ideas for how to support the PKCE flow are welcome.

sweeneytr commented 1 year ago

So, PKCE and state are different, so far as I understand it, that serve different purposes. state is an opaque value that is passed in the query-params when redirecting to the auth server, which needs to be passed when redirecting back to the app. It's the mechanism by which the app is aware that a GET /auth-callback was initiated by it.

Without state, you're vulnerable to someone sending a user a link like https://yoursite.com/auth-callback?code=<code_for_attacker>, and putting them into a compromised state. Auth0 has good docs on this. Most libraries we work with force the use of state, it'd be a PITA to work around.

I'm taking a crack at some code changes.

xgp commented 1 year ago

Where I'm hung up is whether or not it is safe to put any of the values (state, or code_challenge for PKCE) in the action token.

If we did put state in there then it would allow us to return it to the client in the callback. Similarly, if we put the code_challenge, then it would allow us to validate the code_verifier when the code exchange happened.

In any case (current or including those params) if attacker gets the action token, they're getting in, so it's not worse than today to include them.

However, if we wanted to preserve them on the server, rather than in the action token, that would be a heavier lift, as it would require some mechanism of persisting them between the initial action token creation and the handling of the action token. If it's not clear why, this is because in Keycloak, these are technically two separate authentication sessions. We had to take this route in order to allow using the link on different devices.

cedricjacobs commented 1 year ago

So, I am struggling to implement it in my expressJS backend as I use passport in combination with openid-client. The message I am seeing appearing is very similar (state missing from response).

So my question is: how do you configure it then? if it is not a PKCE flow, which type is it then?

xgp commented 1 year ago

@cedricjacobs it's a normal OIDC Authorization Code Flow without the state parameter.

cedricjacobs commented 1 year ago

The weird thing is, that it does everything quite perfectly, but for the callback. Is there like an example backend where this is implemented? (just to be clear, it's a backend that serves HTML)

xgp commented 1 year ago

"but for the callback". Do you have more information? Can you capture a HAR of what is going on and post it? Or at least what your backend is getting as the redirect/callback?

cedricjacobs commented 1 year ago

I will post a full situation report later on today. I managed however to 'resolve' the matter :-) I will include a gist for future reference for all to suggest/improve

xgp commented 1 year ago

@sweeneytr This approach solves the state/nonce support issue. https://github.com/p2-inc/keycloak-magic-link/pull/29 Let me know if you have the means to test from this in order to validate it works with the OIDC libs you are using.

xgp commented 1 year ago

state/nonce portion fixed in 50f2c0e

nlsrchtr commented 6 months ago

Thanks everyone in looking into this. We are currently also faced with this issue, since we would like to make logging into our mobile app as easy as possible.

@sweeneytr, were you able to move further with the additions @xgp was proposing?

Since I'm not a Java developer, I can offer my help in extending the documentation for this feature - if there is help needed.

Would be great to get this feature out of the door, since it enables really easy logins.

xgp commented 6 months ago

@nlsrchtr There is a PR https://github.com/p2-inc/keycloak-magic-link/pull/56 for PKCE support. Best thing anyone can do to help at this point is to test that branch https://github.com/p2-inc/keycloak-magic-link/tree/xgp/pkce . If I get the :+1: from enough people that it works, I'll merge it.

embesozzi commented 3 months ago

The "problem" with the magic link that occurs in every IdP vendor is that these flows interrupt the login process, which then resumes when you click the link (a process that is not easy to implement in OAuth 2.0 with traditional flows). I must say that the xgp implementation is great in how it deals with this problem in KC.

Just to provide another perspective on solving the problem with state parameters: In other IdPs such as ForgeRock, the magic link redirects to the app (not to the IdP). The app then parses the link and sends it back to the IdP. Therefore, I would say it's a standard federation process that ultimately results in user authentication. Perhaps implementing it in that way will help in some scenarios.

cbeckmann486 commented 2 weeks ago

@nlsrchtr There is a PR #56 for PKCE support. Best thing anyone can do to help at this point is to test that branch https://github.com/p2-inc/keycloak-magic-link/tree/xgp/pkce . If I get the 👍 from enough people that it works, I'll merge it.

Hi xgp. I tried this patch today, after doing a quick job of updating it for compatibility with 24.0.4. I'm sorry to say that even wtih this patch, I'm not seeing code_challenge or cc in the token generated.

xgp commented 2 weeks ago

@cbeckmann486 Thanks for testing. Can you post your client config and how you are setting up the login in your application? This doesn't set up PKCE for you. It only transits the appropriate variables so the code_challenge, etc transit in the magic link action token.