Skip to content

Automatically confirm logged-in users

lain requested to merge confirm-users into develop

Same as !3227 (closed)

Comment from there:

I'm still working out some kinks with registration and running into some problem areas related to #1993 (closed). The MR that closed it !2809 (merged) significantly improved the situation by making confirmation_pending be OFF when account_activation_required is OFF, but it still has two significant issues:

  1. Any sites running prior to this change will need to run a query like UPDATE users SET confirmation_pending=false WHERE local=true; to make toggling account_activation_required be safe. Otherwise, toggling it will force old users out with no clear way to confirm their email.

  2. There is a lot of brittle code that relies on checking the status of account_activation_required and doing conditional logic on it. See an example below.

  @doc "Returns status account"
  @spec account_status(User.t()) :: account_status()
  def account_status(%User{deactivated: true}), do: :deactivated
  def account_status(%User{password_reset_pending: true}), do: :password_reset_pending
  def account_status(%User{local: true, approval_pending: true}), do: :approval_pending

  def account_status(%User{local: true, confirmation_pending: true}) do
    if Config.get([:instance, :account_activation_required]) do
      :confirmation_pending
    else
      :active
    end
  end

I WANT to put :approval_pending below :confirmation_pending, but to do so I will need to refactor this. My new theory is that checks like this should never happen. account_activation_required should only be checked when writing a user, never reading. In the future I want to remove these checks, which would greatly simplify the code, but in order to do that I want to first fix problem 1.

To do so I've added ConfirmUserPlug to the Endpoint, which confirms every unconfirmed user it sees. My premise is that if a logged in user ever accesses the site, they are "confirmed". This Plug will enforce that behavior. The reason it's a Plug instead of adding it to POST /oauth/token is to enforce the behavior not just when users log in, but also for already-logged-in users. Under all circumstances I believe my premise is true.

Edited by lain

Merge request reports