From 5c21021176813313e656bf1c92177a930a1ca9cc Mon Sep 17 00:00:00 2001
From: Daniel <git@baby-gnu.org>
Date: Wed, 25 Aug 2021 15:40:56 +0000
Subject: Fix undefined variable for Auth::OmniauthCallbacksController (#16654)

The addition of authentication history broke the omniauth login with
the following error:

  method=GET path=/auth/auth/cas/callback format=html
  controller=Auth::OmniauthCallbacksController action=cas status=500
  error='NameError: undefined local variable or method `user' for
  #<Auth::OmniauthCallbacksController:0x00000000036290>
  Did you mean?  @user' duration=435.93 view=0.00 db=36.19

* app/controllers/auth/omniauth_callbacks_controller.rb: fix variable
  name to `@user`
---
 app/controllers/auth/omniauth_callbacks_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb
index 7925e23cb..991a50b03 100644
--- a/app/controllers/auth/omniauth_callbacks_controller.rb
+++ b/app/controllers/auth/omniauth_callbacks_controller.rb
@@ -11,7 +11,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
       if @user.persisted?
         LoginActivity.create(
-          user: user,
+          user: @user,
           success: true,
           authentication_method: :omniauth,
           provider: provider,
-- 
cgit 


From 94bcf453219da73015cc977835717516b9dc0a67 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Wed, 25 Aug 2021 22:52:41 +0200
Subject: Fix authentication failures after going halfway through a sign-in
 attempt (#16607)

* Add tests

* Add security-related tests

My first (unpublished) attempt at fixing the issues introduced (extremely
hard-to-exploit) security vulnerabilities, addressing them in a test.

* Fix authentication failures after going halfway through a sign-in attempt

* Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
---
 app/controllers/auth/sessions_controller.rb        |  16 +--
 .../sign_in_token_authentication_concern.rb        |  20 ++--
 .../concerns/two_factor_authentication_concern.rb  |  22 +++--
 spec/controllers/auth/sessions_controller_spec.rb  | 109 +++++++++++++++++++++
 4 files changed, 144 insertions(+), 23 deletions(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 9c73b39e2..7afd09e10 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -58,16 +58,20 @@ class Auth::SessionsController < Devise::SessionsController
   protected
 
   def find_user
-    if session[:attempt_user_id]
+    if user_params[:email].present?
+      find_user_from_params
+    elsif session[:attempt_user_id]
       User.find_by(id: session[:attempt_user_id])
-    else
-      user   = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication
-      user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication
-      user ||= User.find_for_authentication(email: user_params[:email])
-      user
     end
   end
 
+  def find_user_from_params
+    user   = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication
+    user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication
+    user ||= User.find_for_authentication(email: user_params[:email])
+    user
+  end
+
   def user_params
     params.require(:user).permit(:email, :password, :otp_attempt, :sign_in_token_attempt, credential: {})
   end
diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb
index cbee84569..384c5c50c 100644
--- a/app/controllers/concerns/sign_in_token_authentication_concern.rb
+++ b/app/controllers/concerns/sign_in_token_authentication_concern.rb
@@ -16,14 +16,18 @@ module SignInTokenAuthenticationConcern
   end
 
   def authenticate_with_sign_in_token
-    user = self.resource = find_user
-
-    if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s
-      restart_session
-    elsif user_params.key?(:sign_in_token_attempt) && session[:attempt_user_id]
-      authenticate_with_sign_in_token_attempt(user)
-    elsif user.present? && user.external_or_valid_password?(user_params[:password])
-      prompt_for_sign_in_token(user)
+    if user_params[:email].present?
+      user = self.resource = find_user_from_params
+      prompt_for_sign_in_token(user) if user&.external_or_valid_password?(user_params[:password])
+    elsif session[:attempt_user_id]
+      user = self.resource = User.find_by(id: session[:attempt_user_id])
+      return if user.nil?
+
+      if session[:attempt_user_updated_at] != user.updated_at.to_s
+        restart_session
+      elsif user_params.key?(:sign_in_token_attempt)
+        authenticate_with_sign_in_token_attempt(user)
+      end
     end
   end
 
diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb
index 909ab7717..2583d324b 100644
--- a/app/controllers/concerns/two_factor_authentication_concern.rb
+++ b/app/controllers/concerns/two_factor_authentication_concern.rb
@@ -35,16 +35,20 @@ module TwoFactorAuthenticationConcern
   end
 
   def authenticate_with_two_factor
-    user = self.resource = find_user
+    if user_params[:email].present?
+      user = self.resource = find_user_from_params
+      prompt_for_two_factor(user) if user&.external_or_valid_password?(user_params[:password])
+    elsif session[:attempt_user_id]
+      user = self.resource = User.find_by(id: session[:attempt_user_id])
+      return if user.nil?
 
-    if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s
-      restart_session
-    elsif user.webauthn_enabled? && user_params.key?(:credential) && session[:attempt_user_id]
-      authenticate_with_two_factor_via_webauthn(user)
-    elsif user_params.key?(:otp_attempt) && session[:attempt_user_id]
-      authenticate_with_two_factor_via_otp(user)
-    elsif user.present? && user.external_or_valid_password?(user_params[:password])
-      prompt_for_two_factor(user)
+      if session[:attempt_user_updated_at] != user.updated_at.to_s
+        restart_session
+      elsif user.webauthn_enabled? && user_params.key?(:credential)
+        authenticate_with_two_factor_via_webauthn(user)
+      elsif user_params.key?(:otp_attempt)
+        authenticate_with_two_factor_via_otp(user)
+      end
     end
   end
 
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index d03ae51e8..051a0807d 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -206,6 +206,38 @@ RSpec.describe Auth::SessionsController, type: :controller do
           end
         end
 
+        context 'using email and password after an unfinished log-in attempt to a 2FA-protected account' do
+          let!(:other_user) do
+            Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32))
+          end
+
+          before do
+            post :create, params: { user: { email: other_user.email, password: other_user.password } }
+            post :create, params: { user: { email: user.email, password: user.password } }
+          end
+
+          it 'renders two factor authentication page' do
+            expect(controller).to render_template("two_factor")
+            expect(controller).to render_template(partial: "_otp_authentication_form")
+          end
+        end
+
+        context 'using email and password after an unfinished log-in attempt with a sign-in token challenge' do
+          let!(:other_user) do
+            Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
+          end
+
+          before do
+            post :create, params: { user: { email: other_user.email, password: other_user.password } }
+            post :create, params: { user: { email: user.email, password: user.password } }
+          end
+
+          it 'renders two factor authentication page' do
+            expect(controller).to render_template("two_factor")
+            expect(controller).to render_template(partial: "_otp_authentication_form")
+          end
+        end
+
         context 'using upcase email and password' do
           before do
             post :create, params: { user: { email: user.email.upcase, password: user.password } }
@@ -231,6 +263,21 @@ RSpec.describe Auth::SessionsController, type: :controller do
           end
         end
 
+        context 'using a valid OTP, attempting to leverage previous half-login to bypass password auth' do
+          let!(:other_user) do
+            Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
+          end
+
+          before do
+            post :create, params: { user: { email: other_user.email, password: other_user.password } }
+            post :create, params: { user: { email: user.email, otp_attempt: user.current_otp } }, session: { attempt_user_updated_at: user.updated_at.to_s }
+          end
+
+          it "doesn't log the user in" do
+            expect(controller.current_user).to be_nil
+          end
+        end
+
         context 'when the server has an decryption error' do
           before do
             allow_any_instance_of(User).to receive(:validate_and_consume_otp!).and_raise(OpenSSL::Cipher::CipherError)
@@ -380,6 +427,52 @@ RSpec.describe Auth::SessionsController, type: :controller do
         end
       end
 
+      context 'using email and password after an unfinished log-in attempt to a 2FA-protected account' do
+        let!(:other_user) do
+          Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32))
+        end
+
+        before do
+          post :create, params: { user: { email: other_user.email, password: other_user.password } }
+          post :create, params: { user: { email: user.email, password: user.password } }
+        end
+
+        it 'renders sign in token authentication page' do
+          expect(controller).to render_template("sign_in_token")
+        end
+
+        it 'generates sign in token' do
+          expect(user.reload.sign_in_token).to_not be_nil
+        end
+
+        it 'sends sign in token e-mail' do
+          expect(UserMailer).to have_received(:sign_in_token)
+        end
+      end
+
+      context 'using email and password after an unfinished log-in attempt with a sign-in token challenge' do
+        let!(:other_user) do
+          Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
+        end
+
+        before do
+          post :create, params: { user: { email: other_user.email, password: other_user.password } }
+          post :create, params: { user: { email: user.email, password: user.password } }
+        end
+
+        it 'renders sign in token authentication page' do
+          expect(controller).to render_template("sign_in_token")
+        end
+
+        it 'generates sign in token' do
+          expect(user.reload.sign_in_token).to_not be_nil
+        end
+
+        it 'sends sign in token e-mail' do
+          expect(UserMailer).to have_received(:sign_in_token).with(user, any_args)
+        end
+      end
+
       context 'using a valid sign in token' do
         before do
           user.generate_sign_in_token && user.save
@@ -395,6 +488,22 @@ RSpec.describe Auth::SessionsController, type: :controller do
         end
       end
 
+      context 'using a valid sign in token, attempting to leverage previous half-login to bypass password auth' do
+        let!(:other_user) do
+          Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
+        end
+
+        before do
+          user.generate_sign_in_token && user.save
+          post :create, params: { user: { email: other_user.email, password: other_user.password } }
+          post :create, params: { user: { email: user.email, sign_in_token_attempt: user.sign_in_token } }, session: { attempt_user_updated_at: user.updated_at.to_s }
+        end
+
+        it "doesn't log the user in" do
+          expect(controller.current_user).to be_nil
+        end
+      end
+
       context 'using an invalid sign in token' do
         before do
           post :create, params: { user: { sign_in_token_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s }
-- 
cgit 


From 7283a5d3b94b655172744996ffa43ec80aff0e08 Mon Sep 17 00:00:00 2001
From: Truong Nguyen <truongnmt.dev@gmail.com>
Date: Thu, 26 Aug 2021 23:51:22 +0900
Subject: Explicitly set userVerification to discoraged (#16545)

---
 app/controllers/auth/sessions_controller.rb                          | 5 ++++-
 .../two_factor_authentication/webauthn_credentials_controller.rb     | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 7afd09e10..2c3d510cb 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -45,7 +45,10 @@ class Auth::SessionsController < Devise::SessionsController
     user = find_user
 
     if user&.webauthn_enabled?
-      options_for_get = WebAuthn::Credential.options_for_get(allow: user.webauthn_credentials.pluck(:external_id))
+      options_for_get = WebAuthn::Credential.options_for_get(
+        allow: user.webauthn_credentials.pluck(:external_id),
+        user_verification: 'discouraged'
+      )
 
       session[:webauthn_challenge] = options_for_get.challenge
 
diff --git a/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb b/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb
index 1c557092b..a50d30f06 100644
--- a/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb
+++ b/app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb
@@ -21,7 +21,8 @@ module Settings
             display_name: current_user.account.username,
             id: current_user.webauthn_id,
           },
-          exclude: current_user.webauthn_credentials.pluck(:external_id)
+          exclude: current_user.webauthn_credentials.pluck(:external_id),
+          authenticator_selection: { user_verification: 'discouraged' }
         )
 
         session[:webauthn_challenge] = options_for_create.challenge
-- 
cgit 


From 24f9ea781850ee8d105aa3e396eaef8499bc5efc Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Thu, 30 Sep 2021 05:26:29 +0200
Subject: Fix webauthn secure key authentication (#16792)

* Add tests

* Fix webauthn secure key authentication

Fixes #16769
---
 app/controllers/auth/sessions_controller.rb       |  2 +-
 spec/controllers/auth/sessions_controller_spec.rb | 29 +++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 2c3d510cb..d48abb707 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -42,7 +42,7 @@ class Auth::SessionsController < Devise::SessionsController
   end
 
   def webauthn_options
-    user = find_user
+    user = User.find_by(id: session[:attempt_user_id])
 
     if user&.webauthn_enabled?
       options_for_get = WebAuthn::Credential.options_for_get(
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index 051a0807d..f718f5dd9 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -519,4 +519,33 @@ RSpec.describe Auth::SessionsController, type: :controller do
       end
     end
   end
+
+  describe 'GET #webauthn_options' do
+    context 'with WebAuthn and OTP enabled as second factor' do
+      let(:domain) { "#{Rails.configuration.x.use_https ? 'https' : 'http' }://#{Rails.configuration.x.web_domain}" }
+
+      let(:fake_client) { WebAuthn::FakeClient.new(domain) }
+
+      let!(:user) do
+        Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32))
+      end
+
+      before do
+        user.update(webauthn_id: WebAuthn.generate_user_id)
+        public_key_credential = WebAuthn::Credential.from_create(fake_client.create)
+        user.webauthn_credentials.create(
+          nickname: 'SecurityKeyNickname',
+          external_id: public_key_credential.id,
+          public_key: public_key_credential.public_key,
+          sign_count: '1000'
+        )
+        post :create, params: { user: { email: user.email, password: user.password } }
+      end
+
+      it 'returns http success' do
+        get :webauthn_options
+        expect(response).to have_http_status :ok
+      end
+    end
+  end
 end
-- 
cgit 


From 6da135a493cc039d92bb5925c2a1ef66025623bf Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Sat, 6 Nov 2021 00:13:58 +0100
Subject: Fix reviving revoked sessions and invalidating login (#16943)

Up until now, we have used Devise's Rememberable mechanism to re-log users
after the end of their browser sessions. This mechanism relies on a signed
cookie containing a token. That token was stored on the user's record,
meaning it was shared across all logged in browsers, meaning truly revoking
a browser's ability to auto-log-in involves revoking the token itself, and
revoking access from *all* logged-in browsers.

We had a session mechanism that dynamically checks whether a user's session
has been disabled, and would log out the user if so. However, this would only
clear a session being actively used, and a new one could be respawned with
the `remember_user_token` cookie.

In practice, this caused two issues:
- sessions could be revived after being closed from /auth/edit (security issue)
- auto-log-in would be disabled for *all* browsers after logging out from one
  of them

This PR removes the `remember_token` mechanism and treats the `_session_id`
cookie/token as a browser-specific `remember_token`, fixing both issues.
---
 app/controllers/auth/passwords_controller.rb     |  1 -
 app/controllers/auth/registrations_controller.rb |  3 --
 app/controllers/auth/sessions_controller.rb      |  3 --
 app/models/user.rb                               |  2 +-
 config/initializers/devise.rb                    | 39 ++++++++++++++++++++++--
 5 files changed, 37 insertions(+), 11 deletions(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb
index 5db2668f7..2996c0431 100644
--- a/app/controllers/auth/passwords_controller.rb
+++ b/app/controllers/auth/passwords_controller.rb
@@ -10,7 +10,6 @@ class Auth::PasswordsController < Devise::PasswordsController
     super do |resource|
       if resource.errors.empty?
         resource.session_activations.destroy_all
-        resource.forget_me!
       end
     end
   end
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index a3114ab25..3c1730f25 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 class Auth::RegistrationsController < Devise::RegistrationsController
-  include Devise::Controllers::Rememberable
   include RegistrationSpamConcern
 
   layout :determine_layout
@@ -30,8 +29,6 @@ class Auth::RegistrationsController < Devise::RegistrationsController
     super do |resource|
       if resource.saved_change_to_encrypted_password?
         resource.clear_other_sessions(current_session.session_id)
-        resource.forget_me!
-        remember_me(resource)
       end
     end
   end
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index d48abb707..0184bfb52 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class Auth::SessionsController < Devise::SessionsController
-  include Devise::Controllers::Rememberable
-
   layout 'auth'
 
   skip_before_action :require_no_authentication, only: [:create]
@@ -150,7 +148,6 @@ class Auth::SessionsController < Devise::SessionsController
     clear_attempt_from_session
 
     user.update_sign_in!(request, new_sign_in: true)
-    remember_me(user)
     sign_in(user)
     flash.delete(:notice)
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 4059c96b5..c4dec4813 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -64,7 +64,7 @@ class User < ApplicationRecord
   devise :two_factor_backupable,
          otp_number_of_backup_codes: 10
 
-  devise :registerable, :recoverable, :rememberable, :validatable,
+  devise :registerable, :recoverable, :validatable,
          :confirmable
 
   include Omniauthable
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index ef612e177..5232e6cfd 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -1,3 +1,5 @@
+require 'devise/strategies/authenticatable'
+
 Warden::Manager.after_set_user except: :fetch do |user, warden|
   if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
     session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
@@ -72,17 +74,48 @@ module Devise
   mattr_accessor :ldap_uid_conversion_replace
   @@ldap_uid_conversion_replace = nil
 
-  class Strategies::PamAuthenticatable
-    def valid?
-      super && ::Devise.pam_authentication
+  module Strategies
+    class PamAuthenticatable
+      def valid?
+        super && ::Devise.pam_authentication
+      end
+    end
+
+    class SessionActivationRememberable < Authenticatable
+      def valid?
+        @session_cookie = nil
+        session_cookie.present?
+      end
+
+      def authenticate!
+        resource = SessionActivation.find_by(session_id: session_cookie)&.user
+
+        unless resource
+          cookies.delete('_session_id')
+          return pass
+        end
+
+        if validate(resource)
+          success!(resource)
+        end
+      end
+
+      private
+
+      def session_cookie
+        @session_cookie ||= cookies.signed['_session_id']
+      end
     end
   end
 end
 
+Warden::Strategies.add(:session_activation_rememberable, Devise::Strategies::SessionActivationRememberable)
+
 Devise.setup do |config|
   config.warden do |manager|
     manager.default_strategies(scope: :user).unshift :two_factor_ldap_authenticatable if Devise.ldap_authentication
     manager.default_strategies(scope: :user).unshift :two_factor_pam_authenticatable  if Devise.pam_authentication
+    manager.default_strategies(scope: :user).unshift :session_activation_rememberable
     manager.default_strategies(scope: :user).unshift :two_factor_authenticatable
     manager.default_strategies(scope: :user).unshift :two_factor_backupable
   end
-- 
cgit 


From 8e84ebf0cb211c1d94145399b05c9f2ad0e4d4b0 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sun, 16 Jan 2022 13:23:50 +0100
Subject: Remove IP tracking columns from users table (#16409)

---
 .../api/v1/admin/accounts_controller.rb            |  2 +-
 app/controllers/auth/sessions_controller.rb        |  2 +-
 app/controllers/concerns/user_tracking_concern.rb  |  6 +--
 app/helpers/admin/dashboard_helper.rb              | 10 ++--
 app/models/account.rb                              |  1 -
 app/models/account_filter.rb                       |  2 +-
 app/models/user.rb                                 | 58 ++++++----------------
 app/models/user_ip.rb                              | 19 +++++++
 app/serializers/rest/admin/account_serializer.rb   | 13 +++--
 app/serializers/rest/admin/ip_serializer.rb        |  5 ++
 app/views/admin/accounts/show.html.haml            | 10 ++--
 .../admin_mailer/new_pending_account.text.erb      |  2 +-
 app/workers/scheduler/ip_cleanup_scheduler.rb      |  2 +-
 config/initializers/devise.rb                      | 15 +++---
 db/migrate/20210616214526_create_user_ips.rb       |  5 ++
 ...6214135_remove_current_sign_in_ip_from_users.rb | 12 +++++
 db/schema.rb                                       | 24 ++++++++-
 db/views/user_ips_v01.sql                          | 26 ++++++++++
 spec/controllers/auth/sessions_controller_spec.rb  |  2 +-
 19 files changed, 141 insertions(+), 75 deletions(-)
 create mode 100644 app/models/user_ip.rb
 create mode 100644 app/serializers/rest/admin/ip_serializer.rb
 create mode 100644 db/migrate/20210616214526_create_user_ips.rb
 create mode 100644 db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb
 create mode 100644 db/views/user_ips_v01.sql

(limited to 'app/controllers/auth')

diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb
index 63cc521ed..9b8f2fb05 100644
--- a/app/controllers/api/v1/admin/accounts_controller.rb
+++ b/app/controllers/api/v1/admin/accounts_controller.rb
@@ -94,7 +94,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController
   private
 
   def set_accounts
-    @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
+    @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite, :ips]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
   end
 
   def set_account
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 0184bfb52..3337a43c4 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -147,7 +147,7 @@ class Auth::SessionsController < Devise::SessionsController
 
     clear_attempt_from_session
 
-    user.update_sign_in!(request, new_sign_in: true)
+    user.update_sign_in!(new_sign_in: true)
     sign_in(user)
     flash.delete(:notice)
 
diff --git a/app/controllers/concerns/user_tracking_concern.rb b/app/controllers/concerns/user_tracking_concern.rb
index efda37fae..45f3aab0d 100644
--- a/app/controllers/concerns/user_tracking_concern.rb
+++ b/app/controllers/concerns/user_tracking_concern.rb
@@ -3,7 +3,7 @@
 module UserTrackingConcern
   extend ActiveSupport::Concern
 
-  UPDATE_SIGN_IN_HOURS = 24
+  UPDATE_SIGN_IN_FREQUENCY = 24.hours.freeze
 
   included do
     before_action :update_user_sign_in
@@ -12,10 +12,10 @@ module UserTrackingConcern
   private
 
   def update_user_sign_in
-    current_user.update_sign_in!(request) if user_needs_sign_in_update?
+    current_user.update_sign_in! if user_needs_sign_in_update?
   end
 
   def user_needs_sign_in_update?
-    user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_HOURS.hours.ago)
+    user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_FREQUENCY.ago)
   end
 end
diff --git a/app/helpers/admin/dashboard_helper.rb b/app/helpers/admin/dashboard_helper.rb
index 32aaf9f5e..d4a30b97e 100644
--- a/app/helpers/admin/dashboard_helper.rb
+++ b/app/helpers/admin/dashboard_helper.rb
@@ -2,17 +2,17 @@
 
 module Admin::DashboardHelper
   def relevant_account_ip(account, ip_query)
-    default_ip = [account.user_current_sign_in_ip || account.user_sign_up_ip]
+    ips = account.user.ips.to_a
 
     matched_ip = begin
       ip_query_addr = IPAddr.new(ip_query)
-      account.user.recent_ips.find { |(_, ip)| ip_query_addr.include?(ip) } || default_ip
+      ips.find { |ip| ip_query_addr.include?(ip.ip) } || ips.first
     rescue IPAddr::Error
-      default_ip
-    end.last
+      ips.first
+    end
 
     if matched_ip
-      link_to matched_ip, admin_accounts_path(ip: matched_ip)
+      link_to matched_ip.ip, admin_accounts_path(ip: matched_ip.ip)
     else
       '-'
     end
diff --git a/app/models/account.rb b/app/models/account.rb
index 238ea1d65..c459125c7 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -123,7 +123,6 @@ class Account < ApplicationRecord
 
   delegate :email,
            :unconfirmed_email,
-           :current_sign_in_ip,
            :current_sign_in_at,
            :created_at,
            :sign_up_ip,
diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb
index defd531ac..dcb174122 100644
--- a/app/models/account_filter.rb
+++ b/app/models/account_filter.rb
@@ -21,7 +21,7 @@ class AccountFilter
   end
 
   def results
-    scope = Account.includes(:account_stat, user: [:session_activations, :invite_request]).without_instance_actor.reorder(nil)
+    scope = Account.includes(:account_stat, user: [:ips, :invite_request]).without_instance_actor.reorder(nil)
 
     params.each do |key, value|
       scope.merge!(scope_for(key, value.to_s.strip)) if value.present?
diff --git a/app/models/user.rb b/app/models/user.rb
index 374b82d05..49dcb8156 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -14,8 +14,6 @@
 #  sign_in_count             :integer          default(0), not null
 #  current_sign_in_at        :datetime
 #  last_sign_in_at           :datetime
-#  current_sign_in_ip        :inet
-#  last_sign_in_ip           :inet
 #  admin                     :boolean          default(FALSE), not null
 #  confirmation_token        :string
 #  confirmed_at              :datetime
@@ -81,6 +79,7 @@ class User < ApplicationRecord
   has_many :invites, inverse_of: :user
   has_many :markers, inverse_of: :user, dependent: :destroy
   has_many :webauthn_credentials, dependent: :destroy
+  has_many :ips, class_name: 'UserIp', inverse_of: :user
 
   has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy
   accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text }
@@ -107,7 +106,7 @@ class User < ApplicationRecord
   scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) }
   scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended_at: nil }) }
   scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) }
-  scope :matches_ip, ->(value) { where('current_sign_in_ip <<= ?', value).or(where('users.sign_up_ip <<= ?', value)).or(where('users.last_sign_in_ip <<= ?', value)).or(where(id: SessionActivation.select(:user_id).where('ip <<= ?', value))) }
+  scope :matches_ip, ->(value) { left_joins(:ips).where('user_ips.ip <<= ?', value) }
   scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) }
 
   before_validation :sanitize_languages
@@ -174,15 +173,11 @@ class User < ApplicationRecord
     prepare_new_user! if new_user && approved?
   end
 
-  def update_sign_in!(request, new_sign_in: false)
+  def update_sign_in!(new_sign_in: false)
     old_current, new_current = current_sign_in_at, Time.now.utc
     self.last_sign_in_at     = old_current || new_current
     self.current_sign_in_at  = new_current
 
-    old_current, new_current = current_sign_in_ip, request.remote_ip
-    self.last_sign_in_ip     = old_current || new_current
-    self.current_sign_in_ip  = new_current
-
     if new_sign_in
       self.sign_in_count ||= 0
       self.sign_in_count  += 1
@@ -201,7 +196,7 @@ class User < ApplicationRecord
   end
 
   def suspicious_sign_in?(ip)
-    !otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !recent_ip?(ip)
+    !otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !ips.where(ip: ip).exists?
   end
 
   def functional?
@@ -277,31 +272,28 @@ class User < ApplicationRecord
     @shows_application ||= settings.show_application
   end
 
-  # rubocop:disable Naming/MethodParameterName
-  def token_for_app(a)
-    return nil if a.nil? || a.owner != self
-    Doorkeeper::AccessToken.find_or_create_by(application_id: a.id, resource_owner_id: id) do |t|
-      t.scopes = a.scopes
-      t.expires_in = Doorkeeper.configuration.access_token_expires_in
+  def token_for_app(app)
+    return nil if app.nil? || app.owner != self
+
+    Doorkeeper::AccessToken.find_or_create_by(application_id: app.id, resource_owner_id: id) do |t|
+      t.scopes            = app.scopes
+      t.expires_in        = Doorkeeper.configuration.access_token_expires_in
       t.use_refresh_token = Doorkeeper.configuration.refresh_token_enabled?
     end
   end
-  # rubocop:enable Naming/MethodParameterName
 
   def activate_session(request)
-    session_activations.activate(session_id: SecureRandom.hex,
-                                 user_agent: request.user_agent,
-                                 ip: request.remote_ip).session_id
+    session_activations.activate(
+      session_id: SecureRandom.hex,
+      user_agent: request.user_agent,
+      ip: request.remote_ip
+    ).session_id
   end
 
   def clear_other_sessions(id)
     session_activations.exclusive(id)
   end
 
-  def session_active?(id)
-    session_activations.active? id
-  end
-
   def web_push_subscription(session)
     session.web_push_subscription.nil? ? nil : session.web_push_subscription
   end
@@ -364,22 +356,6 @@ class User < ApplicationRecord
     setting_display_media == 'hide_all'
   end
 
-  def recent_ips
-    @recent_ips ||= begin
-      arr = []
-
-      session_activations.each do |session_activation|
-        arr << [session_activation.updated_at, session_activation.ip]
-      end
-
-      arr << [current_sign_in_at, current_sign_in_ip] if current_sign_in_ip.present?
-      arr << [last_sign_in_at, last_sign_in_ip] if last_sign_in_ip.present?
-      arr << [created_at, sign_up_ip] if sign_up_ip.present?
-
-      arr.sort_by { |pair| pair.first || Time.now.utc }.uniq(&:last).reverse!
-    end
-  end
-
   def sign_in_token_expired?
     sign_in_token_sent_at.nil? || sign_in_token_sent_at < 5.minutes.ago
   end
@@ -410,10 +386,6 @@ class User < ApplicationRecord
 
   private
 
-  def recent_ip?(ip)
-    recent_ips.any? { |(_, recent_ip)| recent_ip == ip }
-  end
-
   def send_pending_devise_notifications
     pending_devise_notifications.each do |notification, args, kwargs|
       render_and_send_devise_message(notification, *args, **kwargs)
diff --git a/app/models/user_ip.rb b/app/models/user_ip.rb
new file mode 100644
index 000000000..a8e802e13
--- /dev/null
+++ b/app/models/user_ip.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+# == Schema Information
+#
+# Table name: user_ips
+#
+#  user_id :bigint(8)        primary key
+#  ip      :inet
+#  used_at :datetime
+#
+
+class UserIp < ApplicationRecord
+  self.primary_key = :user_id
+
+  belongs_to :user, foreign_key: :user_id
+
+  def readonly?
+    true
+  end
+end
diff --git a/app/serializers/rest/admin/account_serializer.rb b/app/serializers/rest/admin/account_serializer.rb
index f579d3302..3480e8c5a 100644
--- a/app/serializers/rest/admin/account_serializer.rb
+++ b/app/serializers/rest/admin/account_serializer.rb
@@ -9,6 +9,7 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
   attribute :created_by_application_id, if: :created_by_application?
   attribute :invited_by_account_id, if: :invited?
 
+  has_many :ips, serializer: REST::Admin::IpSerializer
   has_one :account, serializer: REST::AccountSerializer
 
   def id
@@ -19,10 +20,6 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
     object.user_email
   end
 
-  def ip
-    object.user_current_sign_in_ip.to_s.presence
-  end
-
   def role
     object.user_role
   end
@@ -74,4 +71,12 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
   def created_by_application?
     object.user&.created_by_application_id&.present?
   end
+
+  def ips
+    object.user&.ips
+  end
+
+  def ip
+    ips&.first
+  end
 end
diff --git a/app/serializers/rest/admin/ip_serializer.rb b/app/serializers/rest/admin/ip_serializer.rb
new file mode 100644
index 000000000..d11699dc4
--- /dev/null
+++ b/app/serializers/rest/admin/ip_serializer.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class REST::Admin::IpSerializer < ActiveModel::Serializer
+  attributes :ip, :used_at
+end
diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml
index 64cfc9a77..3867d1b19 100644
--- a/app/views/admin/accounts/show.html.haml
+++ b/app/views/admin/accounts/show.html.haml
@@ -156,12 +156,14 @@
               %time.formatted{ datetime: @account.created_at.iso8601, title: l(@account.created_at) }= l @account.created_at
             %td
 
-          - @account.user.recent_ips.each_with_index do |(_, ip), i|
+          - recent_ips = @account.user.ips.order(used_at: :desc).to_a
+
+          - recent_ips.each_with_index do |recent_ip, i|
             %tr
               - if i.zero?
-                %th{ rowspan: @account.user.recent_ips.size }= t('admin.accounts.most_recent_ip')
-              %td= ip
-              %td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: ip)
+                %th{ rowspan: recent_ips.size }= t('admin.accounts.most_recent_ip')
+              %td= recent_ip.ip
+              %td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: recent_ip.ip)
 
           %tr
             %th= t('admin.accounts.most_recent_activity')
diff --git a/app/views/admin_mailer/new_pending_account.text.erb b/app/views/admin_mailer/new_pending_account.text.erb
index bcc251819..a8a2a35fa 100644
--- a/app/views/admin_mailer/new_pending_account.text.erb
+++ b/app/views/admin_mailer/new_pending_account.text.erb
@@ -3,7 +3,7 @@
 <%= raw t('admin_mailer.new_pending_account.body') %>
 
 <%= @account.user_email %> (@<%= @account.username %>)
