0
0
Fork 0

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.
This commit is contained in:
Eugen Rochko 2019-07-22 10:48:50 +02:00 committed by GitHub
parent fea903f574
commit 964ae8eee5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
35 changed files with 297 additions and 147 deletions

View file

@ -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,

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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