Delete all sessions when calling `Pow.Plug.delete_user/2`

[danschultzer]

Prompted by #386 (comment)

Currently Pow.Plug.delete_user/2 calls do_delete/2 for the plug, which only deletes the current session. However it should clear all sessions for the user. Only Pow.Store.CredentialsCache knows about related sessions which makes this tricky. Might work if a config value is passed on e.g. delete_all_for: user.

PowPersistentSession might interfere with this. The way to resolve that is to delete all persistent session tokens that has a session fingerprint that's deleted. There could be a method in PowPersistentSession.Store.PersistentSessionCache to do that, but it might also be a good idea to be able to look up all persistent sessions for the user.

[Kurisu3]

May I ask a question about this?

I have a custom controller in an admin panel to manage users (list - delete). I will maybe add new - create actions later since with the Invitation extension it is not possible to set a role along the invited user email.

But let's come back to my question about user deletion. If I understand correctly Pow.Plug.delete_user/2 is not for the case when an admin want to delete another user....
So in that case would just calling something like Repo.delete(user) be enough? I mean can I just delete the user from the database without any issue?

[danschultzer]

If I understand correctly Pow.Plug.delete_user/2 is not for the case when an admin want to delete another user....

Correct, it's only when the user deletes the current user signed in, and deletes the associated session.

I mean can I just delete the user from the database without any issue?

The session will still be active, so if you don't check if the user exists in the DB the user may be able to continue using the app until their session expire. #563 would resolve this.

To remove all the sessions for the user it's necessary to call Pow.Store.CredentialsCache.sessions/2 and then Pow.Store.CredentialsCache.delete/2 for each of them.

I wonder if permitting a second argument with the user to be deleted in Pow.Plug.delete_user/1 makes sense.

[Kurisu3]

Thanks for this clarification.

I wonder if permitting a second argument with the user to be deleted in Pow.Plug.delete_user/1 makes sense.

That would be great! ^^

[benonymus]

If I understand correctly Pow.Plug.delete_user/2 is not for the case when an admin want to delete another user....

Correct, it's only when the user deletes the current user signed in, and deletes the associated session.

I mean can I just delete the user from the database without any issue?

The session will still be active, so if you don't check if the user exists in the DB the user may be able to continue using the app until their session expire. #563 would resolve this.

To remove all the sessions for the user it's necessary to call Pow.Store.CredentialsCache.sessions/2 and then Pow.Store.CredentialsCache.delete/2 for each of them.

I wonder if permitting a second argument with the user to be deleted in Pow.Plug.delete_user/1 makes sense.

Hey @danschultzer ,

I ran into this comment while I was looking for a way to clear up sessions in case a user updates their password.
What config do I need to pass to these to query both the sessions and the credentials?

If I do CredentialsCache.sessions([backend: "backend here"], user)
and I delete, the persistent_session stays around and the user is not logged out.

We have a liveview page and getting the config from the socket was not working.

The use case is in case a user account is compromised, and they chnaged the password we want preferably the other sessions to be deleted.

Would it be possible to remove the reset_token as well?

Thanks

[sb8244]

@benonymus I ended up doing something like this to clear out the session + the persistent session. Unfortunately, due to how persistent sessions are stored, there's no way except full enumeration to clear it out:

  def before_respond(PowResetPassword.Phoenix.ResetPasswordController, :update, {:ok, user = %{id: user_id}, conn}, _config) do
    session_key = :erlang.term_to_binary([UserService.User, :user, user.id])

    # Delete all active sessions
    from(
      sesh in UserService.PostgresPowStore,
      where: sesh.original_key == ^session_key
    )
    |> Super.Repo.delete_all()

    # Delete the persistent session entries
    from(
      pow in UserService.PostgresPowStore,
      where: pow.namespace == "persistent_session"
    )
    |> Super.Repo.all()
    |> Enum.each(fn entry ->
      case :erlang.binary_to_term(entry.value) do
        {%UserService.User{id: ^user_id}, _opts} ->
          from(
            pow in UserService.PostgresPowStore,
            where: pow.key == ^entry.key and pow.namespace == ^entry.namespace
          )
          |> Super.Repo.delete_all()

        _ ->
          nil
      end
    end)

    {:ok, user, conn}
  end

I think enumeration that doesn't load all (batch it) and using the actual store functions would make this better. I don't care about being generic though, so just winged it a bit :)