-<%= @account.user_current_sign_in_ip %>
+<%= @account.user_sign_up_ip %>
 <% if @account.user&.invite_request&.text.present? %>
 
 <%= quote_wrap(@account.user&.invite_request&.text) %>
diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb
index 918c10ac9..adc99c605 100644
--- a/app/workers/scheduler/ip_cleanup_scheduler.rb
+++ b/app/workers/scheduler/ip_cleanup_scheduler.rb
@@ -16,7 +16,7 @@ class Scheduler::IpCleanupScheduler
 
   def clean_ip_columns!
     SessionActivation.where('updated_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all
-    User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(last_sign_in_ip: nil, current_sign_in_ip: nil, sign_up_ip: nil)
+    User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(sign_up_ip: nil)
     LoginActivity.where('created_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all
   end
 
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index 5232e6cfd..b434c68fa 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -1,11 +1,8 @@
 require 'devise/strategies/authenticatable'
 
 Warden::Manager.after_set_user except: :fetch do |user, warden|
-  if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
-    session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
-  else
-    session_id = user.activate_session(warden.request)
-  end
+  session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
+  session_id = user.activate_session(warden.request) unless user.session_activations.active?(session_id)
 
   warden.cookies.signed['_session_id'] = {
     value: session_id,
@@ -17,9 +14,13 @@ Warden::Manager.after_set_user except: :fetch do |user, warden|
 end
 
 Warden::Manager.after_fetch do |user, warden|
-  if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
+  session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
+
+  if session_id && (session = user.session_activations.find_by(session_id: session_id))
+    session.update(ip: warden.request.remote_ip) if session.ip != warden.request.remote_ip
+
     warden.cookies.signed['_session_id'] = {
-      value: warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'],
+      value: session_id,
       expires: 1.year.from_now,
       httponly: true,
       secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'),
diff --git a/db/migrate/20210616214526_create_user_ips.rb b/db/migrate/20210616214526_create_user_ips.rb
new file mode 100644
index 000000000..68e81a9d8
--- /dev/null
+++ b/db/migrate/20210616214526_create_user_ips.rb
@@ -0,0 +1,5 @@
+class CreateUserIps < ActiveRecord::Migration[6.1]
+  def change
+    create_view :user_ips
+  end
+end
diff --git a/db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb b/db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb
new file mode 100644
index 000000000..b53b247f2
--- /dev/null
+++ b/db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class RemoveCurrentSignInIpFromUsers < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    safety_assured do
+      remove_column :users, :current_sign_in_ip, :inet
+      remove_column :users, :last_sign_in_ip, :inet
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a1d169b23..d1446c652 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -923,8 +923,6 @@ ActiveRecord::Schema.define(version: 2021_12_13_040746) do
     t.integer "sign_in_count", default: 0, null: false
     t.datetime "current_sign_in_at"
     t.datetime "last_sign_in_at"
-    t.inet "current_sign_in_ip"
-    t.inet "last_sign_in_ip"
     t.boolean "admin", default: false, null: false
     t.string "confirmation_token"
     t.datetime "confirmed_at"
@@ -1120,6 +1118,28 @@ ActiveRecord::Schema.define(version: 2021_12_13_040746) do
   SQL
   add_index "instances", ["domain"], name: "index_instances_on_domain", unique: true
 
+  create_view "user_ips", sql_definition: <<-SQL
+      SELECT t0.user_id,
+      t0.ip,
+      max(t0.used_at) AS used_at
+     FROM ( SELECT users.id AS user_id,
+              users.sign_up_ip AS ip,
+              users.created_at AS used_at
+             FROM users
+            WHERE (users.sign_up_ip IS NOT NULL)
+          UNION ALL
+           SELECT session_activations.user_id,
+              session_activations.ip,
+              session_activations.updated_at
+             FROM session_activations
+          UNION ALL
+           SELECT login_activities.user_id,
+              login_activities.ip,
+              login_activities.created_at
+             FROM login_activities
+            WHERE (login_activities.success = true)) t0
+    GROUP BY t0.user_id, t0.ip;
+  SQL
   create_view "account_summaries", materialized: true, sql_definition: <<-SQL
       SELECT accounts.id AS account_id,
       mode() WITHIN GROUP (ORDER BY t0.language) AS language,
diff --git a/db/views/user_ips_v01.sql b/db/views/user_ips_v01.sql
new file mode 100644
index 000000000..50a8201cd
--- /dev/null
+++ b/db/views/user_ips_v01.sql
@@ -0,0 +1,26 @@
+SELECT
+  user_id,
+  ip,
+  max(used_at) AS used_at
+FROM (
+  SELECT
+    id AS user_id,
+    sign_up_ip AS ip,
+    created_at AS used_at
+  FROM users
+  WHERE sign_up_ip IS NOT NULL
+  UNION ALL
+  SELECT
+    user_id,
+    ip,
+    updated_at
+  FROM session_activations
+  UNION ALL
+  SELECT
+    user_id,
+    ip,
+    created_at
+  FROM login_activities
+  WHERE success = 't'
+) AS t0
+GROUP BY user_id, ip
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index f718f5dd9..2368cc2bf 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -400,7 +400,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
     end
 
     context 'when 2FA is disabled and IP is unfamiliar' do
-      let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago, current_sign_in_ip: '0.0.0.0') }
+      let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago) }
 
       before do
         request.remote_ip  = '10.10.10.10'
-- 
cgit 


From cfa583fa7111cfc16b9ce548f9d9b58963f154bd Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Sun, 23 Jan 2022 15:50:41 +0100
Subject: Remove support for OAUTH_REDIRECT_AT_SIGN_IN (#17287)

Fixes #15959

Introduced in #6540, OAUTH_REDIRECT_AT_SIGN_IN allowed skipping the log-in form
to instead redirect to the external OmniAuth login provider.

However, it did not prevent the log-in form on /about introduced by #10232 from
appearing, and completely broke with the introduction of #15228.

As I restoring that previous log-in flow without introducing a security
vulnerability may require extensive care and knowledge of how OmniAuth works,
this commit removes support for OAUTH_REDIRECT_AT_SIGN_IN instead for the time
being.
---
 .env.nanobox                                |  4 ----
 app/controllers/auth/sessions_controller.rb | 16 ----------------
 config/initializers/omniauth.rb             |  1 -
 3 files changed, 21 deletions(-)

(limited to 'app/controllers/auth')

diff --git a/.env.nanobox b/.env.nanobox
index ad941c947..51dfdbd58 100644
--- a/.env.nanobox
+++ b/.env.nanobox
@@ -202,10 +202,6 @@ SMTP_FROM_ADDRESS=notifications@${APP_NAME}.nanoapp.io
 # Name of the pam service used for checking if an user can register (pam "account" section is evaluated) (nil (disabled) by default)
 # PAM_CONTROLLED_SERVICE=rpam
 
-# Global OAuth settings (optional) :
-# If you have only one strategy, you may want to enable this
-# OAUTH_REDIRECT_AT_SIGN_IN=true
-
 # Optional CAS authentication (cf. omniauth-cas) :
 # CAS_ENABLED=true
 # CAS_URL=https://sso.myserver.com/
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 3337a43c4..4d2695bf5 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -13,14 +13,6 @@ class Auth::SessionsController < Devise::SessionsController
   before_action :set_instance_presenter, only: [:new]
   before_action :set_body_classes
 
-  def new
-    Devise.omniauth_configs.each do |provider, config|
-      return redirect_to(omniauth_authorize_path(resource_name, provider)) if config.strategy.redirect_at_sign_in
-    end
-
-    super
-  end
-
   def create
     super do |resource|
       # We only need to call this if this hasn't already been
@@ -87,14 +79,6 @@ class Auth::SessionsController < Devise::SessionsController
     end
   end
 
-  def after_sign_out_path_for(_resource_or_scope)
-    Devise.omniauth_configs.each_value do |config|
-      return root_path if config.strategy.redirect_at_sign_in
-    end
-
-    super
-  end
-
   def require_no_authentication
     super
 
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index 5039b4c1f..19d59f155 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -5,7 +5,6 @@ end
 Devise.setup do |config|
   # Devise omniauth strategies
   options = {}
-  options[:redirect_at_sign_in] = ENV['OAUTH_REDIRECT_AT_SIGN_IN'] == 'true'
 
   # CAS strategy
   if ENV['CAS_ENABLED'] == 'true'
-- 
cgit 


From bddd9ba36d9f4e86e2a4bbea77f967c143afa2cc Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Sun, 23 Jan 2022 15:52:58 +0100
Subject: Add OMNIAUTH_ONLY environment variable to enforce externa log-in
 (#17288)

* Remove support for OAUTH_REDIRECT_AT_SIGN_IN

Fixes #15959

Introduced in #6540, OAUTH_REDIRECT_AT_SIGN_IN allowed skipping the log-in form
to instead redirect to the external OmniAuth login provider.

However, it did not prevent the log-in form on /about introduced by #10232 from
appearing, and completely broke with the introduction of #15228.

As I restoring that previous log-in flow without introducing a security
vulnerability may require extensive care and knowledge of how OmniAuth works,
this commit removes support for OAUTH_REDIRECT_AT_SIGN_IN instead for the time
being.

* Add OMNIAUTH_ONLY environment variable to enforce external log-in only

* Disable user registration when OMNIAUTH_ONLY is set to true

* Replace log-in links When OMNIAUTH_ONLY is set with exactly one OmniAuth provider
---
 app/controllers/api/v1/accounts_controller.rb    |  6 ++++-
 app/controllers/auth/registrations_controller.rb |  6 ++++-
 app/helpers/application_helper.rb                | 26 ++++++++++++++++++++-
 app/views/about/_login.html.haml                 | 29 ++++++++++++++++--------
 app/views/auth/sessions/new.html.haml            | 25 ++++++++++----------
 app/views/auth/shared/_links.html.haml           |  2 +-
 app/views/layouts/public.html.haml               |  2 +-
 app/views/statuses/_status.html.haml             |  2 +-
 config/locales/en.yml                            |  1 +
 9 files changed, 71 insertions(+), 28 deletions(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index cbccd64f3..5c47158e0 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -83,10 +83,14 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def check_enabled_registrations
-    forbidden if single_user_mode? || !allowed_registrations?
+    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations?
   end
 
   def allowed_registrations?
     Setting.registrations_mode != 'none'
   end
+
+  def omniauth_only?
+    ENV['OMNIAUTH_ONLY'] == 'true'
+  end
 end
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 3c1730f25..f37e906fd 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -81,13 +81,17 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   end
 
   def check_enabled_registrations
-    redirect_to root_path if single_user_mode? || !allowed_registrations?
+    redirect_to root_path if single_user_mode? || omniauth_only? || !allowed_registrations?
   end
 
   def allowed_registrations?
     Setting.registrations_mode != 'none' || @invite&.valid_for_use?
   end
 
+  def omniauth_only?
+    ENV['OMNIAUTH_ONLY'] == 'true'
+  end
+
   def invite_code
     if params[:user]
       params[:user][:invite_code]
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 34fc46615..9e16de5b5 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -50,13 +50,37 @@ module ApplicationHelper
   end
 
   def available_sign_up_path
-    if closed_registrations?
+    if closed_registrations? || omniauth_only?
       'https://joinmastodon.org/#getting-started'
     else
       new_user_registration_path
     end
   end
 
+  def omniauth_only?
+    ENV['OMNIAUTH_ONLY'] == 'true'
+  end
+
+  def link_to_login(name = nil, html_options = nil, &block)
+    target = new_user_session_path
+
+    if omniauth_only? && Devise.mappings[:user].omniauthable? && User.omniauth_providers.size == 1
+      target = omniauth_authorize_path(:user, User.omniauth_providers[0])
+      html_options ||= {}
+      html_options[:method] = :post
+    end
+
+    if block_given?
+      link_to(target, html_options, &block)
+    else
+      link_to(name, target, html_options)
+    end
+  end
+
+  def provider_sign_in_link(provider)
+    link_to I18n.t("auth.providers.#{provider}", default: provider.to_s.chomp('_oauth2').capitalize), omniauth_authorize_path(:user, provider), class: "button button-#{provider}", method: :post
+  end
+
   def open_deletion?
     Setting.open_deletion
   end
diff --git a/app/views/about/_login.html.haml b/app/views/about/_login.html.haml
index fa58f04d7..0f19e8164 100644
--- a/app/views/about/_login.html.haml
+++ b/app/views/about/_login.html.haml
@@ -1,13 +1,22 @@
-= simple_form_for(new_user, url: user_session_path, namespace: 'login') do |f|
-  .fields-group
-    - if use_seamless_external_login?
-      = f.input :email, placeholder: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false
-    - else
-      = f.input :email, placeholder: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false
+- unless omniauth_only?
+  = simple_form_for(new_user, url: user_session_path, namespace: 'login') do |f|
+    .fields-group
+      - if use_seamless_external_login?
+        = f.input :email, placeholder: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false
+      - else
+        = f.input :email, placeholder: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false
 
-    = f.input :password, placeholder: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password') }, hint: false
+      = f.input :password, placeholder: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password') }, hint: false
 
-  .actions
-    = f.button :button, t('auth.login'), type: :submit, class: 'button button-primary'
+    .actions
+      = f.button :button, t('auth.login'), type: :submit, class: 'button button-primary'
 
-  %p.hint.subtle-hint= link_to t('auth.trouble_logging_in'), new_user_password_path
+    %p.hint.subtle-hint= link_to t('auth.trouble_logging_in'), new_user_password_path
+
+- if Devise.mappings[:user].omniauthable? and User.omniauth_providers.any?
+  .simple_form.alternative-login
+    %h4= omniauth_only? ? t('auth.log_in_with') : t('auth.or_log_in_with')
+
+    .actions
+      - User.omniauth_providers.each do |provider|
+        = provider_sign_in_link(provider)
diff --git a/app/views/auth/sessions/new.html.haml b/app/views/auth/sessions/new.html.haml
index 9713bdaeb..a4323d1d9 100644
--- a/app/views/auth/sessions/new.html.haml
+++ b/app/views/auth/sessions/new.html.haml
@@ -4,24 +4,25 @@
 - content_for :header_tags do
   = render partial: 'shared/og'
 
-= simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f|
-  .fields-group
-    - if use_seamless_external_login?
-      = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false
-    - else
-      = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false
-  .fields-group
-    = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password'), :autocomplete => 'off' }, hint: false
+- unless omniauth_only?
+  = simple_form_for(resource, as: resource_name, url: session_path(resource_name)) do |f|
+    .fields-group
+      - if use_seamless_external_login?
+        = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.username_or_email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.username_or_email') }, hint: false
+      - else
+        = f.input :email, autofocus: true, wrapper: :with_label, label: t('simple_form.labels.defaults.email'), input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, hint: false
+    .fields-group
+      = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.password'), :autocomplete => 'off' }, hint: false
 
-  .actions
-    = f.button :button, t('auth.login'), type: :submit
+    .actions
+      = f.button :button, t('auth.login'), type: :submit
 
 - if devise_mapping.omniauthable? and resource_class.omniauth_providers.any?
   .simple_form.alternative-login
-    %h4= t('auth.or_log_in_with')
+    %h4= omniauth_only? ? t('auth.log_in_with') : t('auth.or_log_in_with')
 
     .actions
       - resource_class.omniauth_providers.each do |provider|
-        = link_to t("auth.providers.#{provider}", default: provider.to_s.chomp("_oauth2").capitalize), omniauth_authorize_path(resource_name, provider), class: "button button-#{provider}", method: :post
+        = provider_sign_in_link(provider)
 
 .form-footer= render 'auth/shared/links'
diff --git a/app/views/auth/shared/_links.html.haml b/app/views/auth/shared/_links.html.haml
index 66ed5b93f..f078e2f7e 100644
--- a/app/views/auth/shared/_links.html.haml
+++ b/app/views/auth/shared/_links.html.haml
@@ -3,7 +3,7 @@
     %li= link_to t('settings.account_settings'), edit_user_registration_path
   - else
     - if controller_name != 'sessions'
-      %li= link_to t('auth.login'), new_user_session_path
+      %li= link_to_login t('auth.login')
 
     - if controller_name != 'registrations'
       %li= link_to t('auth.register'), available_sign_up_path
diff --git a/app/views/layouts/public.html.haml b/app/views/layouts/public.html.haml
index bdb8a3a8e..069931cfd 100644
--- a/app/views/layouts/public.html.haml
+++ b/app/views/layouts/public.html.haml
@@ -22,7 +22,7 @@
             - if user_signed_in?
               = link_to t('settings.back'), root_url, class: 'nav-link nav-button webapp-btn'
             - else
-              = link_to t('auth.login'), new_user_session_path, class: 'webapp-btn nav-link nav-button'
+              = link_to_login t('auth.login'), class: 'webapp-btn nav-link nav-button'
               = link_to t('auth.register'), available_sign_up_path, class: 'webapp-btn nav-link nav-button'
 
     .container= yield
diff --git a/app/views/statuses/_status.html.haml b/app/views/statuses/_status.html.haml
index 9f3197d0d..3b7152753 100644
--- a/app/views/statuses/_status.html.haml
+++ b/app/views/statuses/_status.html.haml
@@ -56,6 +56,6 @@
 
 - if include_threads && !embedded_view? && !user_signed_in?
   .entry{ class: entry_classes }
-    = link_to new_user_session_path, class: 'load-more load-gap' do
+    = link_to_login class: 'load-more load-gap' do
       = fa_icon 'comments'
       = t('statuses.sign_in_to_participate')
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 13a252a47..85aa87c7a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -844,6 +844,7 @@ en:
     invalid_reset_password_token: Password reset token is invalid or expired. Please request a new one.
     link_to_otp: Enter a two-factor code from your phone or a recovery code
     link_to_webauth: Use your security key device
+    log_in_with: Log in with
     login: Log in
     logout: Logout
     migrate_account: Move to a different account
-- 
cgit 


From 1b493c9fee954b5bd4c4b00f9f945a5d97e2d699 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 24 Jan 2022 19:06:19 +0100
Subject: Add optional hCaptcha support

Fixes #1649

This requires setting `HCAPTCHA_SECRET_KEY` and `HCAPTCHA_SITE_KEY`, then
enabling the admin setting at
`/admin/settings/edit#form_admin_settings_captcha_enabled`

Subsequently, a hCaptcha widget will be displayed on `/about` and
`/auth/sign_up` unless:
- the user is already signed-up already
- the user has used an invite link
- the user has already solved the captcha (and registration failed for another
  reason)

The Content-Security-Policy headers are altered automatically to allow the
third-party hCaptcha scripts on `/about` and `/auth/sign_up` following the same
rules as above.
---
 .env.production.sample                           |  4 ++
 Gemfile                                          |  2 +
 Gemfile.lock                                     |  3 ++
 app/controllers/about_controller.rb              |  2 +
 app/controllers/api/v1/accounts_controller.rb    |  4 +-
 app/controllers/auth/registrations_controller.rb | 17 ++++++
 app/controllers/concerns/captcha_concern.rb      | 66 ++++++++++++++++++++++++
 app/helpers/admin/settings_helper.rb             |  4 ++
 app/javascript/flavours/glitch/styles/forms.scss |  4 ++
 app/models/form/admin_settings.rb                |  2 +
 app/views/about/_registration.html.haml          |  3 ++
 app/views/admin/settings/edit.html.haml          |  5 +-
 app/views/auth/registrations/new.html.haml       |  3 ++
 config/locales-glitch/en.yml                     |  3 ++
 config/settings.yml                              |  1 +
 15 files changed, 121 insertions(+), 2 deletions(-)
 create mode 100644 app/controllers/concerns/captcha_concern.rb

(limited to 'app/controllers/auth')

diff --git a/.env.production.sample b/.env.production.sample
index 13e89b40d..7de5e00f4 100644
--- a/.env.production.sample
+++ b/.env.production.sample
@@ -285,3 +285,7 @@ MAX_POLL_OPTION_CHARS=100
 # Units are in bytes
 MAX_EMOJI_SIZE=51200
 MAX_REMOTE_EMOJI_SIZE=204800
+
+# Optional hCaptcha support
+# HCAPTCHA_SECRET_KEY=
+# HCAPTCHA_SITE_KEY=
diff --git a/Gemfile b/Gemfile
index eae5f11b7..282ab65e4 100644
--- a/Gemfile
+++ b/Gemfile
@@ -156,3 +156,5 @@ gem 'concurrent-ruby', require: false
 gem 'connection_pool', require: false
 
 gem 'xorcist', '~> 1.1'
+
+gem "hcaptcha", "~> 7.1"
diff --git a/Gemfile.lock b/Gemfile.lock
index 8d72732eb..cc9a53e41 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -271,6 +271,8 @@ GEM
       railties (>= 4.0.1)
     hashdiff (1.0.1)
     hashie (4.1.0)
+    hcaptcha (7.1.0)
+      json
     highline (2.0.3)
     hiredis (0.6.3)
     hkdf (0.3.0)
@@ -719,6 +721,7 @@ DEPENDENCIES
   fog-openstack (~> 0.3)
   fuubar (~> 2.5)
   hamlit-rails (~> 0.2)
+  hcaptcha (~> 7.1)
   hiredis (~> 0.6)
   htmlentities (~> 4.3)
   http (~> 5.0)
diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 620c0ff78..5a35dbbcb 100644
--- a/app/controllers/about_controller.rb
+++ b/app/controllers/about_controller.rb
@@ -2,6 +2,7 @@
 
 class AboutController < ApplicationController
   include RegistrationSpamConcern
+  include CaptchaConcern
 
   before_action :set_pack
 
@@ -12,6 +13,7 @@ class AboutController < ApplicationController
   before_action :set_instance_presenter
   before_action :set_expires_in, only: [:more, :terms]
   before_action :set_registration_form_time, only: :show
+  before_action :extend_csp_for_captcha!, only: :show
 
   skip_before_action :require_functional!, only: [:more, :terms]
 
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index 5c47158e0..8916c3f96 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Api::V1::AccountsController < Api::BaseController
+  include CaptchaConcern
+
   before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:create, :follow, :unfollow, :remove_from_followers, :block, :unblock, :mute, :unmute]
   before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, only: [:follow, :unfollow, :remove_from_followers]
   before_action -> { doorkeeper_authorize! :follow, :'write:mutes' }, only: [:mute, :unmute]
@@ -83,7 +85,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def check_enabled_registrations
-    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations?
+    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations? || captcha_enabled?
   end
 
   def allowed_registrations?
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 6b1f3fa82..3c9b38a4b 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -2,6 +2,7 @@
 
 class Auth::RegistrationsController < Devise::RegistrationsController
   include RegistrationSpamConcern
+  include CaptchaConcern
 
   layout :determine_layout
 
@@ -15,6 +16,8 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   before_action :require_not_suspended!, only: [:update]
   before_action :set_cache_headers, only: [:edit, :update]
   before_action :set_registration_form_time, only: :new
+  before_action :extend_csp_for_captcha!, only: [:new, :create]
+  before_action :check_captcha!, only: :create
 
   skip_before_action :require_functional!, only: [:edit, :update]
 
@@ -135,4 +138,18 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   def set_cache_headers
     response.headers['Cache-Control'] = 'no-cache, no-store, max-age=0, must-revalidate'
   end
+
+  def sign_up(resource_name, resource)
+    clear_captcha!
+    super
+  end
+
+  def check_captcha!
+    super do |error|
+      build_resource(sign_up_params)
+      resource.validate
+      resource.errors.add(:base, error)
+      respond_with resource
+    end
+  end
 end
diff --git a/app/controllers/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb
new file mode 100644
index 000000000..5a23e59e3
--- /dev/null
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+module CaptchaConcern
+  extend ActiveSupport::Concern
+  include Hcaptcha::Adapters::ViewMethods
+
+  CAPTCHA_TIMEOUT = 2.hours.freeze
+
+  included do
+    helper_method :render_captcha_if_needed
+  end
+
+  def captcha_available?
+    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present?
+  end
+
+  def captcha_enabled?
+    captcha_available? && Setting.captcha_enabled
+  end
+
+  def captcha_recently_passed?
+    session[:captcha_passed_at].present? && session[:captcha_passed_at] >= CAPTCHA_TIMEOUT.ago
+  end
+
+  def captcha_required?
+    captcha_enabled? && !current_user && !(@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?) && !captcha_recently_passed?
+  end
+
+  def clear_captcha!
+    session.delete(:captcha_passed_at)
+  end
+
+  def check_captcha!
+    return true unless captcha_required?
+
+    if verify_hcaptcha
+      session[:captcha_passed_at] = Time.now.utc
+      return true
+    else
+      if block_given?
+        message = flash[:hcaptcha_error]
+        flash.delete(:hcaptcha_error)
+        yield message
+      end
+      return false
+    end
+  end
+
+  def extend_csp_for_captcha!
+    policy = request.content_security_policy
+    return unless captcha_required? && policy.present?
+
+    %w(script_src frame_src style_src connect_src).each do |directive|
+      values = policy.send(directive)
+      values << 'https://hcaptcha.com' unless values.include?('https://hcaptcha.com') || values.include?('https:')
+      values << 'https://*.hcaptcha.com' unless values.include?('https://*.hcaptcha.com') || values.include?('https:')
+      policy.send(directive, *values)
+    end
+  end
+
+  def render_captcha_if_needed
+    return unless captcha_required?
+
+    hcaptcha_tags
+  end
+end
diff --git a/app/helpers/admin/settings_helper.rb b/app/helpers/admin/settings_helper.rb
index baf14ab25..f99a2b8c8 100644
--- a/app/helpers/admin/settings_helper.rb
+++ b/app/helpers/admin/settings_helper.rb
@@ -8,4 +8,8 @@ module Admin::SettingsHelper
     link = link_to t('admin.site_uploads.delete'), admin_site_upload_path(upload), data: { method: :delete }
     safe_join([hint, link], '<br/>'.html_safe)
   end
+
+  def captcha_available?
+    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present?
+  end
 end
diff --git a/app/javascript/flavours/glitch/styles/forms.scss b/app/javascript/flavours/glitch/styles/forms.scss
index 3433abcdd..64d441fb2 100644
--- a/app/javascript/flavours/glitch/styles/forms.scss
+++ b/app/javascript/flavours/glitch/styles/forms.scss
@@ -1058,3 +1058,7 @@ code {
     display: none;
   }
 }
+
+.simple_form .h-captcha {
+  text-align: center;
+}
diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb
index 3202d1fc2..34f14e312 100644
--- a/app/models/form/admin_settings.rb
+++ b/app/models/form/admin_settings.rb
@@ -40,6 +40,7 @@ class Form::AdminSettings
     noindex
     outgoing_spoilers
     require_invite_text
+    captcha_enabled
   ).freeze
 
   BOOLEAN_KEYS = %i(
@@ -58,6 +59,7 @@ class Form::AdminSettings
     trendable_by_default
     noindex
     require_invite_text
+    captcha_enabled
   ).freeze
 
   UPLOAD_KEYS = %i(
diff --git a/app/views/about/_registration.html.haml b/app/views/about/_registration.html.haml
index e4d614d71..5bb5d08a2 100644
--- a/app/views/about/_registration.html.haml
+++ b/app/views/about/_registration.html.haml
@@ -21,6 +21,9 @@
     .fields-group
       = f.input :agreement, as: :boolean, wrapper: :with_label, label: t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true, disabled: closed_registrations?
 
+    .fields-group
+      = render_captcha_if_needed
+
     .actions
       = f.button :button, sign_up_message, type: :submit, class: 'button button-primary', disabled: closed_registrations?
 
diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml
index b9daae8f0..49b03a9e3 100644
--- a/app/views/admin/settings/edit.html.haml
+++ b/app/views/admin/settings/edit.html.haml
@@ -42,7 +42,10 @@
 
   .fields-group
     = f.input :require_invite_text, as: :boolean, wrapper: :with_label, label: t('admin.settings.registrations.require_invite_text.title'), hint: t('admin.settings.registrations.require_invite_text.desc_html'), disabled: !approved_registrations?
-  .fields-group
+
+  - if captcha_available?
+    .fields-group
+      = f.input :captcha_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.captcha_enabled.title'), hint: t('admin.settings.captcha_enabled.desc_html')
 
   %hr.spacer/
 
diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml
index 6981195ed..5cb558297 100644
--- a/app/views/auth/registrations/new.html.haml
+++ b/app/views/auth/registrations/new.html.haml
@@ -38,6 +38,9 @@
   .fields-group
     = f.input :agreement, as: :boolean, wrapper: :with_label, label: whitelist_mode? ? t('auth.checkbox_agreement_without_rules_html', terms_path: terms_path) : t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true
 
+  .field-group
+    = render_captcha_if_needed
+
   .actions
     = f.button :button, @invite.present? ? t('auth.register') : sign_up_message, type: :submit
 
diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml
index 5cc2625fc..c96f21c92 100644
--- a/config/locales-glitch/en.yml
+++ b/config/locales-glitch/en.yml
@@ -2,6 +2,9 @@
 en:
   admin:
     settings:
+      captcha_enabled:
+        desc_html: Enable hCaptcha integration, requiring new users to solve a challenge when signing up. Note that this disables app-based registration, and requires third-party scripts from hCaptcha to be embedded in the registration pages. This may have security and privacy concerns.
+        title: Require new users to go through a CAPTCHA to sign up
       enable_keybase:
         desc_html: Allow your users to prove their identity via keybase
         title: Enable keybase integration
diff --git a/config/settings.yml b/config/settings.yml
index 094209822..7d192f369 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -77,6 +77,7 @@ defaults: &defaults
   show_domain_blocks_rationale: 'disabled'
   outgoing_spoilers: ''
   require_invite_text: false
+  captcha_enabled: false
 
 development:
   <<: *defaults
-- 
cgit 


From 6a2f248fe4ed59f512dd318a006209fb7b71aa7e Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 24 Jan 2022 21:52:45 +0100
Subject: Renew Rails session ID on successful registration

---
 app/controllers/auth/registrations_controller.rb | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 3c9b38a4b..0db9cb84d 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -141,6 +141,11 @@ class Auth::RegistrationsController < Devise::RegistrationsController
 
   def sign_up(resource_name, resource)
     clear_captcha!
+
+    old_session_values = session.to_hash
+    reset_session
+    session.update old_session_values.except('session_id')
+
     super
   end
 
-- 
cgit 


From 0fb907441c827cadc767641b29d5d2c0e554f7a4 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Tue, 25 Jan 2022 22:37:12 +0100
Subject: Add ability to set hCaptcha either on registration form or on e-mail
 validation

Upshot of CAPTCHA on e-mail validation is it does not need to break the in-band
registration API.
---
 app/controllers/auth/confirmations_controller.rb | 50 ++++++++++++++++++++++++
 app/controllers/concerns/captcha_concern.rb      | 12 +++++-
 app/models/form/admin_settings.rb                |  4 +-
 app/serializers/rest/instance_serializer.rb      |  2 +-
 app/views/admin/settings/edit.html.haml          |  2 +-
 app/views/auth/confirmations/captcha.html.haml   | 11 ++++++
 config/locales-glitch/en.yml                     | 17 ++++++--
 config/routes.rb                                 |  1 +
 config/settings.yml                              |  2 +-
 9 files changed, 91 insertions(+), 10 deletions(-)
 create mode 100644 app/views/auth/confirmations/captcha.html.haml

(limited to 'app/controllers/auth')

diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb
index 0b5a2f3c9..e9a646f91 100644
--- a/app/controllers/auth/confirmations_controller.rb
+++ b/app/controllers/auth/confirmations_controller.rb
@@ -1,12 +1,18 @@
 # frozen_string_literal: true
 
 class Auth::ConfirmationsController < Devise::ConfirmationsController
+  include CaptchaConcern
+
   layout 'auth'
 
   before_action :set_body_classes
   before_action :set_pack
+  before_action :set_confirmation_user!, only: [:show, :confirm_captcha]
   before_action :require_unconfirmed!
 
+  before_action :extend_csp_for_captcha!, only: [:show, :confirm_captcha]
+  before_action :require_captcha_if_needed!, only: [:show]
+
   skip_before_action :require_functional!
 
   def new
@@ -15,8 +21,52 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
     resource.email = current_user.unconfirmed_email || current_user.email if user_signed_in?
   end
 
+  def show
+    clear_captcha!
+
+    old_session_values = session.to_hash
+    reset_session
+    session.update old_session_values.except('session_id')
+
+    super
+  end
+
+  def confirm_captcha
+    check_captcha! do |message|
+      flash.now[:alert] = message
+      render :captcha
+      return
+    end
+
+    show
+  end
+
   private
 
+  def require_captcha_if_needed!
+    render :captcha if captcha_required?
+  end
+
+  def set_confirmation_user!
+    # We need to reimplement looking up the user because
+    # Devise::ConfirmationsController#show looks up and confirms in one
+    # step.
+    confirmation_token = params[:confirmation_token]
+    return if confirmation_token.nil?
+    @confirmation_user = User.find_first_by_auth_conditions(confirmation_token: confirmation_token)
+  end
+
+  def captcha_user_bypass?
+    return true if @confirmation_user.nil? || @confirmation_user.confirmed?
+
+    invite = Invite.find(@confirmation_user.invite_id) if @confirmation_user.invite_id.present?
+    invite.present? && !invite.max_uses.nil?
+  end
+
+  def captcha_context
+    'email-confirmation'
+  end
+
   def set_pack
     use_pack 'auth'
   end
diff --git a/app/controllers/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb
index 4a942c988..02069d205 100644
--- a/app/controllers/concerns/captcha_concern.rb
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -15,17 +15,21 @@ module CaptchaConcern
   end
 
   def captcha_enabled?
-    captcha_available? && Setting.captcha_enabled
+    captcha_available? && Setting.captcha_mode == captcha_context
   end
 
   def captcha_recently_passed?
     session[:captcha_passed_at].present? && session[:captcha_passed_at] >= CAPTCHA_TIMEOUT.ago
   end
 
+  def captcha_user_bypass?
+    current_user.present? || (@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?)
+  end
+
   def captcha_required?
     return false if ENV['OMNIAUTH_ONLY'] == 'true'
     return false unless Setting.registrations_mode != 'none' || @invite&.valid_for_use?
-    captcha_enabled? && !current_user && !(@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?) && !captcha_recently_passed?
+    captcha_enabled? && !captcha_user_bypass? && !captcha_recently_passed?
   end
 
   def clear_captcha!
@@ -65,4 +69,8 @@ module CaptchaConcern
 
     hcaptcha_tags
   end
+
+  def captcha_context
+    'registration-form'
+  end
 end
diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb
index 34f14e312..7abb0d6c6 100644
--- a/app/models/form/admin_settings.rb
+++ b/app/models/form/admin_settings.rb
@@ -40,7 +40,7 @@ class Form::AdminSettings
     noindex
     outgoing_spoilers
     require_invite_text
-    captcha_enabled
+    captcha_mode
   ).freeze
 
   BOOLEAN_KEYS = %i(
@@ -59,7 +59,6 @@ class Form::AdminSettings
     trendable_by_default
     noindex
     require_invite_text
-    captcha_enabled
   ).freeze
 
   UPLOAD_KEYS = %i(
@@ -83,6 +82,7 @@ class Form::AdminSettings
   validates :bootstrap_timeline_accounts, existing_username: { multiple: true }
   validates :show_domain_blocks, inclusion: { in: %w(disabled users all) }
   validates :show_domain_blocks_rationale, inclusion: { in: %w(disabled users all) }
+  validates :captcha_mode, inclusion: { in: %w(disabled registration-form email-confirmation) }
 
   def initialize(_attributes = {})
     super
diff --git a/app/serializers/rest/instance_serializer.rb b/app/serializers/rest/instance_serializer.rb
index 94cc3ffe3..d343cca20 100644
--- a/app/serializers/rest/instance_serializer.rb
+++ b/app/serializers/rest/instance_serializer.rb
@@ -116,6 +116,6 @@ class REST::InstanceSerializer < ActiveModel::Serializer
   end
 
   def captcha_enabled?
-    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present? && Setting.captcha_enabled
+    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present? && Setting.captcha_mode == 'registration-form'
   end
 end
diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml
index 49b03a9e3..fc042f845 100644
--- a/app/views/admin/settings/edit.html.haml
+++ b/app/views/admin/settings/edit.html.haml
@@ -45,7 +45,7 @@
 
   - if captcha_available?
     .fields-group
-      = f.input :captcha_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.captcha_enabled.title'), hint: t('admin.settings.captcha_enabled.desc_html')
+      = f.input :captcha_mode, as: :radio_buttons, collection: %w(disabled registration-form email-confirmation), include_blank: false, wrapper: :with_block_label, label_method: ->(type) { safe_join([t("admin.settings.captcha.#{type}.title"), content_tag(:span, t("admin.settings.captcha.#{type}.desc_html"), class: 'hint')])}, label: t('admin.settings.captcha.title'), hint: t('admin.settings.captcha.desc_html')
 
   %hr.spacer/
 
diff --git a/app/views/auth/confirmations/captcha.html.haml b/app/views/auth/confirmations/captcha.html.haml
new file mode 100644
index 000000000..850bc1479
--- /dev/null
+++ b/app/views/auth/confirmations/captcha.html.haml
@@ -0,0 +1,11 @@
+- content_for :page_title do
+  = t('auth.confirm_captcha')
+
+= form_tag auth_captcha_confirmation_url, method: 'POST', class: 'simple_form' do
+  = hidden_field_tag :confirmation_token, params[:confirmation_token]
+
+  .field-group
+    = render_captcha_if_needed
+
+  .actions
+    %button.button= t('challenge.continue')
diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml
index 9bd66c969..6ad5a5365 100644
--- a/config/locales-glitch/en.yml
+++ b/config/locales-glitch/en.yml
@@ -2,9 +2,18 @@
 en:
   admin:
     settings:
-      captcha_enabled:
-        desc_html: Enable hCaptcha integration, requiring new users to solve a challenge when signing up. Note that this disables app-based registration, may prevent your instance from being listed as having open registrations, and requires third-party scripts from hCaptcha to be embedded in the registration pages. This may have security and privacy concerns.
-        title: Require new users to go through a CAPTCHA to sign up
+      captcha:
+        desc_html: Configure hCaptcha integration, relying on third-party scripts. This may have security and privacy implications.
+        email-confirmation:
+          desc_html: Require new users to go through hCaptcha at the e-mail validation step. Bots will not be deterred from creating accounts, but they may be prevented from confirming them, leaving them to be automatically cleaned up after a couple days. This does not interfere with app-based registrations.
+          title: CAPTCHA on email validation
+        disabled:
+          desc_html: Do not require a CAPTCHA
+          title: Disabled
+        registration-form:
+          desc_html: Require new users to go through hCaptcha on the registration form, so that CAPTCHA requirement is immediately apparent to them. This disables app-based registrations and may prevent your instance from being listed as having open registrations.
+          title: CAPTCHA on registration forms
+        title: CAPTCHA configuration
       enable_keybase:
         desc_html: Allow your users to prove their identity via keybase
         title: Enable keybase integration
@@ -20,6 +29,8 @@ en:
       show_replies_in_public_timelines:
         desc_html: In addition to public self-replies (threads), show public replies in local and public timelines.
         title: Show replies in public timelines
+  auth:
+    confirm_captcha: User verification
   generic:
     use_this: Use this
   settings:
diff --git a/config/routes.rb b/config/routes.rb
index 65dd7ad63..d0eeda1e8 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -44,6 +44,7 @@ Rails.application.routes.draw do
       resource :setup, only: [:show, :update], controller: :setup
       resource :challenge, only: [:create], controller: :challenges
       get 'sessions/security_key_options', to: 'sessions#webauthn_options'
+      post 'captcha_confirmation', to: 'confirmations#confirm_captcha', as: :captcha_confirmation
     end
   end
 
diff --git a/config/settings.yml b/config/settings.yml
index 7d192f369..b5437caee 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -77,7 +77,7 @@ defaults: &defaults
   show_domain_blocks_rationale: 'disabled'
   outgoing_spoilers: ''
   require_invite_text: false
-  captcha_enabled: false
+  captcha_mode: 'disabled'
 
 development:
   <<: *defaults
-- 
cgit 


From b7cf3941b3783220e6b3bc9a6d3975ceecdc64cb Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Tue, 25 Jan 2022 23:56:57 +0100
Subject: Change CAPTCHA handling to be only on email verification

This simplifies the implementation considerably, and while not providing
ideal UX, it's the most flexible approach.
---
 app/controllers/about_controller.rb              |  2 --
 app/controllers/api/v1/accounts_controller.rb    |  4 +---
 app/controllers/auth/confirmations_controller.rb |  6 ------
 app/controllers/auth/registrations_controller.rb | 22 -------------------
 app/controllers/concerns/captcha_concern.rb      | 27 +++++-------------------
 app/models/form/admin_settings.rb                |  4 ++--
 app/serializers/rest/instance_serializer.rb      |  6 +-----
 app/views/about/_registration.html.haml          |  3 ---
 app/views/admin/settings/edit.html.haml          |  2 +-
 app/views/auth/confirmations/captcha.html.haml   |  2 +-
 app/views/auth/registrations/new.html.haml       |  3 ---
 config/locales-glitch/en.yml                     | 15 +++----------
 config/settings.yml                              |  2 +-
 spec/views/about/show.html.haml_spec.rb          |  1 -
 14 files changed, 15 insertions(+), 84 deletions(-)

(limited to 'app/controllers/auth')

diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 5a35dbbcb..620c0ff78 100644
--- a/app/controllers/about_controller.rb
+++ b/app/controllers/about_controller.rb
@@ -2,7 +2,6 @@
 
 class AboutController < ApplicationController
   include RegistrationSpamConcern
-  include CaptchaConcern
 
   before_action :set_pack
 
@@ -13,7 +12,6 @@ class AboutController < ApplicationController
   before_action :set_instance_presenter
   before_action :set_expires_in, only: [:more, :terms]
   before_action :set_registration_form_time, only: :show
-  before_action :extend_csp_for_captcha!, only: :show
 
   skip_before_action :require_functional!, only: [:more, :terms]
 
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index 8916c3f96..5c47158e0 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class Api::V1::AccountsController < Api::BaseController
-  include CaptchaConcern
-
   before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:create, :follow, :unfollow, :remove_from_followers, :block, :unblock, :mute, :unmute]
   before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, only: [:follow, :unfollow, :remove_from_followers]
   before_action -> { doorkeeper_authorize! :follow, :'write:mutes' }, only: [:mute, :unmute]
@@ -85,7 +83,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def check_enabled_registrations
-    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations? || captcha_enabled?
+    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations?
   end
 
   def allowed_registrations?
diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb
index e9a646f91..17ad56fa8 100644
--- a/app/controllers/auth/confirmations_controller.rb
+++ b/app/controllers/auth/confirmations_controller.rb
@@ -22,8 +22,6 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
   end
 
   def show
-    clear_captcha!
-
     old_session_values = session.to_hash
     reset_session
     session.update old_session_values.except('session_id')
@@ -63,10 +61,6 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
     invite.present? && !invite.max_uses.nil?
   end
 
-  def captcha_context
-    'email-confirmation'
-  end
-
   def set_pack
     use_pack 'auth'
   end
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 0db9cb84d..6b1f3fa82 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -2,7 +2,6 @@
 
 class Auth::RegistrationsController < Devise::RegistrationsController
   include RegistrationSpamConcern
-  include CaptchaConcern
 
   layout :determine_layout
 
@@ -16,8 +15,6 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   before_action :require_not_suspended!, only: [:update]
   before_action :set_cache_headers, only: [:edit, :update]
   before_action :set_registration_form_time, only: :new
-  before_action :extend_csp_for_captcha!, only: [:new, :create]
-  before_action :check_captcha!, only: :create
 
   skip_before_action :require_functional!, only: [:edit, :update]
 
@@ -138,23 +135,4 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   def set_cache_headers
     response.headers['Cache-Control'] = 'no-cache, no-store, max-age=0, must-revalidate'
   end
-
-  def sign_up(resource_name, resource)
-    clear_captcha!
-
-    old_session_values = session.to_hash
-    reset_session
-    session.update old_session_values.except('session_id')
-
-    super
-  end
-
-  def check_captcha!
-    super do |error|
-      build_resource(sign_up_params)
-      resource.validate
-      resource.errors.add(:base, error)
-      respond_with resource
-    end
-  end
 end
diff --git a/app/controllers/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb
index 02069d205..538c1ffb1 100644
--- a/app/controllers/concerns/captcha_concern.rb
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -4,10 +4,8 @@ module CaptchaConcern
   extend ActiveSupport::Concern
   include Hcaptcha::Adapters::ViewMethods
 
-  CAPTCHA_TIMEOUT = 2.hours.freeze
-
   included do
-    helper_method :render_captcha_if_needed
+    helper_method :render_captcha
   end
 
   def captcha_available?
@@ -15,32 +13,21 @@ module CaptchaConcern
   end
 
   def captcha_enabled?
-    captcha_available? && Setting.captcha_mode == captcha_context
-  end
-
-  def captcha_recently_passed?
-    session[:captcha_passed_at].present? && session[:captcha_passed_at] >= CAPTCHA_TIMEOUT.ago
+    captcha_available? && Setting.captcha_enabled
   end
 
   def captcha_user_bypass?
