From 964ae8eee593687f922c873fa7b378bb6e3e39bb Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Mon, 22 Jul 2019 10:48:50 +0200
Subject: [PATCH] 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.
---
app/controllers/about_controller.rb | 2 +-
app/controllers/api/base_controller.rb | 2 +-
app/controllers/application_controller.rb | 6 +-
.../auth/confirmations_controller.rb | 21 +------
.../auth/omniauth_callbacks_controller.rb | 2 +-
.../auth/registrations_controller.rb | 9 ++-
app/controllers/auth/sessions_controller.rb | 4 +-
app/controllers/auth/setup_controller.rb | 58 +++++++++++++++++++
.../authorized_applications_controller.rb | 2 +
.../settings/deletes_controller.rb | 7 +++
.../settings/sessions_controller.rb | 2 +
.../confirmations_controller.rb | 2 +
.../recovery_codes_controller.rb | 2 +
.../two_factor_authentications_controller.rb | 2 +
app/javascript/styles/mastodon/admin.scss | 58 +++++++++++--------
app/javascript/styles/mastodon/forms.scss | 7 +++
app/models/concerns/omniauthable.rb | 2 +-
app/models/user.rb | 6 +-
.../confirmations/finish_signup.html.haml | 15 -----
.../auth/registrations/_sessions.html.haml | 4 +-
.../auth/registrations/_status.html.haml | 16 +++++
app/views/auth/registrations/edit.html.haml | 35 ++++++-----
app/views/auth/setup/show.html.haml | 23 ++++++++
.../authorized_applications/index.html.haml | 2 +-
config/locales/en.yml | 9 ++-
config/routes.rb | 5 +-
db/seeds.rb | 2 +-
spec/controllers/api/base_controller_spec.rb | 42 +++++++++++++-
.../application_controller_spec.rb | 4 +-
.../auth/confirmations_controller_spec.rb | 41 -------------
.../auth/registrations_controller_spec.rb | 25 +++++---
.../auth/sessions_controller_spec.rb | 4 +-
.../settings/deletes_controller_spec.rb | 17 ++++++
spec/features/log_in_spec.rb | 4 +-
spec/models/user_spec.rb | 4 +-
35 files changed, 298 insertions(+), 148 deletions(-)
create mode 100644 app/controllers/auth/setup_controller.rb
delete mode 100644 app/views/auth/confirmations/finish_signup.html.haml
create mode 100644 app/views/auth/registrations/_status.html.haml
create mode 100644 app/views/auth/setup/show.html.haml
diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 33bac9bbc7..31cf177105 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 eca558f421..6f33a1ea99 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 b8a1faf77e..41ce1a0ca2 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 c28c7471c0..0d7c6e7c2d 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 bbf63bed30..682c77016f 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 83797cf1f7..019caf9c1a 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 fb8615c313..7e6dbf19e8 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 0000000000..46c5f29581
--- /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 f3d2353669..fb8389034b 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 dd19aadf63..97fe4d3281 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 84ebb21f2c..df5ace8036 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 02652a36c9..3145e092da 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 874bf532ba..09a759860e 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 e12c430746..6904076e42 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 373a102603..f625bc1398 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 456ee4e0d3..ac99124ea8 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 2830330839..b9c124841b 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 31c99630c3..474c77293c 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 9d09b74e16..0000000000
--- 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 d7d96a1bb3..395e36a9fd 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 0000000000..b38a83d67d
--- /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 694461fdf2..710ee5c689 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 0000000000..8bb44ca7f7
--- /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 19af5f55db..7203d758da 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?
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 89c52b84ae..9e1be87be1 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -524,7 +524,6 @@ en:
apply_for_account: Request an invite
change_password: Password
checkbox_agreement_html: I agree to the <a href="%{rules_path}" target="_blank">server rules</a> and <a href="%{terms_path}" target="_blank">terms of service</a>
- confirm_email: Confirm email
delete_account: Delete account
delete_account_html: If you wish to delete your account, you can <a href="%{path}">proceed here</a>. You will be asked for confirmation.
didnt_get_confirmation: Didn't receive confirmation instructions?
@@ -544,6 +543,14 @@ en:
reset_password: Reset password
security: Security
set_new_password: Set new password
+ setup:
+ email_below_hint_html: If the below e-mail address is incorrect, you can change it here and receive a new confirmation e-mail.
+ email_settings_hint_html: The confirmation e-mail was sent to %{email}. If that e-mail address is not correct, you can change it in account settings.
+ title: Setup
+ status:
+ account_status: Account status
+ confirming: Waiting for e-mail confirmation to be completed.
+ pending: Your application is pending review by our staff. This may take some time. You will receive an e-mail if your application is approved.
trouble_logging_in: Trouble logging in?
authorize_follow:
already_following: You are already following this account
diff --git a/config/routes.rb b/config/routes.rb
index 27b5366419..b6c215888a 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -34,7 +34,10 @@ Rails.application.routes.draw do
devise_scope :user do
get '/invite/:invite_code', to: 'auth/registrations#new', as: :public_invite
- match '/auth/finish_signup' => 'auth/confirmations#finish_signup', via: [:get, :patch], as: :finish_signup
+
+ namespace :auth do
+ resource :setup, only: [:show, :update], controller: :setup
+ end
end
devise_for :users, path: 'auth', controllers: {
diff --git a/db/seeds.rb b/db/seeds.rb
index b112cf0738..0bfb5d0db5 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -1,4 +1,4 @@
-Doorkeeper::Application.create!(name: 'Web', superapp: true, redirect_uri: Doorkeeper.configuration.native_redirect_uri, scopes: 'read write follow')
+Doorkeeper::Application.create!(name: 'Web', superapp: true, redirect_uri: Doorkeeper.configuration.native_redirect_uri, scopes: 'read write follow push')
domain = ENV['LOCAL_DOMAIN'] || Rails.configuration.x.local_domain
account = Account.find_or_initialize_by(id: -99, actor_type: 'Application', locked: true, username: domain)
diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb
index 750ccc8cf6..05a42d1c19 100644
--- a/spec/controllers/api/base_controller_spec.rb
+++ b/spec/controllers/api/base_controller_spec.rb
@@ -15,7 +15,7 @@ describe Api::BaseController do
end
end
- describe 'Forgery protection' do
+ describe 'forgery protection' do
before do
routes.draw { post 'success' => 'api/base#success' }
end
@@ -27,7 +27,45 @@ describe Api::BaseController do
end
end
- describe 'Error handling' do
+ describe 'non-functional accounts handling' do
+ let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
+ let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read') }
+
+ controller do
+ before_action :require_user!
+ end
+
+ before do
+ routes.draw { post 'success' => 'api/base#success' }
+ allow(controller).to receive(:doorkeeper_token) { token }
+ end
+
+ it 'returns http forbidden for unconfirmed accounts' do
+ user.update(confirmed_at: nil)
+ post 'success'
+ expect(response).to have_http_status(403)
+ end
+
+ it 'returns http forbidden for pending accounts' do
+ user.update(approved: false)
+ post 'success'
+ expect(response).to have_http_status(403)
+ end
+
+ it 'returns http forbidden for disabled accounts' do
+ user.update(disabled: true)
+ post 'success'
+ expect(response).to have_http_status(403)
+ end
+
+ it 'returns http forbidden for suspended accounts' do
+ user.account.suspend!
+ post 'success'
+ expect(response).to have_http_status(403)
+ end
+ end
+
+ describe 'error handling' do
ERRORS_WITH_CODES = {
ActiveRecord::RecordInvalid => 422,
Mastodon::ValidationError => 422,
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 27946b60f9..1811500dfe 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -187,10 +187,10 @@ describe ApplicationController, type: :controller do
expect(response).to have_http_status(200)
end
- it 'returns http 403 if user who signed in is suspended' do
+ it 'redirects to account status page' do
sign_in(Fabricate(:user, account: Fabricate(:account, suspended: true)))
get 'success'
- expect(response).to have_http_status(403)
+ expect(response).to redirect_to(edit_user_registration_path)
end
end
diff --git a/spec/controllers/auth/confirmations_controller_spec.rb b/spec/controllers/auth/confirmations_controller_spec.rb
index e9a471fc5a..0b6b74ff90 100644
--- a/spec/controllers/auth/confirmations_controller_spec.rb
+++ b/spec/controllers/auth/confirmations_controller_spec.rb
@@ -50,45 +50,4 @@ describe Auth::ConfirmationsController, type: :controller do
end
end
end
-
- describe 'GET #finish_signup' do
- subject { get :finish_signup }
-
- let(:user) { Fabricate(:user) }
- before do
- sign_in user, scope: :user
- @request.env['devise.mapping'] = Devise.mappings[:user]
- end
-
- it 'renders finish_signup' do
- is_expected.to render_template :finish_signup
- expect(assigns(:user)).to have_attributes id: user.id
- end
- end
-
- describe 'PATCH #finish_signup' do
- subject { patch :finish_signup, params: { user: { email: email } } }
-
- let(:user) { Fabricate(:user) }
- before do
- sign_in user, scope: :user
- @request.env['devise.mapping'] = Devise.mappings[:user]
- end
-
- context 'when email is valid' do
- let(:email) { 'new_' + user.email }
-
- it 'redirects to root_path' do
- is_expected.to redirect_to root_path
- end
- end
-
- context 'when email is invalid' do
- let(:email) { '' }
-
- it 'renders finish_signup' do
- is_expected.to render_template :finish_signup
- end
- end
- end
end
diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb
index a4337039e1..3e11b34b53 100644
--- a/spec/controllers/auth/registrations_controller_spec.rb
+++ b/spec/controllers/auth/registrations_controller_spec.rb
@@ -46,6 +46,15 @@ RSpec.describe Auth::RegistrationsController, type: :controller do
post :update
expect(response).to have_http_status(200)
end
+
+ context 'when suspended' do
+ it 'returns http forbidden' do
+ request.env["devise.mapping"] = Devise.mappings[:user]
+ sign_in(Fabricate(:user, account_attributes: { username: 'test', suspended_at: Time.now.utc }), scope: :user)
+ post :update
+ expect(response).to have_http_status(403)
+ end
+ end
end
describe 'GET #new' do
@@ -94,9 +103,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do
post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678' } }
end
- it 'redirects to login page' do
+ it 'redirects to setup' do
subject
- expect(response).to redirect_to new_user_session_path
+ expect(response).to redirect_to auth_setup_path
end
it 'creates user' do
@@ -120,9 +129,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do
post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678' } }
end
- it 'redirects to login page' do
+ it 'redirects to setup' do
subject
- expect(response).to redirect_to new_user_session_path
+ expect(response).to redirect_to auth_setup_path
end
it 'creates user' do
@@ -148,9 +157,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do
post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code } }
end
- it 'redirects to login page' do
+ it 'redirects to setup' do
subject
- expect(response).to redirect_to new_user_session_path
+ expect(response).to redirect_to auth_setup_path
end
it 'creates user' do
@@ -176,9 +185,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do
post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code } }
end
- it 'redirects to login page' do
+ it 'redirects to setup' do
subject
- expect(response).to redirect_to new_user_session_path
+ expect(response).to redirect_to auth_setup_path
end
it 'creates user' do
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index 71fcc1a6e3..87ef4f2bb2 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -160,8 +160,8 @@ RSpec.describe Auth::SessionsController, type: :controller do
let(:unconfirmed_user) { user.tap { |u| u.update!(confirmed_at: nil) } }
let(:accept_language) { 'fr' }
- it 'shows a translated login error' do
- expect(flash[:alert]).to eq(I18n.t('devise.failure.unconfirmed', locale: accept_language))
+ it 'redirects to home' do
+ expect(response).to redirect_to(root_path)
end
end
diff --git a/spec/controllers/settings/deletes_controller_spec.rb b/spec/controllers/settings/deletes_controller_spec.rb
index 35fd64e9b9..996872efd1 100644
--- a/spec/controllers/settings/deletes_controller_spec.rb
+++ b/spec/controllers/settings/deletes_controller_spec.rb
@@ -15,6 +15,15 @@ describe Settings::DeletesController do
get :show
expect(response).to have_http_status(200)
end
+
+ context 'when suspended' do
+ let(:user) { Fabricate(:user, account_attributes: { username: 'alice', suspended_at: Time.now.utc }) }
+
+ it 'returns http forbidden' do
+ get :show
+ expect(response).to have_http_status(403)
+ end
+ end
end
context 'when not signed in' do
@@ -49,6 +58,14 @@ describe Settings::DeletesController do
it 'marks account as suspended' do
expect(user.account.reload).to be_suspended
end
+
+ context 'when suspended' do
+ let(:user) { Fabricate(:user, account_attributes: { username: 'alice', suspended_at: Time.now.utc }) }
+
+ it 'returns http forbidden' do
+ expect(response).to have_http_status(403)
+ end
+ end
end
context 'with incorrect password' do
diff --git a/spec/features/log_in_spec.rb b/spec/features/log_in_spec.rb
index 53a1f9b126..f6c26cd0f1 100644
--- a/spec/features/log_in_spec.rb
+++ b/spec/features/log_in_spec.rb
@@ -31,12 +31,12 @@ feature "Log in" do
context do
given(:confirmed_at) { nil }
- scenario "A unconfirmed user is not able to log in" do
+ scenario "A unconfirmed user is able to log in" do
fill_in "user_email", with: email
fill_in "user_password", with: password
click_on I18n.t('auth.login')
- is_expected.to have_css(".flash-message", text: failure_message("unconfirmed"))
+ is_expected.to have_css("div.admin-wrapper")
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 856254ce4b..d7c0b53593 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -506,7 +506,7 @@ RSpec.describe User, type: :model do
context 'when user is not confirmed' do
let(:confirmed_at) { nil }
- it { is_expected.to be false }
+ it { is_expected.to be true }
end
end
@@ -522,7 +522,7 @@ RSpec.describe User, type: :model do
context 'when user is not confirmed' do
let(:confirmed_at) { nil }
- it { is_expected.to be false }
+ it { is_expected.to be true }
end
end
end
--
GitLab