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

How to pass custom state after the CSRF refactoring? #83

Open
fedme opened this issue Jul 22, 2021 · 18 comments
Open

How to pass custom state after the CSRF refactoring? #83

fedme opened this issue Jul 22, 2021 · 18 comments
Assignees

Comments

@fedme
Copy link

fedme commented Jul 22, 2021

Hi!

I have noticed that after #82 the state query parameter is set internally by Ueberauth to perform CSRF validation.

The question now is, can we still pass custom state data during the OAuth process? Does anyone know how?

It seems that whatever you pass via the state query param gets overwritten with Ueberauth's CSRF token. This breaks our app logic, which uses state to know where to redirect users after login.

Thanks!

@yordis
Copy link
Member

yordis commented Jul 22, 2021

@fedme would you mind suggesting some changes in ueberauth where we configure the state token creation?

@fedme
Copy link
Author

fedme commented Jul 22, 2021

Hey @yordis, thanks for getting back to me!

I am not sure my Elixir skills are yet at the point where I can contribute a change of this kind.

But I am thinking maybe I can use a cookie to store my custom state between the request and callback (instead of using the state query parameter). That should be possible, right?

@yordis
Copy link
Member

yordis commented Jul 22, 2021

@fedme well, it depends on how Google OAuth2 handles the CSRF token. Do you know?

Chaging this line,

|> with_state_param(conn)
to should do the trick, and pass the token to the right configuration

That function just read the state, https://github.com/ueberauth/ueberauth/blob/5c297f1d7574a034b2117a4f6ffb3075ca66d359/lib/ueberauth/strategies/helpers.ex#L178 and add it to the configuration, but you don't have to use it.

Also, I notice that maybe, we need to disable CSRF attack for this provider.

Will get back to you on this late today hopefully

@yordis yordis self-assigned this Jul 22, 2021
@fedme
Copy link
Author

fedme commented Jul 22, 2021

Thanks for your help @yordis!

As a workaround, I did manage to use the session cookie to store my custom state (instead of using the state query parameter).

See the following code that shows how I managed to do it:

defmodule MyAppWeb.AuthController do
  use MyAppWeb, :controller
 
  plug(Ueberauth, providers: [:google_custom])

  @provider_config {Ueberauth.Strategy.Google, [default_scope: "email profile"]}

  def request(conn, %{"provider" => "google", "custom_state" => custom_state) do
    # Store custom state in the session
    conn
    |> put_session(:auth_custom_state, custom_state)
    |> Ueberauth.run_request("google", @provider_config)
  end

  def callback(conn, params) do
    %{assigns: %{ueberauth_auth: auth}} =
      conn
      |> Ueberauth.run_callback("google", @provider_config)

    # Get custom state back from session
    auth_custom_state = get_session(conn, :auth_custom_state)

    IO.inspect(auth_custom_state, label: "Auth custom state")
  end
end

With that code, I am able to start the OAuth flow passing some custom state in the URL (e.g. https://localhost:4000/auth/google?custom_state=some_values_here) and then take it back from the session in the callback function.

It seems to work well as a workaround, so maybe there is no need to make any changes to the library?

@yordis
Copy link
Member

yordis commented Jul 25, 2021

Hey @fedme

It seems that Google OAuth2 does support state param, I would suggest stopping using such thing for other things than security reasons as Ueberauth is trying to do at the moment.

https://developers.google.com/identity/protocols/oauth2/web-server

Screen Shot 2021-07-24 at 10 09 16 PM

@yordis
Copy link
Member

yordis commented Jul 25, 2021

I am closing this, strongly recommend refactoring your side, and use something else for your intention.

@yordis yordis closed this as completed Jul 25, 2021
@fernandofleury
Copy link

fernandofleury commented Jul 26, 2021

@yordis just to understand correctly, state is a valid part of Google's OAuth2 spec, but will this ueberauth plugin re-introduce support to pass it through? Or am I missing something?

@yordis
Copy link
Member

yordis commented Jul 26, 2021

@fernandofleury the latest integration will inject a state into the URL, to avoid CSRF attacks.

His issue is that we overwrite the value of such state key, so he lost his value to do other stuff he was trying to do, so it conflicts with state coming from the CSRF attack.

@danturn
Copy link

danturn commented Jul 27, 2021

it does seem a bit confusing to overwrite it in ueberauth imo, the google spec does explicitly suggest using it for "directing the user to the correct resource" as fernando is doing in his original code?

would it make sense to add the csrf (e.g. use a map that gets serialised) to the user provided state inside ueberauth and then strip it out when passing it back to the application code?

@chazu
Copy link

chazu commented Oct 20, 2021

This is also a hindrance for us - the state parameter is not meant solely to be used for security purposes.

@Hanspagh
Copy link
Contributor

Sounds like we might need to reopen this again

@apognu
Copy link

apognu commented May 24, 2022

Would you consider opening this again?

It is really blocking when we need to pass a custom state, either because it's used for another thing than security purposes, or because we need to pass the CSRF nonce generated by another, external, tool (that breaks because of this)?

Defaulting to using Ueberauth's own CSRF is good, but breaks workflows if there are no escape hatches. A simple way to tap into the ignore_csrf_attack attribute would be interesting.

@yordis yordis reopened this May 24, 2022
@yordis
Copy link
Member

yordis commented May 24, 2022

Hey peeps, let me get back to this when I can, if you feel capable or want to collaborate on adding the feature, please take the lead.

@Hanspagh
Copy link
Contributor

Still feels a bit like an anti pattern to store anything else than a nonce in the state param. I would suggest to follow the advice from the link above and store the information locally to get her with the nonce and then verify the state nonce after the auth flow.

I believe the only missing feature for this to work is for this us to get the state nonce into user land.

Would this work for you @apognu

@yordis
Copy link
Member

yordis commented May 24, 2022

The primary reason for using the state parameter is to mitigate CSRF attacks

@Hanspagh from https://auth0.com/docs/secure/attack-protection/state-parameters#csrf-attacks or the Google one I shared.

I don't feel confident that using such state field should be used for anything else than security reasons. It is not clear to me that they are proposing to use it for other stuff.

@Hanspagh
Copy link
Contributor

Yes from auth0 they mention here how to do redirects while still only using the state param for csrf protection
https://auth0.com/docs/secure/attack-protection/state-parameters#redirect-users

@yordis
Copy link
Member

yordis commented Jun 15, 2022

Are you referring to https://auth0.com/docs/secure/attack-protection/state-parameters#alternate-redirect-method

  1. Generate and store a nonce value locally.
  2. Encode any desired state (like the redirect URL) along with the nonce in a protected message (that will need to be encrypted/signed to avoid tampering).
  3. In the response processing, unprotect the message, getting the nonce and other properties stored.
  4. Validate that the included nonce matches what was stored locally and, if so, accept the OAuth2 message.

Just to confirm that I didn't misread since English isn't my first language and I would like to avoid ambiguous language and misunderstanding from my side.
Auth0 is popular enough that if they are teaching people to do that, probably we should open the package to support it then.

@Hanspagh
Copy link
Contributor

Hanspagh commented Jun 15, 2022

Yes, precisely.
The state is still "just" used for csrf protecting, but users can access the nonce before and after the auth request and therefore store any information related to it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants