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

Rework the Auth0 Guide #443

Open
zachdaniel opened this issue Sep 28, 2023 · 5 comments
Open

Rework the Auth0 Guide #443

zachdaniel opened this issue Sep 28, 2023 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@zachdaniel
Copy link
Collaborator

Based on feedback from multiple users, for example: https://elixirforum.com/t/onboarding-feedback-the-good-the-bad-and-the-ugly/58527/6?u=zachdaniel

Ultimately what should be done is that we should rework the guide to assume that the user has not already set up ash_authentication, and then highlight some areas that might be different if they already have set it up for password authentication, for example.

@zachdaniel zachdaniel added the documentation Improvements or additions to documentation label Sep 28, 2023
@dewetblomerus
Copy link
Contributor

dewetblomerus commented Nov 1, 2023

After following the current guide, I am running into what could be a severe problem.

This is not an actual issue with the code, just with the guide, so I'm not opening a separate issue.

After following the guide, my app is vulnerable to account takeover. Here are the steps to reproduce:

  1. Enable both Gmail and Email/Password connectors in Auth0
  2. Sign up with Gmail, now my app considers the user to be [email protected]
  3. In an incognito tab, sign up with Username/Password and choose the same email address [email protected]
  4. Do not verify the email address
  5. AshAuthentication considers me to be the same user as the one from Gmail, even though we have no guarantees that it is the same person, they just used the same email to sign up.

What I think we might need to do

The user_info that comes from Auth0 includes a sub, these are not the same for the two users.

"sub" => "google-oauth2|redacted-numbers"
"sub" => "auth0|redacted-numbers-and-letters"

If instead of:

  identities do
    identity :unique_email, [:email]
  end

We made the identity :unique_auth0_id, [:auth0_id]

Then, the different Auth0 accounts would be considered different users in Ash.

If they were later linked, the sub will not change. If you look at the example profiles on that page I linked, the primary and linked profiles have identical sub`s.

So if an Ash user followed a guide that correlated the User resource to an auth0_id, they can later build Auth0-user-linking and continue with the same auth0_id.

I would love for someone with more Auth0 experience to weigh in.

@jimsynz
Copy link
Collaborator

jimsynz commented Nov 1, 2023

Yeah, this seems like an oversight on my part. We do have the UserIdentity extension and resource which we can use to track a user's multiple identities, but it all still comes down to whatever we decide to use for the unique identity for the user. At the very least I think we should update the guides to explain how this could happen in the case where there are multiple strategies enabled.

@dewetblomerus
Copy link
Contributor

dewetblomerus commented Nov 3, 2023

No worries, it already felt like magic when I followed a guide, got auth working, and I have the absolute minimum amount of code to maintain. The UserIdentity extension looks cool.

One more reason to use the auth0_id is that if someone changes their email address, using the email as unique identity would mean that we need to change the email in our database and on Auth0 at the same time, if the email was only changed in Auth0, we would end up creating a new user. If we use the auth0_id, the email just needs to be changed in Auth0 and then it automatically gets changed in our app on the next login.

What do you think of a PR to the guide that changes the action to:

    create :register_with_auth0 do
      argument :user_info, :map, allow_nil?: false
      argument :oauth_tokens, :map, allow_nil?: false
      upsert? true
      upsert_identity :unique_auth0_id

      # Required if you have token generation enabled.
      change AshAuthentication.GenerateTokenChange

      # Required if you have the `identity_resource` configuration enabled.
      change AshAuthentication.Strategy.OAuth2.IdentityChange

      change fn changeset, _ ->
        user_info = Ash.Changeset.get_argument(changeset, :user_info)

        changes = %{
          "email" => Map.get(user_info, "email"),
          "auth0_id" => Map.get(user_info, "sub")
        }

        Ash.Changeset.change_attributes(
          changeset,
          changes
        )
      end

I would also wan to incorporate whatever I learn from here

@jimsynz
Copy link
Collaborator

jimsynz commented Nov 5, 2023

Absolutely. Rewrite the whole thing if you want 😂

@jimsynz
Copy link
Collaborator

jimsynz commented Nov 15, 2023

@dewetblomerus were you still intending to work up a PR for this or should I add it to my backlog?

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

No branches or pull requests

3 participants