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

Purpose of session_params foi APIs #210

Open
sezaru opened this issue Jun 21, 2021 · 2 comments
Open

Purpose of session_params foi APIs #210

sezaru opened this issue Jun 21, 2021 · 2 comments

Comments

@sezaru
Copy link

sezaru commented Jun 21, 2021

Hello,

Looking at pow_assent documentation and source code, it's not clear to me what is the objective of the session_params config, why it is stored in the connection, etc.

I'm using the google strategy with the API guide, but if, for example, I change the returned session_params for something else before calling the callback endpoint, I still receive the access_token and renew_token without any issue and everything seems to work ok.

This makes me wonder what is the real objective behind creating and store this value if it doesn´t seem to be verified or used anywhere (at least I didn't found any place looking at the code).

Maybe this is something only relevant when using it with web applications instead of mobile ones (where I only access the backend via my API)? If that is the case, is it safe to ignore it entirely?

@sezaru
Copy link
Author

sezaru commented Jun 21, 2021

Just to be clearer, I'm talking specifically about the state value inside session_params.

For example, the API new endpoint returns me a URL with state=88a6975cac9fe8a935c469b3eb977b0c9ff51cf3ab9dc

When calling the API post callback endpoint, if I change this state to anything else, everything seems to work without a problem.

@sezaru
Copy link
Author

sezaru commented Jun 21, 2021

Doing some more research, I found where the session_params is checked, this is done in the oauth2.ex file from assent project in the following code:

defp maybe_check_state(%{state: stored_state}, %{"state" => provided_state}) do
    case Assent.constant_time_compare(stored_state, provided_state) do
      true -> :ok
      false -> {:error, CallbackCSRFError.new("state")}
    end
  end
  defp maybe_check_state(%{state: _state}, params) do
    {:error, MissingParamError.new("state", params)}
  end
  defp maybe_check_state(_session_params, _params), do: :ok

But now this make even less sense, inside the callback function for the API tutorial, there is this code line:

params = Map.drop(params, ["provider", "session_params"])

This will remove the session_params from the params map and later add it to the config in file plug.ex from pow_assent in this code:

  defp maybe_set_session_params_config(config, %{private: %{pow_assent_session_params: params}}), do: Config.put(config, :session_params, params)
  defp maybe_set_session_params_config(config, _conn), do: config

later the maybe_check_state is called with this config and params map.

So, in the end, maybe_check_state will be called with the state from session_params that is extracted from config but was originally in the params map.

This means that, unless I'm missing something, it is impossible to params contain the state from session_params and this check will always be ignored.

@sezaru sezaru changed the title Purpose of session_params Purpose of session_params foi APIs Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant