pow-auth / pow_assent

Multi-provider authentication for your Pow enabled app
https://powauth.com
MIT License
321 stars 50 forks source link

Invalidating UserIdentity #96

Closed git-toni closed 4 years ago

git-toni commented 4 years ago

Hello, Not sure if this is the most appropriate place for these questions, so do forward me somewhere if it's not :slightly_smiling_face:

I'm new to the pow library, and enjoying it so far. So thanks for the effort!

Currently I'm trying implement logout of Github provider via two use cases: Case 1) Redirect user to some URL where all session data is removed and full log-in is again required. Case 2) Invalidate one of the UserIdentity's from Backend. Hence forcing a redirect to login at Frontend.

Tried so far for Case 1 Step-by-step.

  1. Forward /logout to PowAssent.Phoenix.SessionController, :delete or PowAsseng.Phoenix.AuthorizationController, :delete.
  2. Then going back to root detects login missing and requests it again.
  3. However jumping to /auth/github/new from there, it automatically remembers the session. So no login is required.

Desired behavior: full github login is required after visiting :delete.

Tried so far for Case 2

  1. Login via Github --> Creates User<>----UserIdentity rows
  2. Once logged-in, I delete the User's Github UserIdentity from the DB.
  3. Jumping to root path it then sort of behaves like a normal email/password registration, ie requesting email.

Desired behavior: find some way to invalidate User's Github UserIdentity and force full github login again.

danschultzer commented 4 years ago

Case 1

This is the default OAuth 2.0 behavior for most providers. If the user has previously consented access, and the user is authenticated, Github will transparently re-authorize access.

To require users to consent again, you'll have to use the Github API to revoke the grant:

DELETE /applications/:client_id/grants/:access_token

For this you may wish to capture the access token: https://hexdocs.pm/pow_assent/capture_access_token.html#content

You can leverage the HTTP client in Assent (previously PowAssent) to handle the request:

auth    = Base.url_encode64("#{client_id}:#{client_secret}", padding: false)
headers = [{"authorization", "Basic #{auth}"}]
body    = nil
url     = "https://api.github.com/#{client_id}/tokens/#{access_token}",
opts    = []

Assent.Strategy.request(:delete, url, body, headers, opts)

Case two

If you mean that sessions have to be invalidated if the user identity no longer exists, then I would suggest adding a plug to verify that the user still has a user identity assigned. If you mean that you want the user to consent to access again at Github, then you'll have to do the same as above and delete the grant.

git-toni commented 4 years ago

Nice, thanks! I still don't clearly understand why it defaults to email registration when no UderIdentity is found. Ideally I'd like my system to only allow login via Github(or other 3rd party providers), never via email, for which I removed pow_routes from router. However as you mention there should be some extra verification on my end. I'll investigate it via plug as you suggest.

Feel free to close issue since the solution is probably more related to OAuth2 fundamentals rather than pow_assent itself. :)

danschultzer commented 4 years ago

Oh, I think I understand what you mean now. If the user id has already been taken by another user (in this case the email), then the user will be prompted to enter a different user id. So that's why if you just delete the user identity and then try auth from scratch with the user already in the DB, it'll ask for a different email. There's more here: https://github.com/danschultzer/pow_assent/issues/79

So just to recap: If you want to disable email sign up then yes, you just have to remove pow_routes/0 and maybe add a custom routes module for fallbacks:

defmodule MyAppWeb.Pow.Routes do
  use Pow.Phoenix.Routes
  alias MyAppWeb.Router.Helpers, as: Routes

  def session_path(conn, :new), do: Routes.page_path(conn, :index)
  def registration_path(conn, :new), do: Routes.page_path(conn, :index)
  def registration_path(conn, :edit), do: Routes.profi;e_path(conn, :index)
end

The above routes are called if the authorization/registration fails due to unspecified error or that the identity has already been created for another user, and after deleting an identity.

Alternatively, if you want to just use Github for auth and don't really need to keep user details in the DB then you could drop Pow/PowAssent entirely and just use Assent directly for the authorization:

defmodule MyAppWeb.AuthController do
  use MyAppWeb.AuthController, :controller

  def new(conn, _params) do
    config()
    |> Keyword.put(:redirect_uri, callback_url(conn))
    |> Assent.Strategy.Github.authorize_url()
    |> case do
      {:ok, %{url: url, session_params: session_params}} ->
        conn
        |> Plug.Conn.put_session(:assent_session_params, session_params)
        |> redirect(external: url)
    end
  end

  def callback(conn, params) do
    session_params = conn.private[:assent_session_params]

    config
    |> Keyword.put(:redirect_uri, callback_url(conn))
    |> Keyword.put(:session_params, session_params)
    |> Assent.Strategy.Github.callback(params)
    |> case do
      {:ok, %{user: user}} ->
        conn
        |> put_session(:current_user, user)
        |> redirect(to: Routes.index_path(conn, :index))
      {:error, _error} ->
        conn
        |> put_flash(:error, "Couldn't authenticate")
        |> redirect(to: Routes.index_path(conn, :index))
    end
  end

  defp config do
    Application.get_env(:my_app_web, :github_provider) || raise "Need to set :github_provider first!"
  end

  defp callback_url(conn) do
    Routes.auth_url(conn, :callback)
  end
end
git-toni commented 4 years ago

Great info here! Your explanation makes sense and #79 then #18 helped as well.

and maybe add a custom routes module for fallbacks

That's good to know, I totally missed the :backend_routes config option from the docs, which I'll definitely use to overwrite the default values for session_path and registration_path. If I understood it well, those two are used for example here https://github.com/danschultzer/pow_assent/blob/c8d161ec47/lib/pow_assent/phoenix/controllers/authorization_controller.ex#L131 via the routes function, which comes from pow itself? If so, I'm wondering: is it desirable that pow_assent assumes the existence of registration_path and session_path? I've got a ton to learn here, so I might be missing something with the observation.

Pow/PowAssent entirely and just use Assent directly for the authorization:

That's a possibility I hadn't thought of, I see now what Assent does, interesting. For this use-case I don't think I'll use it since I do want to keep user details in the DB and there will be a user-invitation layer as well(looking at you PowInvitation :smile: ). Regardless it's worth keeping in mind for other occasions. Thanks!

danschultzer commented 4 years ago

If I understood it well, those two are used for example here https://github.com/danschultzer/pow_assent/blob/c8d161ec47/lib/pow_assent/phoenix/controllers/authorization_controller.ex#L131 via the routes function, which comes from pow itself?

Yup, and these two:

https://github.com/danschultzer/pow_assent/blob/v0.4.1/lib/pow_assent/phoenix/controllers/authorization_controller.ex#L161 https://github.com/danschultzer/pow_assent/blob/v0.4.1/lib/pow_assent/phoenix/controllers/registration_controller.ex#L46

Other than that there are a few after_registration_path/1 and after_sign_in_path/1 calls which just defaults to /.

If so, I'm wondering: is it desirable that pow_assent assumes the existence of registration_path and session_path? I've got a ton to learn here, so I might be missing something with the observation.

PowAssent depends on Pow so I think so. However, it may make sense to have a routes module for PowAssent like there is for Pow to make it more explicit or possibly easier to configure.

If you got ideas for how to streamline the flow more please let me know. I'm all for making less assumptions in PowAssent where possible. Maybe to start with I should just move all Pow route calls into small private methods so it would be easier to understand from the code what routes are being used from Pow.

git-toni commented 4 years ago

I think my misunderstanding was fundamentally regarding what role each library plays. For example I initially thought it was possible to have pow_assent living on its own completely, but it's actually an extension on top of pow, so it's rightly coupled to it :) Now thanks to your comment I know that assent is what I need for that, ie 3rd party authentication only(albeit without User store).

The way I see it now(pls correct me otherwise) then there are different "features". 1) User store (taking email as default user ID) 2) Email login + registration (sort of a UserIdentity type arguably?) 3) Session Logic 4) Authentication with 3rd party providers 5) UserIdentity's store and management

pow provides 1), 2), and 3). assent provides solely 4). pow_assent provides 5) on top of pow and assent.

By following your recommendation of re-writing those route helpers(session_path, registration_path) I'll have it all except 2), which is what I'm looking for, right?

I'll be happy to collaborate on a routes module for pow_assent, sounds like a good first issue, but I'd need some more time to familiarize myself with this :smile:

danschultzer commented 4 years ago

By following your recommendation of re-writing those route helpers(session_path, registration_path) I'll have it all except 2), which is what I'm looking for, right?

Yeah, you are correct. Just to be clear User ID (email) registration/login isn't user identity. User identity only exists with identity providers, otherwise it's just regular user credentials.

I'll be happy to collaborate on a routes module for pow_assent, sounds like a good first issue, but I'd need some more time to familiarize myself with this đŸ˜„

:rocket: