Switching OAuth provider when the user_id field isn't in the response.

[morgz]

Just want to track this issue, I haven't thought about a solution yet. The issue is a follows:

• We are switching from Uberauth.
• A user has been created from FB using Uberauth. The user didn't have an email field so we had our own version of the missing id field flow seen in POW.
• After switching, POW also doesn't see the email in the response and takes the user down the missing id (email) flow.
• The changeset fails because of the unique validation on the email field.

[morgz]

Possibly a migration task to create user_identities for any previously OAuth'ed Users?

[morgz]

Simple mix task did the trick for me. If anyone is curious:

defmodule App.Auth.PowAssentMigrator do
  alias App.Accounts.User
  alias App.Accounts
  require Logger

  def migrate_facebook_users do
    Accounts.list_users |> Enum.each(&(
      case maybe_migrate_user(&1) do
        nil -> Logger.info("User has no FB Link #{&1.email}")
        {:ok, _user_identity} -> Logger.info("Identity Created for #{&1.email}")
        {:error, nil} -> Logger.info("Identity FAILED for #{&1.email}")
      end
    ))
  end

  def maybe_migrate_user(%User{facebook_id: nil}), do: nil
  def maybe_migrate_user(%User{facebook_id: facebook_id, facebook_access_token: facebook_access_token} = user) do
    config = [otp_app: :wilde]

    identity_params = %{
      "provider" => "facebook",
      "uid" => facebook_id,
      "access_token" => facebook_access_token
    }

    PowAssent.Ecto.UserIdentities.Context.upsert(user, identity_params, config)
  end
end

[danschultzer]

Yeah, this would be great to have as a guide similar to the Coherence migration guide.

I would have added the above to an Ecto migration though, something like this (that way the db is kept clean and not dependent on any structs):

defmodule MyApp.Repo.Migrations.RemoveUeberauthFromUser do
  use Ecto.Migration

  import Ecto.Query
  alias MyApp.Repo

  def up do
    create_user_identities()

    alter table(:users) do
      remove :facebook_id
      remove :facebook_access_token
    end
  end

  def down do
    alter table(:users) do
      add :facebook_id, :string
      add :facebook_access_token, :string
    end

    flush()

    set_identity_columns()
  end

  defp create_user_identities() do
    user_identities =
      "users"
      |> where([u], not is_nil(u.facebook_id))
      |> select([u], %{user_id: u.id, provider: "facebook", uid: u.facebook_id, inserted_at: u.inserted_at, updated_at: u.inserted_at})
      # Or with access token:
      # |> select([u], %{user_id: u.id, provider: "facebook", uid: u.facebook_id, access_token: u.facebook_access_token, inserted_at: u.inserted_at, updated_at: u.inserted_at})
      |> Repo.all()
      |> IO.inspect()

    Repo.insert_all("user_identities", user_identities)
  end

  defp set_identity_columns() do
    "user_identities"
    |> select([i], {i.user_id, %{facebook_id: i.uid}})
    # Or with access token:
    # |> select([i], {i.user_id, %{facebook_id: i.uid, facebook_access_token: i.access_token}})
    |> where([i], i.provider == "facebook")
    |> Repo.all()
    |> Enum.each(fn {id, update} ->
      "users"
      |> where([u], u.id == ^id)
      |> update(set: ^Map.to_list(update))
      |> Repo.update_all([])
    end)
  end
end

Not tested, but something like the above should work.

[coladarci]

Heyya - I am in a similar situation on my end when it comes to migrating from a legacy system.

My initial plan was:

• Create Users w/ no passwords and no identifies (but everything else we care about)
• Force users to reset their passwords to get into the new system
• allow users to connect w/ FB if they want to

I was thinking that the way it'd work is when you connect w/ FB, it'd just attach that to the existing User based on email address, but I am seeing it takes you down the add-user-id because it thinks your email address is already taken...

I don't think this was the normal auth flow out in the wild - traditionally I thought the email was trusted, so you can login at any point w/ any provider.

Thoughts?

[danschultzer]

I realized that the Facebook strategy in Assent doesn't pass email_verified: true, so if you use PowEmailConfirmation the flow will be halted to confirm the email. There is no official documentation for it, and the only thing I could find is this StackOverflow post:

Conclusion: The email address you get from Facebook using the Graph API or a FQL query is a verified email. If an account hasn't verified it's email yet it's not possible to get it.

I don't think it's a good idea to assume the email has been verified when it's not documented by Facebook. The docs only says that:

The User's primary email address listed on their profile. This field will not be returned if no valid email address is available.

So with this in mind, to add my thoughts to your post @coladarci:

I don't think this was the normal auth flow out in the wild - traditionally I thought the email was trusted, so you can login at any point w/ any provider.

It's not safe if the user has at least one other auth method. Facebook's guide to implement with existing login system also details how the user has to first login before they can associated with a FB account.

It may be ok if the user has no other auth methods (e.g. no identity providers and no password) and the email is verified. But the email from Facebook can't really be trusted to be verified since there is no official docs on it.

Alternatively, if the user does have an existing account with an email they could be prompted to sign in first to associate the Facebook account instead of getting the prompt to enter a different user id. It doesn't make sense in your case though since the user has no other auth method.

@coladarci does your legacy system have FB login? In that case it's best to just move over the data like as @morgz did. If not, why does the user not have a password?

If you insist on associating the accounts without auth then you could set up custom controller logic for the callback. Or maybe a plug when the user hits the add user id route. In either case I recommend that you keep a limited time window to permit association after migration since this potentially opens up an attack vector.

I aim for best practice security in Pow and PowAssent. There may be no issue to automatically associate a FB account with an existing account without auth in some platforms, but I do want to keep it difficult to go down that road so the dev understand what they are doing.

[coladarci]

Thanks @danschultzer - I'm comfortable making this my own app's concern if you don't feel comfortable making it a general purpose rule (which I totally get given the above).

Migrating the UserIdentity is trivial given my old system did have FB (which I'll do), I was just surprised (and still am a little) that if a user creates an account w/out a provider, there is no longer a way to connect to a provider down the line (unless I'm missing something).

Often times when I use an app, I return after a while and don't remember if I used FB or LinkedIn or Google, I just click one of them and I have never been asked to select an email address because mine has already been used. THAT SAID, if the reco is for me to update the form to say that essentially there is already an account w/ that email, please login w/ that email FIRST and then connect to that provider, at least there is a path forward for the user and we'd all agree there's an extra level of security. But is this possible?

As always, thanks!

[danschultzer]

that if a user creates an account w/out a provider, there is no longer a way to connect to a provider down the line (unless I'm missing something)

If the user creates an account without a provider they would have a password set right? In that case, the appropriate way of handling it is to require the user to sign in first before it can be associated with the FB account. You shouldn't really trust that the email FB has is an email that is confirmed the account owner has access to.

if the reco is for me to update the form to say that essentially there is already an account w/ that email, please login w/ that email FIRST and then connect to that provider, at least there is a path forward for the user and we'd all agree there's an extra level of security. But is this possible?

Yeah, it's possible with either custom controller or a plug. You could also customize the template and just add a note saying something like no valid email set for fb account or the email has already been used for an existing account.

But I think the UX maybe can be improved after testing the logic in PowAssent.

In #114 I cast the user params to the changeset so when you see the form in the :add_user_id route, it'll already be filled with an email (if one was fetched from the provider), instead of just showing a blank field. I have to think about it when I got some more time, but this might make better sense than how it currently works.

If there is a unique constraint error then no has already been taken error will be shown since it requires a call to the DB. It'll first show up when you submit the form.

Another aspect to this is how information leakage can be prevented. I'm working on that in pow-auth/pow#350, but I would probably have to update PowAssent too to conform to this behavior.

When a you use PowEmailConfirmation, and a user signs up with PowAssent, but the email has already been taken then they should get a success message with You'll need to confirm your e-mail before you can sign in. An e-mail confirmation link has been sent to you. message to prevent knowledge about whether the account existed beforehand or not.

I'll be pretty busy the next days so I'll have to get back to this when I got more time available to think all of this through.

[coladarci]

Yeah the above all makes sense.

I think the quickest win is def pre-populating the email + having a failure message of sorts that say that the email is already taken.

That said, I am already customizing the form, so I can easily add the error message which I'll likely do instructing the user to do a password reset if they don't remember their password. As a user, I'd probably find that annoying, but it's super niche so not worth obsessing over, I don't think.

[coladarci]

Found this interesting, this is how CBS handles the exact flow (connect w/ email, then try and connect w/ an account w/ that email)

ie

1. Require a password; don't let them chose a new email
2. Upon entering the password, your accounts are linked so you can log in either way.

[danschultzer]

@coladarci UX has been improved with #137. After refactoring a lot of stuff, and remove user enumeration attack vectors, I can now pass on the changeset so the user will see changeset errors when e.g. the user id is missing, invalid, or already taken. Now I can look at #115.

@morgz I've begun working on a migration guide with #129