Change unconfirmed user login behaviour (#11375)

Allow access to account settings, 2FA, authorized applications, and
account deletions to unconfirmed and pending users, as well as
users who had their accounts disabled. Suspended users cannot update
their e-mail or password or delete their account.

Display account status on account settings page, for example, when
an account is frozen, limited, unconfirmed or pending review.

After sign up, login users straight away and show a simple page that
tells them the status of their account with links to account settings
and logout, to reduce onboarding friction and allow users to correct
wrongly typed e-mail addresses.

Move the final sign-up step of SSO integrations to be the same
as above to reduce code duplication.

24 files changed, 203 insertions, 86 deletions

diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 33bac9bbc..31cf17710 100644
--- a/app/controllers/about_controller.rb
+++ b/app/controllers/about_controller.rb
@@ -7,7 +7,7 @@ class AboutController < ApplicationController
   before_action :set_instance_presenter
   before_action :set_expires_in
 
-  skip_before_action :check_user_permissions, only: [:more, :terms]
+  skip_before_action :require_functional!, only: [:more, :terms]
 
   def show; end
 
diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
index eca558f42..6f33a1ea9 100644
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@@ -7,7 +7,7 @@ class Api::BaseController < ApplicationController
   include RateLimitHeaders
 
   skip_before_action :store_current_location
-  skip_before_action :check_user_permissions
+  skip_before_action :require_functional!
 
   before_action :set_cache_headers
 
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index b8a1faf77..41ce1a0ca 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -25,7 +25,7 @@ class ApplicationController < ActionController::Base
   rescue_from Mastodon::NotPermittedError, with: :forbidden
 
   before_action :store_current_location, except: :raise_not_found, unless: :devise_controller?
-  before_action :check_user_permissions, if: :user_signed_in?
+  before_action :require_functional!, if: :user_signed_in?
 
   def raise_not_found
     raise ActionController::RoutingError, "No route matches #{params[:unmatched_route]}"
@@ -57,8 +57,8 @@ class ApplicationController < ActionController::Base
     forbidden unless current_user&.staff?
   end
 
-  def check_user_permissions
-    forbidden if current_user.disabled? || current_user.account.suspended?
+  def require_functional!
+    redirect_to edit_user_registration_path unless current_user.functional?
   end
 
   def after_sign_out_path_for(_resource_or_scope)
diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb
index c28c7471c..0d7c6e7c2 100644
--- a/app/controllers/auth/confirmations_controller.rb
+++ b/app/controllers/auth/confirmations_controller.rb
@@ -4,34 +4,15 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
   layout 'auth'
 
   before_action :set_body_classes
-  before_action :set_user, only: [:finish_signup]
 
-  def finish_signup
-    return unless request.patch? && params[:user]
-
-    if @user.update(user_params)
-      @user.skip_reconfirmation!
-      bypass_sign_in(@user)
-      redirect_to root_path, notice: I18n.t('devise.confirmations.send_instructions')
-    else
-      @show_errors = true
-    end
-  end
+  skip_before_action :require_functional!
 
   private
 
-  def set_user
-    @user = current_user
-  end
-
   def set_body_classes
     @body_classes = 'lighter'
   end
 
-  def user_params
-    params.require(:user).permit(:email)
-  end
-
   def after_confirmation_path_for(_resource_name, user)
     if user.created_by_application && truthy_param?(:redirect_to_app)
       user.created_by_application.redirect_uri
diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb
index bbf63bed3..682c77016 100644
--- a/app/controllers/auth/omniauth_callbacks_controller.rb
+++ b/app/controllers/auth/omniauth_callbacks_controller.rb
@@ -27,7 +27,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
     if resource.email_verified?
       root_path
     else
-      finish_signup_path
+      auth_setup_path(missing_email: '1')
     end
   end
 end
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 83797cf1f..019caf9c1 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -9,6 +9,9 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   before_action :set_sessions, only: [:edit, :update]
   before_action :set_instance_presenter, only: [:new, :create, :update]
   before_action :set_body_classes, only: [:new, :create, :edit, :update]
+  before_action :require_not_suspended!, only: [:update]
+
+  skip_before_action :require_functional!, only: [:edit, :update]
 
   def new
     super(&:build_invite_request)
@@ -43,7 +46,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   end
 
   def after_sign_up_path_for(_resource)
-    new_user_session_path
+    auth_setup_path
   end
 
   def after_sign_in_path_for(_resource)
@@ -102,4 +105,8 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   def set_sessions
     @sessions = current_user.session_activations
   end
+
+  def require_not_suspended!
+    forbidden if current_account.suspended?
+  end
 end
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index fb8615c31..7e6dbf19e 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -6,8 +6,10 @@ class Auth::SessionsController < Devise::SessionsController
   layout 'auth'
 
   skip_before_action :require_no_authentication, only: [:create]
-  skip_before_action :check_user_permissions, only: [:destroy]
+  skip_before_action :require_functional!
+
   prepend_before_action :authenticate_with_two_factor, if: :two_factor_enabled?, only: [:create]
+
   before_action :set_instance_presenter, only: [:new]
   before_action :set_body_classes
 
diff --git a/app/controllers/auth/setup_controller.rb b/app/controllers/auth/setup_controller.rb
new file mode 100644
index 000000000..46c5f2958
--- /dev/null
+++ b/app/controllers/auth/setup_controller.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+class Auth::SetupController < ApplicationController
+  layout 'auth'
+
+  before_action :authenticate_user!
+  before_action :require_unconfirmed_or_pending!
+  before_action :set_body_classes
+  before_action :set_user
+
+  skip_before_action :require_functional!
+
+  def show
+    flash.now[:notice] = begin
+      if @user.pending?
+        I18n.t('devise.registrations.signed_up_but_pending')
+      else
+        I18n.t('devise.registrations.signed_up_but_unconfirmed')
+      end
+    end
+  end
+
+  def update
+    # This allows updating the e-mail without entering a password as is required
+    # on the account settings page; however, we only allow this for accounts
+    # that were not confirmed yet
+
+    if @user.update(user_params)
+      redirect_to auth_setup_path, notice: I18n.t('devise.confirmations.send_instructions')
+    else
+      render :show
+    end
+  end
+
+  helper_method :missing_email?
+
+  private
+
+  def require_unconfirmed_or_pending!
+    redirect_to root_path if current_user.confirmed? && current_user.approved?
+  end
+
+  def set_user
+    @user = current_user
+  end
+
+  def set_body_classes
+    @body_classes = 'lighter'
+  end
+
+  def user_params
+    params.require(:user).permit(:email)
+  end
+
+  def missing_email?
+    truthy_param?(:missing_email)
+  end
+end
diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb
index f3d235366..fb8389034 100644
--- a/app/controllers/oauth/authorized_applications_controller.rb
+++ b/app/controllers/oauth/authorized_applications_controller.rb
@@ -7,6 +7,8 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio
   before_action :authenticate_resource_owner!
   before_action :set_body_classes
 
+  skip_before_action :require_functional!
+
   include Localized
 
   def destroy
diff --git a/app/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb
index dd19aadf6..97fe4d328 100644
--- a/app/controllers/settings/deletes_controller.rb
+++ b/app/controllers/settings/deletes_controller.rb
@@ -5,6 +5,9 @@ class Settings::DeletesController < Settings::BaseController
 
   before_action :check_enabled_deletion
   before_action :authenticate_user!
+  before_action :require_not_suspended!
+
+  skip_before_action :require_functional!
 
   def show
     @confirmation = Form::DeleteConfirmation.new
@@ -29,4 +32,8 @@ class Settings::DeletesController < Settings::BaseController
   def delete_params
     params.require(:form_delete_confirmation).permit(:password)
   end
+
+  def require_not_suspended!
+    forbidden if current_account.suspended?
+  end
 end
diff --git a/app/controllers/settings/sessions_controller.rb b/app/controllers/settings/sessions_controller.rb
index 84ebb21f2..df5ace803 100644
--- a/app/controllers/settings/sessions_controller.rb
+++ b/app/controllers/settings/sessions_controller.rb
@@ -4,6 +4,8 @@ class Settings::SessionsController < Settings::BaseController
   before_action :authenticate_user!
   before_action :set_session, only: :destroy
 
+  skip_before_action :require_functional!
+
   def destroy
     @session.destroy!
     flash[:notice] = I18n.t('sessions.revoke_success')
diff --git a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb
index 02652a36c..3145e092d 100644
--- a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb
+++ b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb
@@ -8,6 +8,8 @@ module Settings
       before_action :authenticate_user!
       before_action :ensure_otp_secret
 
+      skip_before_action :require_functional!
+
       def new
         prepare_two_factor_form
       end
diff --git a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb
index 874bf532b..09a759860 100644
--- a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb
+++ b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb
@@ -7,6 +7,8 @@ module Settings
 
       before_action :authenticate_user!
 
+      skip_before_action :require_functional!
+
       def create
         @recovery_codes = current_user.generate_otp_backup_codes!
         current_user.save!
diff --git a/app/controllers/settings/two_factor_authentications_controller.rb b/app/controllers/settings/two_factor_authentications_controller.rb
index e12c43074..6904076e4 100644
--- a/app/controllers/settings/two_factor_authentications_controller.rb
+++ b/app/controllers/settings/two_factor_authentications_controller.rb
@@ -7,6 +7,8 @@ module Settings
     before_action :authenticate_user!
     before_action :verify_otp_required, only: [:create]
 
+    skip_before_action :require_functional!
+
     def show
       @confirmation = Form::TwoFactorConfirmation.new
     end
diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss
index 373a10260..f625bc139 100644
--- a/app/javascript/styles/mastodon/admin.scss
+++ b/app/javascript/styles/mastodon/admin.scss
@@ -204,29 +204,6 @@ $content-width: 840px;
         border: 0;
       }
     }
