about summary refs log tree commit diff
path: root/spec
diff options
context:
space:
mode:
authorEugen Rochko <eugen@zeonfederated.com>2019-07-22 10:48:50 +0200
committerGitHub <noreply@github.com>2019-07-22 10:48:50 +0200
commit964ae8eee593687f922c873fa7b378bb6e3e39bb (patch)
treeeb67d6521d6cecc6679e75800c4d170ea5883fa0 /spec
parentfea903f574cd59e6938c775427727337d7f929c3 (diff)
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.
Diffstat (limited to 'spec')
-rw-r--r--spec/controllers/api/base_controller_spec.rb42
-rw-r--r--spec/controllers/application_controller_spec.rb4
-rw-r--r--spec/controllers/auth/confirmations_controller_spec.rb41
-rw-r--r--spec/controllers/auth/registrations_controller_spec.rb25
-rw-r--r--spec/controllers/auth/sessions_controller_spec.rb4
-rw-r--r--spec/controllers/settings/deletes_controller_spec.rb17
-rw-r--r--spec/features/log_in_spec.rb4
-rw-r--r--spec/models/user_spec.rb4
8 files changed, 82 insertions, 59 deletions
diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb
index 750ccc8cf..05a42d1c1 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 27946b60f..1811500df 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 e9a471fc5..0b6b74ff9 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 a4337039e..3e11b34b5 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 71fcc1a6e..87ef4f2bb 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 35fd64e9b..996872efd 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 53a1f9b12..f6c26cd0f 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 856254ce4..d7c0b5359 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