-    current_user.present? || (@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?)
+    false
   end
 
   def captcha_required?
-    return false if ENV['OMNIAUTH_ONLY'] == 'true'
-    return false unless Setting.registrations_mode != 'none' || @invite&.valid_for_use?
-    captcha_enabled? && !captcha_user_bypass? && !captcha_recently_passed?
-  end
-
-  def clear_captcha!
-    session.delete(:captcha_passed_at)
+    captcha_enabled? && !captcha_user_bypass?
   end
 
   def check_captcha!
     return true unless captcha_required?
 
     if verify_hcaptcha
-      session[:captcha_passed_at] = Time.now.utc
       true
     else
       if block_given?
@@ -64,13 +51,9 @@ module CaptchaConcern
     end
   end
 
-  def render_captcha_if_needed
+  def render_captcha
     return unless captcha_required?
 
     hcaptcha_tags
   end
-
-  def captcha_context
-    'registration-form'
-  end
 end
diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb
index 7abb0d6c6..34f14e312 100644
--- a/app/models/form/admin_settings.rb
+++ b/app/models/form/admin_settings.rb
@@ -40,7 +40,7 @@ class Form::AdminSettings
     noindex
     outgoing_spoilers
     require_invite_text
-    captcha_mode
+    captcha_enabled
   ).freeze
 
   BOOLEAN_KEYS = %i(
@@ -59,6 +59,7 @@ class Form::AdminSettings
     trendable_by_default
     noindex
     require_invite_text
+    captcha_enabled
   ).freeze
 
   UPLOAD_KEYS = %i(
@@ -82,7 +83,6 @@ class Form::AdminSettings
   validates :bootstrap_timeline_accounts, existing_username: { multiple: true }
   validates :show_domain_blocks, inclusion: { in: %w(disabled users all) }
   validates :show_domain_blocks_rationale, inclusion: { in: %w(disabled users all) }
-  validates :captcha_mode, inclusion: { in: %w(disabled registration-form email-confirmation) }
 
   def initialize(_attributes = {})
     super
diff --git a/app/serializers/rest/instance_serializer.rb b/app/serializers/rest/instance_serializer.rb
index d343cca20..48bbb55c8 100644
--- a/app/serializers/rest/instance_serializer.rb
+++ b/app/serializers/rest/instance_serializer.rb
@@ -98,7 +98,7 @@ class REST::InstanceSerializer < ActiveModel::Serializer
   end
 
   def registrations
-    Setting.registrations_mode != 'none' && !Rails.configuration.x.single_user_mode && !captcha_enabled?
+    Setting.registrations_mode != 'none' && !Rails.configuration.x.single_user_mode
   end
 
   def approval_required
@@ -114,8 +114,4 @@ class REST::InstanceSerializer < ActiveModel::Serializer
   def instance_presenter
     @instance_presenter ||= InstancePresenter.new
   end
-
-  def captcha_enabled?
-    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present? && Setting.captcha_mode == 'registration-form'
-  end
 end
diff --git a/app/views/about/_registration.html.haml b/app/views/about/_registration.html.haml
index 5bb5d08a2..e4d614d71 100644
--- a/app/views/about/_registration.html.haml
+++ b/app/views/about/_registration.html.haml
@@ -21,9 +21,6 @@
     .fields-group
       = f.input :agreement, as: :boolean, wrapper: :with_label, label: t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true, disabled: closed_registrations?
 
-    .fields-group
-      = render_captcha_if_needed
-
     .actions
       = f.button :button, sign_up_message, type: :submit, class: 'button button-primary', disabled: closed_registrations?
 
diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml
index fc042f845..49b03a9e3 100644
--- a/app/views/admin/settings/edit.html.haml
+++ b/app/views/admin/settings/edit.html.haml
@@ -45,7 +45,7 @@
 
   - if captcha_available?
     .fields-group
-      = f.input :captcha_mode, as: :radio_buttons, collection: %w(disabled registration-form email-confirmation), include_blank: false, wrapper: :with_block_label, label_method: ->(type) { safe_join([t("admin.settings.captcha.#{type}.title"), content_tag(:span, t("admin.settings.captcha.#{type}.desc_html"), class: 'hint')])}, label: t('admin.settings.captcha.title'), hint: t('admin.settings.captcha.desc_html')
+      = f.input :captcha_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.captcha_enabled.title'), hint: t('admin.settings.captcha_enabled.desc_html')
 
   %hr.spacer/
 
diff --git a/app/views/auth/confirmations/captcha.html.haml b/app/views/auth/confirmations/captcha.html.haml
index 850bc1479..0f7cf9c59 100644
--- a/app/views/auth/confirmations/captcha.html.haml
+++ b/app/views/auth/confirmations/captcha.html.haml
@@ -5,7 +5,7 @@
   = hidden_field_tag :confirmation_token, params[:confirmation_token]
 
   .field-group
-    = render_captcha_if_needed
+    = render_captcha
 
   .actions
     %button.button= t('challenge.continue')
diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml
index 5cb558297..6981195ed 100644
--- a/app/views/auth/registrations/new.html.haml
+++ b/app/views/auth/registrations/new.html.haml
@@ -38,9 +38,6 @@
   .fields-group
     = f.input :agreement, as: :boolean, wrapper: :with_label, label: whitelist_mode? ? t('auth.checkbox_agreement_without_rules_html', terms_path: terms_path) : t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true
 
-  .field-group
-    = render_captcha_if_needed
-
   .actions
     = f.button :button, @invite.present? ? t('auth.register') : sign_up_message, type: :submit
 
diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml
index 6ad5a5365..ab7f1b976 100644
--- a/config/locales-glitch/en.yml
+++ b/config/locales-glitch/en.yml
@@ -2,18 +2,9 @@
 en:
   admin:
     settings:
-      captcha:
-        desc_html: Configure hCaptcha integration, relying on third-party scripts. This may have security and privacy implications.
-        email-confirmation:
-          desc_html: Require new users to go through hCaptcha at the e-mail validation step. Bots will not be deterred from creating accounts, but they may be prevented from confirming them, leaving them to be automatically cleaned up after a couple days. This does not interfere with app-based registrations.
-          title: CAPTCHA on email validation
-        disabled:
-          desc_html: Do not require a CAPTCHA
-          title: Disabled
-        registration-form:
-          desc_html: Require new users to go through hCaptcha on the registration form, so that CAPTCHA requirement is immediately apparent to them. This disables app-based registrations and may prevent your instance from being listed as having open registrations.
-          title: CAPTCHA on registration forms
-        title: CAPTCHA configuration
+      captcha_enabled:
+        desc_html: Enable hCaptcha integration, requiring new users to solve a challenge when confirming their email address. This requires third-party scripts from hCaptcha to be embedded in the email verification page, which may have security and privacy concerns. Users that have been invited through a limited-use invite will not need to solve a CAPTCHA challenge.
+        title: Require new users to go through a CAPTCHA to confirm their account
       enable_keybase:
         desc_html: Allow your users to prove their identity via keybase
         title: Enable keybase integration
diff --git a/config/settings.yml b/config/settings.yml
index b5437caee..7d192f369 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -77,7 +77,7 @@ defaults: &defaults
   show_domain_blocks_rationale: 'disabled'
   outgoing_spoilers: ''
   require_invite_text: false
-  captcha_mode: 'disabled'
+  captcha_enabled: false
 
 development:
   <<: *defaults
diff --git a/spec/views/about/show.html.haml_spec.rb b/spec/views/about/show.html.haml_spec.rb
index 12c96ea49..d608bbf5d 100644
--- a/spec/views/about/show.html.haml_spec.rb
+++ b/spec/views/about/show.html.haml_spec.rb
@@ -10,7 +10,6 @@ describe 'about/show.html.haml', without_verify_partial_doubles: true do
     allow(view).to receive(:site_title).and_return('example site')
     allow(view).to receive(:new_user).and_return(User.new)
     allow(view).to receive(:use_seamless_external_login?).and_return(false)
-    allow(view).to receive(:render_captcha_if_needed).and_return(nil)
   end
 
   it 'has valid open graph tags' do
-- 
cgit