-
-    .muted-hint {
-      color: $darker-text-color;
-
-      a {
-        color: $highlight-text-color;
-      }
-    }
-
-    .positive-hint {
-      color: $valid-value-color;
-      font-weight: 500;
-    }
-
-    .negative-hint {
-      color: $error-value-color;
-      font-weight: 500;
-    }
-
-    .neutral-hint {
-      color: $dark-text-color;
-      font-weight: 500;
-    }
   }
 
   @media screen and (max-width: $no-columns-breakpoint) {
@@ -249,6 +226,41 @@ $content-width: 840px;
   }
 }
 
+hr.spacer {
+  width: 100%;
+  border: 0;
+  margin: 20px 0;
+  height: 1px;
+}
+
+.muted-hint {
+  color: $darker-text-color;
+
+  a {
+    color: $highlight-text-color;
+  }
+}
+
+.positive-hint {
+  color: $valid-value-color;
+  font-weight: 500;
+}
+
+.negative-hint {
+  color: $error-value-color;
+  font-weight: 500;
+}
+
+.neutral-hint {
+  color: $dark-text-color;
+  font-weight: 500;
+}
+
+.warning-hint {
+  color: $gold-star;
+  font-weight: 500;
+}
+
 .filters {
   display: flex;
   flex-wrap: wrap;
diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss
index 456ee4e0d..ac99124ea 100644
--- a/app/javascript/styles/mastodon/forms.scss
+++ b/app/javascript/styles/mastodon/forms.scss
@@ -300,6 +300,13 @@ code {
     }
   }
 
+  .input.static .label_input__wrapper {
+    font-size: 16px;
+    padding: 10px;
+    border: 1px solid $dark-text-color;
+    border-radius: 4px;
+  }
+
   input[type=text],
   input[type=number],
   input[type=email],
diff --git a/app/models/concerns/omniauthable.rb b/app/models/concerns/omniauthable.rb
index 283033083..b9c124841 100644
--- a/app/models/concerns/omniauthable.rb
+++ b/app/models/concerns/omniauthable.rb
@@ -43,7 +43,7 @@ module Omniauthable
       # Check if the user exists with provided email if the provider gives us a
       # verified email.  If no verified email was provided or the user already
       # exists, we assign a temporary email and ask the user to verify it on
-      # the next step via Auth::ConfirmationsController.finish_signup
+      # the next step via Auth::SetupController.show
 
       user = User.new(user_params_from_auth(auth))
       user.account.avatar_remote_url = auth.info.image if auth.info.image =~ /\A#{URI.regexp(%w(http https))}\z/
diff --git a/app/models/user.rb b/app/models/user.rb
index 31c99630c..474c77293 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -161,7 +161,11 @@ class User < ApplicationRecord
   end
 
   def active_for_authentication?
-    super && approved?
+    true
+  end
+
+  def functional?
+    confirmed? && approved? && !disabled? && !account.suspended?
   end
 
   def inactive_message
diff --git a/app/views/auth/confirmations/finish_signup.html.haml b/app/views/auth/confirmations/finish_signup.html.haml
deleted file mode 100644
index 9d09b74e1..000000000
--- a/app/views/auth/confirmations/finish_signup.html.haml
+++ /dev/null
@@ -1,15 +0,0 @@
-- content_for :page_title do
-  = t('auth.confirm_email')
-
-= simple_form_for(current_user, as: 'user', url: finish_signup_path, html: { role: 'form'}) do |f|
-  - if @show_errors && current_user.errors.any?
-    #error_explanation
-      - current_user.errors.full_messages.each do |msg|
-        = msg
-        %br
-
-  .fields-group
-    = f.input :email, wrapper: :with_label, required: true, hint: false
-
-  .actions
-    = f.submit t('auth.confirm_email'), class: 'button'
diff --git a/app/views/auth/registrations/_sessions.html.haml b/app/views/auth/registrations/_sessions.html.haml
index d7d96a1bb..395e36a9f 100644
--- a/app/views/auth/registrations/_sessions.html.haml
+++ b/app/views/auth/registrations/_sessions.html.haml
@@ -1,6 +1,8 @@
-%h4= t 'sessions.title'
+%h3= t 'sessions.title'
 %p.muted-hint= t 'sessions.explanation'
 
+%hr.spacer/
+
 .table-wrapper
   %table.table.inline-table
     %thead
diff --git a/app/views/auth/registrations/_status.html.haml b/app/views/auth/registrations/_status.html.haml
new file mode 100644
index 000000000..b38a83d67
--- /dev/null
+++ b/app/views/auth/registrations/_status.html.haml
@@ -0,0 +1,16 @@
+%h3= t('auth.status.account_status')
+
+- if @user.account.suspended?
+  %span.negative-hint= t('user_mailer.warning.explanation.suspend')
+- elsif @user.disabled?
+  %span.negative-hint= t('user_mailer.warning.explanation.disable')
+- elsif @user.account.silenced?
+  %span.warning-hint= t('user_mailer.warning.explanation.silence')
+- elsif !@user.confirmed?
+  %span.warning-hint= t('auth.status.confirming')
+- elsif !@user.approved?
+  %span.warning-hint= t('auth.status.pending')
+- else
+  %span.positive-hint= t('auth.status.functional')
+
+%hr.spacer/
diff --git a/app/views/auth/registrations/edit.html.haml b/app/views/auth/registrations/edit.html.haml
index 694461fdf..710ee5c68 100644
--- a/app/views/auth/registrations/edit.html.haml
+++ b/app/views/auth/registrations/edit.html.haml
@@ -1,25 +1,28 @@
 - content_for :page_title do
-  = t('auth.security')
+  = t('settings.account_settings')
+
+= render 'status'
+
+%h3= t('auth.security')
 
 = simple_form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put, class: 'auth_edit' }) do |f|
   = render 'shared/error_messages', object: resource
 
   - if !use_seamless_external_login? || resource.encrypted_password.present?
-    .fields-group
-      = f.input :email, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, required: true, hint: false
-
-    .fields-group
-      = f.input :current_password, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.current_password'), :autocomplete => 'off' }, required: true
-
-    .fields-group
-      = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.new_password'), :autocomplete => 'off' }, hint: false
-
-    .fields-group
-      = f.input :password_confirmation, wrapper: :with_label, label: t('simple_form.labels.defaults.confirm_new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.confirm_new_password'), :autocomplete => 'off' }
-
+    .fields-row
+      .fields-row__column.fields-group.fields-row__column-6
+        = f.input :email, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, required: true, disabled: current_account.suspended?
+      .fields-row__column.fields-group.fields-row__column-6
+        = f.input :current_password, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.current_password'), :autocomplete => 'off' }, required: true, disabled: current_account.suspended?
+
+    .fields-row
+      .fields-row__column.fields-group.fields-row__column-6
+        = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.new_password'), :autocomplete => 'off' }, hint: t('simple_form.hints.defaults.password'), disabled: current_account.suspended?
+      .fields-row__column.fields-group.fields-row__column-6
+        = f.input :password_confirmation, wrapper: :with_label, label: t('simple_form.labels.defaults.confirm_new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.confirm_new_password'), :autocomplete => 'off' }, disabled: current_account.suspended?
 
     .actions
-      = f.button :button, t('generic.save_changes'), type: :submit
+      = f.button :button, t('generic.save_changes'), type: :submit, class: 'button', disabled: current_account.suspended?
   - else
     %p.hint= t('users.seamless_external_login')
 
@@ -27,7 +30,7 @@
 
 = render 'sessions'
 
-- if open_deletion?
+- if open_deletion? && !current_account.suspended?
   %hr.spacer/
-  %h4= t('auth.delete_account')
+  %h3= t('auth.delete_account')
   %p.muted-hint= t('auth.delete_account_html', path: settings_delete_path)
diff --git a/app/views/auth/setup/show.html.haml b/app/views/auth/setup/show.html.haml
new file mode 100644
index 000000000..8bb44ca7f
--- /dev/null
+++ b/app/views/auth/setup/show.html.haml
@@ -0,0 +1,23 @@
+- content_for :page_title do
+  = t('auth.setup.title')
+
+- if missing_email?
+  = simple_form_for(@user, url: auth_setup_path) do |f|
+    = render 'shared/error_messages', object: @user
+
+    .fields-group
+      %p.hint= t('auth.setup.email_below_hint_html')
+
+    .fields-group
+      = f.input :email, required: true, hint: false, input_html: { 'aria-label' => t('simple_form.labels.defaults.email'), :autocomplete => 'off' }
+
+    .actions
+      = f.submit t('admin.accounts.change_email.label'), class: 'button'
+- else
+  .simple_form
+    %p.hint= t('auth.setup.email_settings_hint_html', email: content_tag(:strong, @user.email))
+
+.form-footer
+  %ul.no-list
+    %li= link_to t('settings.account_settings'), edit_user_registration_path
+    %li= link_to t('auth.logout'), destroy_user_session_path, data: { method: :delete }
diff --git a/app/views/oauth/authorized_applications/index.html.haml b/app/views/oauth/authorized_applications/index.html.haml
index 19af5f55d..7203d758d 100644
--- a/app/views/oauth/authorized_applications/index.html.haml
+++ b/app/views/oauth/authorized_applications/index.html.haml
@@ -17,7 +17,7 @@
               = application.name
             - else
               = link_to application.name, application.website, target: '_blank', rel: 'noopener'
-          %th!= application.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.join('<br />')
+          %th!= application.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.join(', ')
           %td= l application.created_at
           %td
             - unless application.superapp?