From 3252b9adb05fdcd88879db2c5ce072f822c9217c Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Mon, 25 Mar 2024 14:25:48 +0100
Subject: [PATCH 1/4] Allow adding and removing OmniAuth account links

---
 .../users/omniauth_callbacks_controller.rb    |  47 ++-
 app/models/user.rb                            |  40 ++-
 app/views/users/registrations/edit.html.slim  |  13 +
 config/locales/de/controllers/users.yml       |   7 +
 config/locales/de/views/users.yml             |   4 +
 config/locales/en/controllers/users.yml       |   7 +
 config/locales/en/views/users.yml             |   4 +
 config/routes.rb                              |   4 +
 lib/omni_auth/nonce_store.rb                  |  35 +++
 lib/omni_auth/strategies/abstract_saml.rb     |  30 +-
 .../omniauth_callbacks_controller_spec.rb     | 281 ++++++++++++++++++
 spec/factories/user_identity.rb               |   9 +
 spec/features/rails_admin_spec.rb             |   9 -
 spec/lib/omni_auth/nonce_store_spec.rb        |  93 ++++++
 spec/models/user_spec.rb                      |  80 +++++
 15 files changed, 623 insertions(+), 40 deletions(-)
 create mode 100644 config/locales/de/controllers/users.yml
 create mode 100644 config/locales/en/controllers/users.yml
 create mode 100644 lib/omni_auth/nonce_store.rb
 create mode 100644 spec/controllers/users/omniauth_callbacks_controller_spec.rb
 create mode 100644 spec/factories/user_identity.rb
 create mode 100644 spec/lib/omni_auth/nonce_store_spec.rb

diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 1a16883b4..2431554eb 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'omni_auth/nonce_store'
+
 module Users
   class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     skip_before_action :require_user!
@@ -8,8 +10,16 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     protect_from_forgery except: %i[mocksaml bird nbp]
 
     def sso_callback # rubocop:disable Metrics/AbcSize
+      # Check if an existing user is already signed in (passed through the RelayState)
+      # and trying to add a new identity to their account. If so, we load the user information
+      # and set it as the current user. This is necessary to avoid creating a new user.
+      current_user = User.find_by(id: OmniAuth::NonceStore.pop(params[:RelayState]))
+
+      # For existing users, we want to redirect to their profile page after adding a new identity
+      store_location_for(:user, edit_user_registration_path) if current_user.present?
+
       # The instance variable `@user` is used by Devise internally and should be set here
-      @user = User.from_omniauth(request.env['omniauth.auth'])
+      @user = User.from_omniauth(request.env['omniauth.auth'], current_user)
 
       if @user.persisted?
         # The `sign_in_and_redirect` will only proceed with the login if the account has been confirmed
@@ -36,7 +46,40 @@ def sso_callback # rubocop:disable Metrics/AbcSize
     alias mocksaml sso_callback
 
     def provider
-      request.env['omniauth.auth'].provider
+      request.env['omniauth.auth']&.provider || params[:provider]
+    end
+
+    def deauthorize # rubocop:disable Metrics/AbcSize
+      identity = current_user.identities.find_by(omniauth_provider: provider)
+      if is_navigational_format? && identity&.destroy
+      if is_navigational_format? && identity.nil?
+        # i18n-tasks-use t('users.omni_auth.failure_deauthorize_not_linked')
+        set_flash_message(:alert, :failure_deauthorize_not_linked, kind: OmniAuth::Utils.camelize(provider),
+          scope: 'users.omni_auth')
+      elsif is_navigational_format? && identity.destroy
+        remove_provider_from_session(provider)
+        # i18n-tasks-use t('users.omni_auth.success_deauthorize')
+        set_flash_message(:notice, :success_deauthorize, kind: OmniAuth::Utils.camelize(provider),
+          scope: 'users.omni_auth')
+      elsif is_navigational_format? && identity.errors.any?
+        # i18n-tasks-use t('users.omni_auth.failure_deauthorize')
+        set_flash_message(:alert, :failure_deauthorize, kind: OmniAuth::Utils.camelize(provider), scope: 'users.omni_auth',
+          reason: identity.errors.full_messages.join(', '))
+      end
+      redirect_to edit_user_registration_path
+    end
+
+    private
+
+    def remove_provider_from_session(provider)
+      # Prevent any further interaction with the given provider, as the user has deauthorized it.
+      # This is necessary to avoid the user being redirected to the IdP after signing out.
+      # In short: Once deauthorized, SLO is not enabled any longer for the provider..
+      return unless session['omniauth_provider'] == provider
+
+      session.delete('saml_uid')
+      session.delete('saml_session_index')
+      session.delete('omniauth_provider')
     end
 
     # More info at:
diff --git a/app/models/user.rb b/app/models/user.rb
index 575a34551..91c973e79 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -57,23 +57,33 @@ def destroy
     save!(validate: false)
   end
 
-  def self.from_omniauth(auth) # rubocop:disable Metrics/AbcSize
-    identity = {omniauth_provider: auth.provider, provider_uid: auth.uid}
-
-    user = joins(:identities).where(identities: identity).first_or_initialize do |new_user|
-      # Set these values initially
-      new_user.password = Devise.friendly_token[0, 20]
-      new_user.identities << UserIdentity.new(identity)
-      # If you are using confirmable and the provider(s) you use validate emails,
-      # uncomment the line below to skip the confirmation emails.
-      new_user.skip_confirmation!
+  def self.from_omniauth(auth, user = nil) # rubocop:disable Metrics/AbcSize
+    identity_params = {omniauth_provider: auth.provider, provider_uid: auth.uid}
+    identity = UserIdentity.new(identity_params)
+
+    if user.nil?
+      # A new user signs up with an external account
+      user = joins(:identities).where(identities: identity_params).first_or_initialize do |new_user|
+        # Set these values initially
+        new_user.password = Devise.friendly_token[0, 20]
+        new_user.identities << identity
+        # If you are using confirmable and the provider(s) you use validate emails,
+        # uncomment the line below to skip the confirmation emails.
+        new_user.skip_confirmation!
+      end
+    else
+      # An existing user connects an external account
+      user.identities << identity
     end
 
-    # Update some profile information on every login
-    user.email = auth.info.email
-    user.first_name = auth.info.first_name
-    user.last_name = auth.info.last_name
-    user.save
+    # Update some profile information on every login if present
+    user.assign_attributes(auth.info.slice(:email, :first_name, :last_name).to_h.compact)
+    if user.changed?
+      # We don't want to send a confirmation email for any of the changes
+      user.skip_confirmation_notification!
+      user.skip_reconfirmation!
+      user.save
+    end
     user
   end
 
diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim
index f15a5c679..f0bce8674 100644
--- a/app/views/users/registrations/edit.html.slim
+++ b/app/views/users/registrations/edit.html.slim
@@ -59,6 +59,19 @@
             class: 'form-control'
     small.form-text.text-body-secondary = t('devise.registrations.edit.we_need_your_current_password_to_confirm_your_changes')
 
+
+  .form-group.field-element
+    .form-label
+      = t('.manage_omniauth')
+    .form-content.manage_omniauth
+      - if devise_mapping.omniauthable?
+        - resource_class.omniauth_providers.each do |provider|
+          - provider_name = OmniAuth::Utils.camelize(provider)
+          - if current_user.identities.pluck(:omniauth_provider).include?(provider.to_s)
+            = link_to t('.remove_identity', kind: provider_name), omniauth_deauthorize_path(provider), class: 'btn btn-light btn-sm mb-2 me-2', method: :delete
+          - else
+            = link_to t('.add_identity', kind: provider_name), omniauth_authorize_path(resource_name, provider), class: 'btn btn-light btn-sm mb-2 me-2', method: :post
+
   .form-group.py-3
     .btn-group[role='group']
       = button_tag type: 'submit', class: 'btn btn-important' do
diff --git a/config/locales/de/controllers/users.yml b/config/locales/de/controllers/users.yml
new file mode 100644
index 000000000..b21af1ac7
--- /dev/null
+++ b/config/locales/de/controllers/users.yml
@@ -0,0 +1,7 @@
+---
+de:
+  users:
+    omni_auth:
+      failure_deauthorize: 'Das Trennen von Ihrem %{kind}-Account war aufgrund des folgenden Grundes nicht möglich: "%{reason}".'
+      failure_deauthorize_not_linked: Sie sind nicht mit einem %{kind}-Account verbunden.
+      success_deauthorize: Sie haben sich erfolgreich von Ihrem %{kind}-Account getrennt.
diff --git a/config/locales/de/views/users.yml b/config/locales/de/views/users.yml
index a834a5e1c..0d4405004 100644
--- a/config/locales/de/views/users.yml
+++ b/config/locales/de/views/users.yml
@@ -7,6 +7,10 @@ de:
     registrations:
       avatar_form:
         remove_avatar: Profilbild entfernen
+      edit:
+        add_identity: Account mit %{kind} verknüpfen
+        manage_omniauth: Verknüpfte Accounts verwalten
+        remove_identity: Account-Verknüpfung zu %{kind} entfernen
     shared:
       notification_modal:
         delete_modal:
diff --git a/config/locales/en/controllers/users.yml b/config/locales/en/controllers/users.yml
new file mode 100644
index 000000000..5cb1c82f5
--- /dev/null
+++ b/config/locales/en/controllers/users.yml
@@ -0,0 +1,7 @@
+---
+en:
+  users:
+    omni_auth:
+      failure_deauthorize: Could not remove authorization from %{kind} because "%{reason}".
+      failure_deauthorize_not_linked: You are not linked to any %{kind} account.
+      success_deauthorize: Successfully removed authorization from %{kind} account.
diff --git a/config/locales/en/views/users.yml b/config/locales/en/views/users.yml
index d4306a62b..5c4955034 100644
--- a/config/locales/en/views/users.yml
+++ b/config/locales/en/views/users.yml
@@ -7,6 +7,10 @@ en:
     registrations:
       avatar_form:
         remove_avatar: Remove Avatar
+      edit:
+        add_identity: Link %{kind} account
+        manage_omniauth: Manage linked accounts
+        remove_identity: Remove account link to %{kind}
     shared:
       notification_modal:
         delete_modal:
diff --git a/config/routes.rb b/config/routes.rb
index 206af2d32..b11c7d540 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -20,6 +20,10 @@
     unlocks: 'users/unlocks',
   }
 
+  devise_scope :user do
+    delete '/users/deauth/:provider' => 'users/omniauth_callbacks#deauthorize', as: :omniauth_deauthorize
+  end
+
   resources :users, only: %i[show] do
     resources :account_links, only: %i[new show create edit update destroy] do
       post :remove_shared_user, on: :member
diff --git a/lib/omni_auth/nonce_store.rb b/lib/omni_auth/nonce_store.rb
new file mode 100644
index 000000000..dbdbced48
--- /dev/null
+++ b/lib/omni_auth/nonce_store.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module OmniAuth
+  class NonceStore
+    MAXIMUM_AGE = 30.minutes
+
+    def self.build_cache_key(nonce)
+      "omniauth_nonce_#{nonce}"
+    end
+
+    def self.add(value)
+      nonce = Devise.friendly_token
+      cache.write(build_cache_key(nonce), value, expires_in: MAXIMUM_AGE)
+      nonce
+    end
+
+    def self.read(nonce)
+      cache.read(build_cache_key(nonce))
+    end
+
+    def self.delete(nonce)
+      cache.delete(build_cache_key(nonce))
+    end
+
+    def self.pop(nonce)
+      value = read(nonce)
+      delete(nonce) if value
+      value
+    end
+
+    def self.cache
+      @cache ||= ActiveSupport::Cache.lookup_store(:file_store, '/tmp/cache/omniauth')
+    end
+  end
+end
diff --git a/lib/omni_auth/strategies/abstract_saml.rb b/lib/omni_auth/strategies/abstract_saml.rb
index bea7bf6c1..b20cbada0 100644
--- a/lib/omni_auth/strategies/abstract_saml.rb
+++ b/lib/omni_auth/strategies/abstract_saml.rb
@@ -24,13 +24,12 @@ class AbstractSaml < OmniAuth::Strategies::SAML
         want_assertions_encrypted: true, # Invalidate SAML messages without an EncryptedAssertion
       }
 
-      # Our auto-login mechanism passes a desired redirect path (signed with a
-      # secret to prevent tampering) to the request phase.
+      # Our auto-login mechanism passes a desired application state to the request phase.
+      # So far, this is used to store a temporary token for the current user in the RelayState.
       # If we forward this via SAML's standard "RelayState" parameter, we will
       # get it back in the callback phase.
-      # TODO: This parameter is not yet used in the app
-      option :idp_sso_target_url_runtime_params, {
-        redirect_path: 'RelayState',
+      option :idp_sso_service_url_runtime_params, {
+        relay_state: 'RelayState',
       }
 
       # This Lambda is responsible for terminating the session and will
@@ -72,13 +71,20 @@ class AbstractSaml < OmniAuth::Strategies::SAML
 
       uid { @name_id || @attributes['urn:oid:0.9.2342.19200300.100.1.1'] }
 
-      def with_settings
+      def with_settings # rubocop:disable Metrics/AbcSize
         # Get persistent IDs to recognize returning users
         options[:name_identifier_format] = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
 
         options[:sp_entity_id] ||= sso_path
         options[:single_logout_service_url] ||= slo_path
         options[:slo_default_relay_state] ||= full_host
+
+        if on_request_path? && current_user
+          # We want to store the current user in the SAML RelayState,
+          # so that we can auto-login the user in the callback phase.
+          # This allows a registered user to add further OmniAuth providers.
+          request.params['relay_state'] = NonceStore.add current_user.id
+        end
         super
       end
 
@@ -91,15 +97,11 @@ def slo_path
         "#{full_host}#{script_name}#{request_path}/slo"
       end
 
-      class << self
-        def sign(url)
-          Rails.application.message_verifier('saml').generate(url)
-        end
-
-        def try_verify(url)
-          Rails.application.message_verifier('saml').verified(url)
-        end
+      def current_user
+        env['warden'].user
+      end
 
+      class << self
         def desired_bindings
           {
             sso_binding: 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
diff --git a/spec/controllers/users/omniauth_callbacks_controller_spec.rb b/spec/controllers/users/omniauth_callbacks_controller_spec.rb
new file mode 100644
index 000000000..c82ce6fda
--- /dev/null
+++ b/spec/controllers/users/omniauth_callbacks_controller_spec.rb
@@ -0,0 +1,281 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Users::OmniauthCallbacksController do
+  render_views
+
+  let(:omniauth_provider) { 'provider' }
+
+  before do
+    OmniAuth.config.test_mode = true
+    request.env['devise.mapping'] = Devise.mappings[:user]
+
+    # Add a new provider to the omniauth config and reload routes
+    Devise.omniauth :sso_callback
+    Rails.application.reload_routes!
+  end
+
+  after do
+    # Remove the provider from the omniauth config and reload routes
+    Devise.class_variable_set(:@@omniauth_configs, {}) # rubocop:disable Style/ClassVars
+    Rails.application.reload_routes!
+  end
+
+  describe '#sso_callback' do
+    let(:info) { attributes_for(:user) }
+    let(:auth) { OmniAuth::AuthHash.new(provider: omniauth_provider, uid: 'uid', info:) }
+
+    before do
+      request.env['omniauth.auth'] = auth
+    end
+
+    context 'when user is new' do
+      it 'creates a new user' do
+        expect { post :sso_callback }.to change(User, :count).by(1)
+      end
+
+      it 'creates a new identity' do
+        expect { post :sso_callback }.to change(UserIdentity, :count).by(1)
+      end
+
+      it 'redirects to the root page' do
+        post :sso_callback
+        expect(response).to redirect_to root_path
+      end
+
+      context 'when no user can be created' do
+        before do
+          # Create a user with the same information passed through the omniauth hash; this will fail validation.
+          create(:user, **info)
+        end
+
+        it 'does not create a new user' do
+          expect { post :sso_callback }.not_to change(User, :count)
+        end
+
+        it 'does not create a new identity' do
+          expect { post :sso_callback }.not_to change(UserIdentity, :count)
+        end
+
+        it 'redirects to the new_user_registration_url' do
+          post :sso_callback
+          expect(response).to redirect_to new_user_registration_url
+        end
+      end
+    end
+
+    context 'when user exists' do
+      let(:user) { create(:user) }
+
+      context 'when signing in' do
+        before do
+          create(:user_identity, user:, omniauth_provider: auth.provider, provider_uid: auth.uid)
+        end
+
+        it 'does not create a new user' do
+          expect { post :sso_callback }.not_to change(User, :count)
+        end
+
+        it 'does not create a new identity' do
+          expect { post :sso_callback }.not_to change(UserIdentity, :count)
+        end
+
+        it 'redirects to root path' do
+          post :sso_callback
+          expect(response).to redirect_to root_path
+        end
+      end
+
+      context 'when adding a new identity' do
+        before do
+          allow(OmniAuth::NonceStore).to receive(:pop).and_return(user.id)
+        end
+
+        it 'does not create a new user' do
+          expect { post :sso_callback }.not_to change(User, :count)
+        end
+
+        it 'creates a new identity' do
+          expect { post :sso_callback }.to change(UserIdentity, :count).by(1)
+        end
+
+        it 'redirects to the edit_user_registration_path' do
+          post :sso_callback
+          expect(response).to redirect_to edit_user_registration_path
+        end
+      end
+    end
+  end
+
+  describe '#deauthorize' do
+    shared_examples 'no removal of affected provider in the session' do
+      let(:provider) { omniauth_provider }
+
+      before do
+        session['omniauth_provider'] = provider
+        session['saml_uid'] = 'saml_uid'
+        session['saml_session_index'] = 'saml_session_index'
+        delete :deauthorize, params: {provider: omniauth_provider}
+      end
+
+      it 'does not remove the provider from the session' do
+        expect(session['omniauth_provider']).to eq provider
+      end
+
+      it 'does not remove the saml_uid from the session' do
+        expect(session['saml_uid']).to eq 'saml_uid'
+      end
+
+      it 'does not remove the saml_session_index from the session' do
+        expect(session['saml_session_index']).to eq 'saml_session_index'
+      end
+    end
+
+    shared_examples 'no change to to other providers in the session' do
+      let(:provider) { 'another_provider' }
+
+      before do
+        session['omniauth_provider'] = provider
+        session['saml_uid'] = 'saml_uid'
+        session['saml_session_index'] = 'saml_session_index'
+        delete :deauthorize, params: {provider: omniauth_provider}
+      end
+
+      it 'removes the provider from the session' do
+        expect(session['omniauth_provider']).to eq provider
+      end
+
+      it 'removes the saml_uid from the session' do
+        expect(session['saml_uid']).to eq 'saml_uid'
+      end
+
+      it 'removes the saml_session_index from the session' do
+        expect(session['saml_session_index']).to eq 'saml_session_index'
+      end
+    end
+
+    shared_examples 'no addition of the provider to the session' do
+      before do
+        delete :deauthorize, params: {provider: omniauth_provider}
+      end
+
+      it 'does not add the provider to the session' do
+        expect(session['omniauth_provider']).to be_nil
+      end
+
+      it 'does not add the saml_uid to the session' do
+        expect(session['saml_uid']).to be_nil
+      end
+
+      it 'does not add the saml_session_index to the session' do
+        expect(session['saml_session_index']).to be_nil
+      end
+    end
+
+    let(:user) { create(:user) }
+    let(:identity) { create(:user_identity, user:, omniauth_provider:) }
+
+    context 'when identity exists' do
+      before do
+        allow(controller).to receive(:current_user).and_return(user)
+        allow(user.identities).to receive(:find_by).with(omniauth_provider:).and_return(identity)
+      end
+
+      it 'destroys the identity' do
+        expect(identity).to receive(:destroy)
+        delete :deauthorize, params: {provider: omniauth_provider}
+      end
+
+      it 'decreases the identity count' do
+        expect { delete :deauthorize, params: {provider: omniauth_provider} }.to change(UserIdentity, :count).by(-1)
+      end
+
+      it 'redirects to edit user registration path' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(response).to redirect_to edit_user_registration_path
+      end
+
+      it 'sets a flash message' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(flash[:notice]).to eq I18n.t('users.omni_auth.success_deauthorize', kind: OmniAuth::Utils.camelize(omniauth_provider))
+      end
+
+      context 'when session contains the provider' do
+        before do
+          session['omniauth_provider'] = omniauth_provider
+          session['saml_uid'] = 'saml_uid'
+          session['saml_session_index'] = 'saml_session_index'
+          delete :deauthorize, params: {provider: omniauth_provider}
+        end
+
+        it 'removes the provider from the session' do
+          expect(session['omniauth_provider']).to be_nil
+        end
+
+        it 'removes the saml_uid from the session' do
+          expect(session['saml_uid']).to be_nil
+        end
+
+        it 'removes the saml_session_index from the session' do
+          expect(session['saml_session_index']).to be_nil
+        end
+      end
+
+      it_behaves_like 'no change to to other providers in the session'
+      it_behaves_like 'no addition of the provider to the session'
+    end
+
+    context 'when identity does not exist' do
+      before do
+        allow(controller).to receive(:current_user).and_return(user)
+        allow(user.identities).to receive(:find_by).with(omniauth_provider:).and_return(nil)
+      end
+
+      it 'does not destroy the identity' do
+        expect(identity).not_to receive(:destroy)
+        delete :deauthorize, params: {provider: omniauth_provider}
+      end
+
+      it 'redirects to edit user registration path' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(response).to redirect_to edit_user_registration_path
+      end
+
+      it 'sets a flash message' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(flash[:alert]).to eq I18n.t('users.omni_auth.failure_deauthorize_not_linked', kind: OmniAuth::Utils.camelize(omniauth_provider))
+      end
+
+      it_behaves_like 'no removal of affected provider in the session'
+      it_behaves_like 'no change to to other providers in the session'
+      it_behaves_like 'no addition of the provider to the session'
+    end
+
+    context 'when identity cannot be deleted' do
+      let(:user_identity) { build(:user_identity, user:, omniauth_provider:, provider_uid: nil) }
+
+      before do
+        allow(controller).to receive(:current_user).and_return(user)
+        allow(user.identities).to receive(:find_by).with(omniauth_provider:).and_return(user_identity)
+        user_identity.valid?
+        allow(user_identity).to receive(:destroy).and_return(false)
+      end
+
+      it 'redirects to edit user registration path' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(response).to redirect_to edit_user_registration_path
+      end
+
+      it 'sets a flash message' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        reason = "#{UserIdentity.human_attribute_name('provider_uid')} can't be blank"
+        expect(flash[:alert]).to eq I18n.t('users.omni_auth.failure_deauthorize', kind: OmniAuth::Utils.camelize(omniauth_provider), reason:)
+      end
+
+      it_behaves_like 'no removal of affected provider in the session'
+      it_behaves_like 'no change to to other providers in the session'
+      it_behaves_like 'no addition of the provider to the session'
+    end
+  end
+end
diff --git a/spec/factories/user_identity.rb b/spec/factories/user_identity.rb
new file mode 100644
index 000000000..f8d97042b
--- /dev/null
+++ b/spec/factories/user_identity.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :user_identity do
+    user
+    omniauth_provider { 'provider' }
+    provider_uid { '123456' }
+  end
+end
diff --git a/spec/features/rails_admin_spec.rb b/spec/features/rails_admin_spec.rb
index d619a1429..1086bdceb 100644
--- a/spec/features/rails_admin_spec.rb
+++ b/spec/features/rails_admin_spec.rb
@@ -6,15 +6,6 @@
   let(:user) { create(role) }
   let(:password) { attributes_for(role)[:password] }
 
-  before do
-    # RailsAdmin will automatically eager load all models (and some further classes).
-    # Since this eager loading is not done as expected, the OmniAuth strategies
-    # dynamically configuring themselves with an URL will fail and raise an error.
-    # To prevent this, we remove the OmniAuth constant from the global namespace,
-    # so that the strategies are not loaded and the error is not raised.
-    Object.send(:remove_const, :OmniAuth) if defined?(OmniAuth)
-  end
-
   context 'when signed out' do
     it 'redirects to the root page' do
       visit(rails_admin.dashboard_path)
diff --git a/spec/lib/omni_auth/nonce_store_spec.rb b/spec/lib/omni_auth/nonce_store_spec.rb
new file mode 100644
index 000000000..940994ec8
--- /dev/null
+++ b/spec/lib/omni_auth/nonce_store_spec.rb
@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe OmniAuth::NonceStore do
+  let(:nonce) { SecureRandom.hex }
+  let(:value) { 'value' }
+  let(:cache) { ActiveSupport::Cache.lookup_store(:memory_store) }
+
+  before do
+    stub_const('OmniAuth::NonceStore::MAXIMUM_AGE', 1)
+    allow(Devise).to receive(:friendly_token).and_return(nonce)
+    allow(described_class).to receive(:cache).and_return(cache)
+  end
+
+  describe '.add' do
+    it 'stores a nonce in the cache' do
+      expect(cache).to receive(:write)
+      described_class.add(value)
+    end
+
+    it 'returns the nonce' do
+      expect(described_class.add(value)).to eq(nonce)
+    end
+  end
+
+  describe '.delete' do
+    it 'deletes a nonce from the cache' do
+      expect(cache).to receive(:delete)
+      described_class.add(value)
+      described_class.delete(nonce)
+    end
+  end
+
+  describe '.read' do
+    it 'returns the value for present nonces' do
+      described_class.add(value)
+      expect(described_class.read(nonce)).to eq value
+    end
+
+    it 'returns nil for expired nonces' do
+      described_class.add(value)
+      expect(described_class.read(nonce)).to eq value
+      sleep(OmniAuth::NonceStore::MAXIMUM_AGE)
+      expect(described_class.read(nonce)).to be_nil
+    end
+
+    it 'returns nil for absent nonces' do
+      expect(described_class.read(nonce)).to be_nil
+    end
+
+    it 'returns nil for deleted nonces' do
+      described_class.add(value)
+      expect(described_class.read(nonce)).to eq value
+      described_class.delete(nonce)
+      expect(described_class.read(nonce)).to be_nil
+    end
+  end
+
+  describe '.pop' do
+    it 'returns the value for present nonces' do
+      described_class.add(value)
+      expect(described_class.pop(nonce)).to eq value
+    end
+
+    it 'returns nil for expired nonces' do
+      described_class.add(value)
+      expect(described_class.pop(nonce)).to eq value
+      sleep(OmniAuth::NonceStore::MAXIMUM_AGE)
+      expect(described_class.pop(nonce)).to be_nil
+    end
+
+    it 'returns nil for absent nonces' do
+      expect(described_class.pop(nonce)).to be_nil
+    end
+
+    it 'deletes the nonce' do
+      described_class.add(value)
+      expect(described_class.pop(nonce)).to eq value
+      expect(described_class.pop(nonce)).to be_nil
+    end
+  end
+
+  describe '.cache' do
+    before do
+      allow(described_class).to receive(:cache).and_call_original
+    end
+
+    it 'returns a file store' do
+      expect(described_class.cache).to be_a(ActiveSupport::Cache::FileStore)
+    end
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 5cc70817a..3fb35c711 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -250,4 +250,84 @@
       end
     end
   end
+
+  describe '.from_omniauth' do
+    let(:auth) { OmniAuth::AuthHash.new(provider: 'provider', uid: 'uid', info: {email: 'test@example.com', first_name: 'Test', last_name: 'User'}) }
+    let(:user) { create(:user) }
+
+    context 'when user is new' do
+      it 'creates a new user' do
+        expect { described_class.from_omniauth(auth) }.to change(described_class, :count).by(1)
+      end
+
+      it 'creates a new identity' do
+        expect { described_class.from_omniauth(auth) }.to change(UserIdentity, :count).by(1)
+      end
+
+      it 'returns a User instance' do
+        expect(described_class.from_omniauth(auth)).to be_a(described_class)
+      end
+
+      it 'returns a user with the correct attributes' do
+        user = described_class.from_omniauth(auth)
+        expect(user&.email).to eq(auth.info.email)
+        expect(user&.first_name).to eq(auth.info.first_name)
+        expect(user&.last_name).to eq(auth.info.last_name)
+      end
+    end
+
+    context 'when user exists' do
+      before { create(:user_identity, user:, omniauth_provider: auth.provider, provider_uid: auth.uid) }
+
+      it 'does not create a new user' do
+        expect { described_class.from_omniauth(auth) }.not_to change(described_class, :count)
+      end
+
+      it 'does not create a new identity' do
+        expect { described_class.from_omniauth(auth) }.not_to change(UserIdentity, :count)
+      end
+
+      it 'returns a User instance' do
+        expect(described_class.from_omniauth(auth)).to be_a(described_class)
+      end
+
+      it 'returns the existing user' do
+        expect(described_class.from_omniauth(auth)).to eq(user)
+      end
+
+      it 'updates the user attributes' do
+        user = described_class.from_omniauth(auth)
+        expect(user&.email).to eq(auth.info.email)
+        expect(user&.first_name).to eq(auth.info.first_name)
+        expect(user&.last_name).to eq(auth.info.last_name)
+      end
+    end
+
+    context 'when user exists but identity does not' do
+      let!(:user) { create(:user, email: auth.info.email) }
+
+      it 'does not create a new user' do
+        expect { described_class.from_omniauth(auth, user) }.not_to change(described_class, :count)
+      end
+
+      it 'creates a new identity' do
+        expect { described_class.from_omniauth(auth, user) }.to change(UserIdentity, :count).by(1)
+      end
+
+      it 'returns a User instance' do
+        expect(described_class.from_omniauth(auth, user)).to be_a(described_class)
+      end
+
+      it 'returns the existing user' do
+        expect(described_class.from_omniauth(auth, user)).to eq(user)
+      end
+
+      it 'updates the user attributes' do
+        returned_user = described_class.from_omniauth(auth, user)
+        expect(returned_user&.email).to eq(auth.info.email)
+        expect(returned_user&.first_name).to eq(user.first_name)
+        expect(returned_user&.last_name).to eq(user.last_name)
+      end
+    end
+  end
 end

From 9bd3dbce7e2e7bcc5f5f7c73a9c1bf45c0001876 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 31 Mar 2024 10:16:57 +0200
Subject: [PATCH 2/4] Allow OmniAuth users to perform changes to their account

In order to allow changes, we disable the password requirement for those users. As soon as a password is set for OmniAuth users, the regular workflow requiring the current password is restored.
---
 app/controllers/users/passwords_controller.rb |  10 +-
 .../users/registrations_controller.rb         |  32 ++++
 app/models/user.rb                            |   2 +
 app/views/users/registrations/edit.html.slim  |  17 +-
 config/locales/de/models.yml                  |   1 +
 config/locales/en/models.yml                  |   1 +
 ...0240329215957_add_password_set_to_users.rb |   7 +
 db/schema.rb                                  |   3 +-
 .../users/passwords_controller_spec.rb        |  63 +++++++
 .../users/registrations_controller_spec.rb    | 157 ++++++++++++++++++
 10 files changed, 281 insertions(+), 12 deletions(-)
 create mode 100644 db/migrate/20240329215957_add_password_set_to_users.rb
 create mode 100644 spec/controllers/users/passwords_controller_spec.rb
 create mode 100644 spec/controllers/users/registrations_controller_spec.rb

diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb
index 1af33621d..e6b86fdc5 100644
--- a/app/controllers/users/passwords_controller.rb
+++ b/app/controllers/users/passwords_controller.rb
@@ -20,9 +20,13 @@ class PasswordsController < Devise::PasswordsController
     # end
 
     # PUT /resource/password
-    # def update
-    #   super
-    # end
+    def update
+      super do |resource|
+        # When the user was updated successfully, a custom password was set.
+        # The `resource.errors.empty?` is also used by Devise internally.
+        resource.update(password_set: true) if resource.errors.empty?
+      end
+    end
 
     # protected
 
diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb
index a116b077e..23df9f4ac 100644
--- a/app/controllers/users/registrations_controller.rb
+++ b/app/controllers/users/registrations_controller.rb
@@ -26,6 +26,12 @@ class RegistrationsController < Devise::RegistrationsController
     def update
       super do |resource|
         resource.avatar.purge if params[:user][:avatar].nil? && params[:user][:avatar_present] == 'false'
+        if resource.password_set_changed?
+          # If a user tried to set a password but failed, we need to reset the password_set flag.
+          # Further, we need to remove the current_password error, since the user didn't enter their current password.
+          resource.errors.delete(:current_password)
+          resource.restore_password_set!
+        end
       end
     end
 
@@ -68,5 +74,31 @@ def after_update_path_for(resource)
     # def after_inactive_sign_up_path_for(resource)
     #   super(resource)
     # end
+    def set_password_for_omniauth(resource, params)
+      if !resource.password_set? && params[:password].present?
+        # If an OmniAuth user tries to set a password, we need to take two extra steps:
+        # 1. Set the password_set flag to true, since a custom password is being set.
+        # 2. Set the encrypted_password and current_password to a dummy value, since the user doesn't have a real password.
+        #    This is needed by Devise to update the password and validate the "current" password.
+        params[:current_password] = Devise.friendly_token
+        resource.assign_attributes(
+          password_set: true,
+          password: params[:password],
+          password_confirmation: params[:password_confirmation],
+          encrypted_password: Devise::Encryptor.digest(resource.class, params[:current_password])
+        )
+      end
+    end
+
+    def update_resource(resource, params)
+      set_password_for_omniauth(resource, params)
+
+      # Only require current password if the user configured one. Otherwise (i.e., for OmniAuth users), don't require it.
+      if resource.password_set?
+        resource.update_with_password(params)
+      else
+        resource.update_without_password(params)
+      end
+    end
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 91c973e79..ee5a3944f 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -16,6 +16,7 @@ class User < ApplicationRecord
 
   validates :email, presence: true, uniqueness: {case_sensitive: false}
   validates :first_name, :last_name, presence: true
+  validates :password_set, inclusion: [true, false]
 
   has_many :tasks, dependent: :nullify
 
@@ -66,6 +67,7 @@ def self.from_omniauth(auth, user = nil) # rubocop:disable Metrics/AbcSize
       user = joins(:identities).where(identities: identity_params).first_or_initialize do |new_user|
         # Set these values initially
         new_user.password = Devise.friendly_token[0, 20]
+        new_user.password_set = false
         new_user.identities << identity
         # If you are using confirmable and the provider(s) you use validate emails,
         # uncomment the line below to skip the confirmation emails.
diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim
index f0bce8674..8a2a1a3cc 100644
--- a/app/views/users/registrations/edit.html.slim
+++ b/app/views/users/registrations/edit.html.slim
@@ -50,14 +50,15 @@
 
   = render 'avatar_form', f: f
 
-  .form-group.field-element
-    = f.label :current_password, class: 'form-label'
-    = f.password_field :current_password,
-            required: true,
-            placeholder: User.human_attribute_name('current_password'),
-            autocomplete: 'current-password',
-            class: 'form-control'
-    small.form-text.text-body-secondary = t('devise.registrations.edit.we_need_your_current_password_to_confirm_your_changes')
+  - if resource.password_set?
+    .form-group.field-element
+      = f.label :current_password, class: 'form-label'
+      = f.password_field :current_password,
+              required: true,
+              placeholder: User.human_attribute_name('current_password'),
+              autocomplete: 'current-password',
+              class: 'form-control'
+      small.form-text.text-body-secondary = t('devise.registrations.edit.we_need_your_current_password_to_confirm_your_changes')
 
 
   .form-group.field-element
diff --git a/config/locales/de/models.yml b/config/locales/de/models.yml
index 763a8f2a5..336a28a8b 100644
--- a/config/locales/de/models.yml
+++ b/config/locales/de/models.yml
@@ -88,6 +88,7 @@ de:
       user:
         first_name: Vorname
         last_name: Nachname
+        password_set: Passwort gesetzt
         role: Rolle
       user_identity:
         omniauth_provider: OmniAuth-Anbieter
diff --git a/config/locales/en/models.yml b/config/locales/en/models.yml
index 0a9711f30..160d5b4e8 100644
--- a/config/locales/en/models.yml
+++ b/config/locales/en/models.yml
@@ -88,6 +88,7 @@ en:
       user:
         first_name: First name
         last_name: Last name
+        password_set: Passwort set
         role: Role
       user_identity:
         omniauth_provider: OmniAuth provider
diff --git a/db/migrate/20240329215957_add_password_set_to_users.rb b/db/migrate/20240329215957_add_password_set_to_users.rb
new file mode 100644
index 000000000..7eb81c9ab
--- /dev/null
+++ b/db/migrate/20240329215957_add_password_set_to_users.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddPasswordSetToUsers < ActiveRecord::Migration[7.1]
+  def change
+    add_column :users, :password_set, :boolean, default: true, null: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ca7f5cac9..0a5220f1a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_03_12_201146) do
+ActiveRecord::Schema[7.1].define(version: 2024_03_29_215957) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -316,6 +316,7 @@
     t.string "unlock_token"
     t.datetime "locked_at"
     t.string "preferred_locale"
+    t.boolean "password_set", default: true, null: false
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
diff --git a/spec/controllers/users/passwords_controller_spec.rb b/spec/controllers/users/passwords_controller_spec.rb
new file mode 100644
index 000000000..b0b296f59
--- /dev/null
+++ b/spec/controllers/users/passwords_controller_spec.rb
@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Users::PasswordsController do
+  render_views
+
+  let(:user) { create(:user, reset_password_token: reset_password_token_for_db, reset_password_sent_at: Time.zone.now) }
+  let(:new_password) { 'new_password' }
+  let(:reset_password_token) { Devise.friendly_token }
+  let(:reset_password_token_for_db) { Devise.token_generator&.digest(User, :reset_password_token, reset_password_token) }
+
+  before do
+    user.save
+    request.env['devise.mapping'] = Devise.mappings[:user]
+  end
+
+  shared_examples 'a successful password update' do
+    before do
+      put :update, params: {user: {password: new_password, password_confirmation: new_password, reset_password_token:}}
+    end
+
+    it 'sets the password_set attribute to true' do
+      expect(user.reload.password_set).to be true
+    end
+
+    it 'updates the password' do
+      expect(user.reload.valid_password?(new_password)).to be true
+    end
+  end
+
+  shared_examples 'a failed password update' do
+    before do
+      put :update, params: {user: {password: new_password, password_confirmation: 'mistyped_password', reset_password_token:}}
+    end
+
+    it 'does not change the password_set attribute' do
+      expect(user.reload.password_set).to be password_set
+    end
+
+    it 'does not update the password' do
+      expect(user.reload.valid_password?(new_password)).to be false
+    end
+  end
+
+  describe '#update' do
+    context 'when no password is set yet (i.e., for OmniAuth users)' do
+      let(:password_set) { false }
+
+      before { user.update(password_set:) }
+
+      it_behaves_like 'a successful password update'
+      it_behaves_like 'a failed password update'
+    end
+
+    context 'when password is already set' do
+      let(:password_set) { true }
+
+      it_behaves_like 'a successful password update'
+      it_behaves_like 'a failed password update'
+    end
+  end
+end
diff --git a/spec/controllers/users/registrations_controller_spec.rb b/spec/controllers/users/registrations_controller_spec.rb
new file mode 100644
index 000000000..bda86fc91
--- /dev/null
+++ b/spec/controllers/users/registrations_controller_spec.rb
@@ -0,0 +1,157 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Users::RegistrationsController do
+  render_views
+
+  let(:user) { create(:user) }
+  let(:new_password) { 'new_password' }
+  let(:new_first_name) { 'New Name' }
+
+  before do
+    request.env['devise.mapping'] = Devise.mappings[:user]
+  end
+
+  describe '#update' do
+    context 'when avatar was removed' do
+      before { user.avatar.attach(io: Rails.root.join('spec/fixtures/files/avatar/profile.png').open, filename: 'profile.png') }
+
+      it 'purges the avatar' do
+        sign_in user
+        put :update, params: {user: {avatar_present: 'false'}}
+        expect(user.reload.avatar.attached?).to be false
+      end
+    end
+
+    context 'when no password is set yet (i.e., for OmniAuth users)' do
+      before { user.update(password_set: false) }
+
+      context 'when a user attempted to set a password but failed' do
+        before do
+          sign_in user
+          put :update, params: {user: {first_name: new_first_name, password: new_password, password_confirmation: 'mistyped_new_password'}}
+        end
+
+        it 'does not require the current_password' do
+          expect(response.body).not_to include(I18n.t('activerecord.attributes.user.current_password'))
+        end
+
+        it 'does not change the password_set flag' do
+          expect(user.reload.password_set).to be false
+        end
+
+        it 'does not update the password' do
+          expect(user.reload.valid_password?(new_password)).to be false
+        end
+
+        it 'does not update to the new name' do
+          expect(user.reload.first_name).not_to eq new_first_name
+        end
+      end
+
+      context 'when a user successfully sets a password' do
+        before do
+          sign_in user
+          put :update, params: {user: {first_name: new_first_name, password: new_password, password_confirmation: new_password}}
+        end
+
+        it 'requires the current_password' do
+          # We need to sign in again as the user changed. This is a Devise limitation.
+          sign_in user.reload
+          get :edit
+          expect(response.body).to include(I18n.t('activerecord.attributes.user.current_password'))
+        end
+
+        it 'changes the password_set flag' do
+          expect(user.reload.password_set).to be true
+        end
+
+        it 'updates the password' do
+          expect(user.reload.valid_password?(new_password)).to be true
+        end
+
+        it 'updates to the new name' do
+          expect(user.reload.first_name).to eq new_first_name
+        end
+      end
+
+      context 'when a user updates without a password' do
+        before do
+          sign_in user
+          put :update, params: {user: {first_name: new_first_name}}
+        end
+
+        it 'does not require the current_password' do
+          # We need to sign in again as the user changed. This is a Devise limitation.
+          sign_in user.reload
+          get :edit
+          expect(response.body).not_to include(I18n.t('activerecord.attributes.user.current_password'))
+        end
+
+        it 'does not change the password_set flag' do
+          expect(user.reload.password_set).to be false
+        end
+
+        it 'does not update the password' do
+          expect(user.reload.valid_password?(new_password)).to be false
+        end
+
+        it 'updates to the new name' do
+          expect(user.reload.first_name).to eq new_first_name
+        end
+      end
+    end
+
+    context 'when a password is already set' do
+      context 'when the user updates with their current password' do
+        before do
+          sign_in user
+          put :update, params: {user: {first_name: new_first_name, password: new_password, password_confirmation: new_password, current_password: user.password}}
+        end
+
+        it 'requires the current_password' do
+          # We need to sign in again as the user changed. This is a Devise limitation.
+          sign_in user.reload
+          get :edit
+          expect(response.body).to include(I18n.t('activerecord.attributes.user.current_password'))
+        end
+
+        it 'updates the password' do
+          expect(user.reload.valid_password?(new_password)).to be true
+        end
+
+        it 'does not modify the password_set flag' do
+          expect(user.reload.password_set).to be true
+        end
+
+        it 'updates to the new name' do
+          expect(user.reload.first_name).to eq new_first_name
+        end
+      end
+
+      context 'when the user updates without their current password' do
+        before do
+          sign_in user
+          put :update, params: {user: {first_name: new_first_name}}
+        end
+
+        it 'requires the current_password' do
+          expect(response.body).to include(I18n.t('activerecord.attributes.user.current_password'))
+        end
+
+        it 'does not update the password' do
+          expect(user.reload.valid_password?(new_password)).to be false
+        end
+
+        it 'does not modify the password_set flag' do
+          expect(user.reload.password_set).to be true
+        end
+
+        it 'does not update to the new name' do
+          expect(user.reload.first_name).not_to eq new_first_name
+        end
+      end
+    end
+  end
+end

From e537f7dac72dc907f50d6f0f0d6d6e1994cb14ed Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 31 Mar 2024 13:28:57 +0200
Subject: [PATCH 3/4] Prevent deletion of last OmniAuth identity

---
 .../users/omniauth_callbacks_controller.rb    |  7 +-
 app/views/users/registrations/edit.html.slim  | 16 ++--
 config/locales/de/controllers/users.yml       |  1 +
 config/locales/de/views/users.yml             |  1 +
 config/locales/en/controllers/users.yml       |  1 +
 config/locales/en/views/users.yml             |  1 +
 .../omniauth_callbacks_controller_spec.rb     | 33 ++++++-
 .../users/registrations_controller_spec.rb    | 93 +++++++++++++++++++
 8 files changed, 142 insertions(+), 11 deletions(-)

diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 2431554eb..ee759fb74 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -49,13 +49,16 @@ def provider
       request.env['omniauth.auth']&.provider || params[:provider]
     end
 
-    def deauthorize # rubocop:disable Metrics/AbcSize
+    def deauthorize # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
       identity = current_user.identities.find_by(omniauth_provider: provider)
-      if is_navigational_format? && identity&.destroy
       if is_navigational_format? && identity.nil?
         # i18n-tasks-use t('users.omni_auth.failure_deauthorize_not_linked')
         set_flash_message(:alert, :failure_deauthorize_not_linked, kind: OmniAuth::Utils.camelize(provider),
           scope: 'users.omni_auth')
+      elsif is_navigational_format? && current_user.identities.count == 1 && !current_user.password_set?
+        # i18n-tasks-use t('users.omni_auth.failure_deauthorize_last_identity')
+        set_flash_message(:alert, :failure_deauthorize_last_identity, kind: OmniAuth::Utils.camelize(provider),
+          scope: 'users.omni_auth')
       elsif is_navigational_format? && identity.destroy
         remove_provider_from_session(provider)
         # i18n-tasks-use t('users.omni_auth.success_deauthorize')
diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim
index 8a2a1a3cc..579e1cde5 100644
--- a/app/views/users/registrations/edit.html.slim
+++ b/app/views/users/registrations/edit.html.slim
@@ -61,15 +61,19 @@
       small.form-text.text-body-secondary = t('devise.registrations.edit.we_need_your_current_password_to_confirm_your_changes')
 
 
-  .form-group.field-element
-    .form-label
-      = t('.manage_omniauth')
-    .form-content.manage_omniauth
-      - if devise_mapping.omniauthable?
+  - if devise_mapping.omniauthable? && resource_class.omniauth_providers.any?
+    .form-group.field-element
+      .form-label
+        = t('.manage_omniauth')
+      .form-content.manage_omniauth
         - resource_class.omniauth_providers.each do |provider|
           - provider_name = OmniAuth::Utils.camelize(provider)
-          - if current_user.identities.pluck(:omniauth_provider).include?(provider.to_s)
+          - configured_providers = current_user.identities.pluck(:omniauth_provider)
+          - if configured_providers.include?(provider.to_s) && (configured_providers.size > 1 || resource.password_set?)
             = link_to t('.remove_identity', kind: provider_name), omniauth_deauthorize_path(provider), class: 'btn btn-light btn-sm mb-2 me-2', method: :delete
+          - elsif configured_providers.include?(provider.to_s)
+            a.btn.btn-light.btn-sm.mb-2.me-2.disabled href='#'
+              = t('.cannot_remove_last_identity', kind: provider_name)
           - else
             = link_to t('.add_identity', kind: provider_name), omniauth_authorize_path(resource_name, provider), class: 'btn btn-light btn-sm mb-2 me-2', method: :post
 
diff --git a/config/locales/de/controllers/users.yml b/config/locales/de/controllers/users.yml
index b21af1ac7..5095d7843 100644
--- a/config/locales/de/controllers/users.yml
+++ b/config/locales/de/controllers/users.yml
@@ -3,5 +3,6 @@ de:
   users:
     omni_auth:
       failure_deauthorize: 'Das Trennen von Ihrem %{kind}-Account war aufgrund des folgenden Grundes nicht möglich: "%{reason}".'
+      failure_deauthorize_last_identity: Sie können sich nicht von Ihrem %{kind}-Account trennen, da dies Ihr letzter verbleibender Account ist. Bitte erstellen Sie ein Passwort, um sich in Zukunft anmelden zu können.
       failure_deauthorize_not_linked: Sie sind nicht mit einem %{kind}-Account verbunden.
       success_deauthorize: Sie haben sich erfolgreich von Ihrem %{kind}-Account getrennt.
diff --git a/config/locales/de/views/users.yml b/config/locales/de/views/users.yml
index 0d4405004..081e5f1ed 100644
--- a/config/locales/de/views/users.yml
+++ b/config/locales/de/views/users.yml
@@ -9,6 +9,7 @@ de:
         remove_avatar: Profilbild entfernen
       edit:
         add_identity: Account mit %{kind} verknüpfen
+        cannot_remove_last_identity: Account-Verknüpfung zu %{kind} kann nicht entfernt werden, da weder ein Passwort gesetzt noch eine andere Identität verknüpft ist
         manage_omniauth: Verknüpfte Accounts verwalten
         remove_identity: Account-Verknüpfung zu %{kind} entfernen
     shared:
diff --git a/config/locales/en/controllers/users.yml b/config/locales/en/controllers/users.yml
index 5cb1c82f5..4529aa630 100644
--- a/config/locales/en/controllers/users.yml
+++ b/config/locales/en/controllers/users.yml
@@ -3,5 +3,6 @@ en:
   users:
     omni_auth:
       failure_deauthorize: Could not remove authorization from %{kind} because "%{reason}".
+      failure_deauthorize_last_identity: You cannot deauthorize your %{kind} account because it is your last remaining account. Please create a password to log in in the future.
       failure_deauthorize_not_linked: You are not linked to any %{kind} account.
       success_deauthorize: Successfully removed authorization from %{kind} account.
diff --git a/config/locales/en/views/users.yml b/config/locales/en/views/users.yml
index 5c4955034..debef15fb 100644
--- a/config/locales/en/views/users.yml
+++ b/config/locales/en/views/users.yml
@@ -9,6 +9,7 @@ en:
         remove_avatar: Remove Avatar
       edit:
         add_identity: Link %{kind} account
+        cannot_remove_last_identity: Cannot remove account link to %{kind} because neither a password is set nor another identity is linked
         manage_omniauth: Manage linked accounts
         remove_identity: Remove account link to %{kind}
     shared:
diff --git a/spec/controllers/users/omniauth_callbacks_controller_spec.rb b/spec/controllers/users/omniauth_callbacks_controller_spec.rb
index c82ce6fda..d53d0c365 100644
--- a/spec/controllers/users/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/users/omniauth_callbacks_controller_spec.rb
@@ -176,7 +176,7 @@
     let(:user) { create(:user) }
     let(:identity) { create(:user_identity, user:, omniauth_provider:) }
 
-    context 'when identity exists' do
+    context 'when password is set and identity exists' do
       before do
         allow(controller).to receive(:current_user).and_return(user)
         allow(user.identities).to receive(:find_by).with(omniauth_provider:).and_return(identity)
@@ -226,7 +226,7 @@
       it_behaves_like 'no addition of the provider to the session'
     end
 
-    context 'when identity does not exist' do
+    context 'when password is set but identity does not exist' do
       before do
         allow(controller).to receive(:current_user).and_return(user)
         allow(user.identities).to receive(:find_by).with(omniauth_provider:).and_return(nil)
@@ -252,7 +252,7 @@
       it_behaves_like 'no addition of the provider to the session'
     end
 
-    context 'when identity cannot be deleted' do
+    context 'when password exists but identity cannot be deleted' do
       let(:user_identity) { build(:user_identity, user:, omniauth_provider:, provider_uid: nil) }
 
       before do
@@ -277,5 +277,32 @@
       it_behaves_like 'no change to to other providers in the session'
       it_behaves_like 'no addition of the provider to the session'
     end
+
+    context 'when password is not set and last identity is deleted' do
+      before do
+        user.update(password_set: false)
+        allow(controller).to receive(:current_user).and_return(user)
+        allow(user.identities).to receive(:find_by).with(omniauth_provider:).and_return(identity)
+      end
+
+      it 'does not destroy the identity' do
+        expect(identity).not_to receive(:destroy)
+        delete :deauthorize, params: {provider: omniauth_provider}
+      end
+
+      it 'redirects to edit user registration path' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(response).to redirect_to edit_user_registration_path
+      end
+
+      it 'sets a flash message' do
+        delete :deauthorize, params: {provider: omniauth_provider}
+        expect(flash[:alert]).to eq I18n.t('users.omni_auth.failure_deauthorize_last_identity', kind: OmniAuth::Utils.camelize(omniauth_provider))
+      end
+
+      it_behaves_like 'no removal of affected provider in the session'
+      it_behaves_like 'no change to to other providers in the session'
+      it_behaves_like 'no addition of the provider to the session'
+    end
   end
 end
diff --git a/spec/controllers/users/registrations_controller_spec.rb b/spec/controllers/users/registrations_controller_spec.rb
index bda86fc91..05bada0ae 100644
--- a/spec/controllers/users/registrations_controller_spec.rb
+++ b/spec/controllers/users/registrations_controller_spec.rb
@@ -13,6 +13,99 @@
     request.env['devise.mapping'] = Devise.mappings[:user]
   end
 
+  describe '#edit' do
+    before { sign_in user }
+
+    it 'renders the edit template' do
+      get :edit
+      expect(response).to render_template(:edit)
+    end
+
+    it 'does not offer to manage OmniAuth accounts' do
+      get :edit
+      expect(response.body).not_to include(I18n.t('users.registrations.edit.manage_omniauth'))
+    end
+
+    context 'with an available identity provider' do
+      let(:provider) { :provider }
+
+      before do
+        # Add a new provider to the omniauth config, reload routes, and make the URL helpers available
+        Devise.omniauth provider
+        Rails.application.reload_routes!
+        Devise.include_helpers(Devise::OmniAuth)
+      end
+
+      after do
+        # Remove the provider from the omniauth config, reload routes
+        Devise.class_variable_set(:@@omniauth_configs, {}) # rubocop:disable Style/ClassVars
+        Rails.application.reload_routes!
+        # The URL helpers cannot be removed, but this only affects methods from Devise::OmniAuth::UrlHelpers.
+        # Those methods won't be called successfully without the provider in the omniauth config, so that this is fine.
+      end
+
+      it 'offers to manage OmniAuth accounts' do
+        get :edit
+        expect(response.body).to include(I18n.t('users.registrations.edit.manage_omniauth', kind: OmniAuth::Utils.camelize(provider)))
+      end
+
+      context 'when no identity is linked' do
+        it 'does not offer to unlink the identity' do
+          get :edit
+          expect(response.body).not_to include(I18n.t('users.registrations.edit.remove_identity', kind: OmniAuth::Utils.camelize(provider)))
+        end
+
+        it 'offers to add the identity' do
+          get :edit
+          expect(response.body).to include(I18n.t('users.registrations.edit.add_identity', kind: OmniAuth::Utils.camelize(provider)))
+        end
+
+        it 'does not explain that the last identity cannot be removed' do
+          get :edit
+          expect(response.body).not_to include(I18n.t('users.registrations.edit.cannot_remove_last_identity', kind: OmniAuth::Utils.camelize(provider)))
+        end
+      end
+
+      context 'when an existing identity is linked' do
+        before { create(:user_identity, user:, omniauth_provider: provider) }
+
+        it 'offers to unlink the identity' do
+          get :edit
+          expect(response.body).to include(I18n.t('users.registrations.edit.remove_identity', kind: OmniAuth::Utils.camelize(provider)))
+        end
+
+        it 'does not offer to add the identity' do
+          get :edit
+          expect(response.body).not_to include(I18n.t('users.registrations.edit.add_identity', kind: OmniAuth::Utils.camelize(provider)))
+        end
+
+        it 'does not explain that the last identity cannot be removed' do
+          get :edit
+          expect(response.body).not_to include(I18n.t('users.registrations.edit.cannot_remove_last_identity', kind: OmniAuth::Utils.camelize(provider)))
+        end
+
+        context 'when no password is set' do
+          before { user.update(password_set: false) }
+
+          it 'does not offer to unlink the identity' do
+            get :edit
+            expect(response.body).not_to include(I18n.t('users.registrations.edit.remove_identity', kind: OmniAuth::Utils.camelize(provider)))
+          end
+
+          it 'does not offer to add the identity' do
+            get :edit
+            expect(response.body).not_to include(I18n.t('users.registrations.edit.add_identity', kind: OmniAuth::Utils.camelize(provider)))
+          end
+
+          it 'explains that the last identity cannot be removed' do
+            get :edit
+            expect(response.body).to include(I18n.t('users.registrations.edit.cannot_remove_last_identity', kind: OmniAuth::Utils.camelize(provider)))
+          end
+        end
+      end
+    end
+  end
+
   describe '#update' do
     context 'when avatar was removed' do
       before { user.avatar.attach(io: Rails.root.join('spec/fixtures/files/avatar/profile.png').open, filename: 'profile.png') }

From a74ba28683c112093585fb817163dfd8bc116ea6 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <MrSerth@users.noreply.github.com>
Date: Tue, 2 Apr 2024 20:39:51 +0200
Subject: [PATCH 4/4] Add specs for OmniAuth::Strategies (#1318)

---
 config/settings/test.yml                      |   4 +
 lib/omni_auth/strategies/abstract_saml.rb     |   3 +-
 .../strategies/abstract_saml_spec.rb          | 193 ++++++++++++++++++
 spec/lib/omni_auth/strategies/bird_spec.rb    |  27 +++
 .../omni_auth/strategies/mock_saml_spec.rb    |  69 +++++++
 spec/lib/omni_auth/strategies/nbp_spec.rb     |  27 +++
 6 files changed, 322 insertions(+), 1 deletion(-)
 create mode 100644 spec/lib/omni_auth/strategies/abstract_saml_spec.rb
 create mode 100644 spec/lib/omni_auth/strategies/bird_spec.rb
 create mode 100644 spec/lib/omni_auth/strategies/mock_saml_spec.rb
 create mode 100644 spec/lib/omni_auth/strategies/nbp_spec.rb

diff --git a/config/settings/test.yml b/config/settings/test.yml
index fa1c6c5df..e58f8e258 100644
--- a/config/settings/test.yml
+++ b/config/settings/test.yml
@@ -5,7 +5,11 @@ omniauth:
     enable: false
   bird:
     enable: false
+    certificate: ~
+    private_key: ~
   nbp:
     enable: false
+    certificate: ~
+    private_key: ~
 oai_pmh:
   admin_mail: admin@example.org
diff --git a/lib/omni_auth/strategies/abstract_saml.rb b/lib/omni_auth/strategies/abstract_saml.rb
index b20cbada0..b27b70adb 100644
--- a/lib/omni_auth/strategies/abstract_saml.rb
+++ b/lib/omni_auth/strategies/abstract_saml.rb
@@ -9,8 +9,9 @@ module Strategies
     class AbstractSaml < OmniAuth::Strategies::SAML
       option :attribute_service_name, 'CodeHarbor'
 
-      # We don't request any specific attributes to get all automatically.
+      # We don't request any specific attributes (statements) to get all automatically.
       option :request_attributes, {}
+      option :attribute_statements, {}
 
       # We want to specify some security options ourselves
       option :security, {
diff --git a/spec/lib/omni_auth/strategies/abstract_saml_spec.rb b/spec/lib/omni_auth/strategies/abstract_saml_spec.rb
new file mode 100644
index 000000000..78b07fcbb
--- /dev/null
+++ b/spec/lib/omni_auth/strategies/abstract_saml_spec.rb
@@ -0,0 +1,193 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe OmniAuth::Strategies::AbstractSaml do
+  let(:strategy) { described_class.new(nil) }
+  let(:env) { {} }
+  let(:session) { {} }
+
+  describe '#idp_slo_session_destroy' do
+    before do
+      env['warden'] = instance_double(Warden::Proxy).as_null_object
+      env['rack.session'] = instance_double(ActionDispatch::Request::Session, options: {}).as_null_object
+      session[:user_id] = 1
+    end
+
+    it 'clears the session' do
+      strategy.options[:idp_slo_session_destroy].call(env, session)
+      expect(session).to be_empty
+    end
+
+    it 'sets secure session option to true' do
+      strategy.options[:idp_slo_session_destroy].call(env, session)
+      expect(env['rack.session'].options[:secure]).to be true
+    end
+
+    it 'sets same_site session option to none' do
+      strategy.options[:idp_slo_session_destroy].call(env, session)
+      expect(env['rack.session'].options[:same_site]).to eq(:none)
+    end
+
+    it 'calls logout on warden' do
+      expect(env['warden']).to receive(:logout)
+      strategy.options[:idp_slo_session_destroy].call(env, session)
+    end
+
+    context 'when in development environment' do
+      before do
+        allow(Rails.env).to receive(:development?).and_return(true)
+        strategy.options[:idp_slo_session_destroy].call(env, session)
+      end
+
+      it 'sets HTTP_X_FORWARDED_PROTO to https' do
+        expect(env['HTTP_X_FORWARDED_PROTO']).to eq('https')
+      end
+    end
+  end
+
+  describe '#info' do
+    before do
+      strategy.instance_variable_set(:@attributes, {
+        'urn:oid:0.9.2342.19200300.100.1.3' => 'email@example.com',
+        'urn:oid:2.5.4.3' => 'name',
+        'urn:oid:2.5.4.42' => 'first_name',
+        'urn:oid:2.5.4.4' => 'last_name',
+        'urn:oid:2.16.840.1.113730.3.1.241' => 'display_name',
+      })
+    end
+
+    it 'returns the correct info hash' do
+      expect(strategy.info).to eq({
+        email: 'email@example.com',
+        name: 'name',
+        first_name: 'first_name',
+        last_name: 'last_name',
+        display_name: 'display_name',
+      })
+    end
+  end
+
+  describe '#uid' do
+    context 'when name_id is present' do
+      before do
+        strategy.instance_variable_set(:@name_id, 'name_id')
+      end
+
+      it 'returns the name_id' do
+        expect(strategy.uid).to eq('name_id')
+      end
+    end
+
+    context 'when name_id is not present' do
+      before do
+        strategy.instance_variable_set(:@attributes, {
+          'urn:oid:0.9.2342.19200300.100.1.1' => 'uid',
+        })
+      end
+
+      it 'returns the uid from attributes' do
+        expect(strategy.uid).to eq('uid')
+      end
+    end
+  end
+
+  describe '#with_settings' do
+    let(:current_user) { create(:user) }
+    let(:mock_request) { instance_double(ActionDispatch::Request) }
+    let(:test_proc) { ->(first_arg, *_other_args) { first_arg } }
+    let(:on_request_path?) { true }
+
+    before do
+      allow(strategy).to receive_messages(full_host: 'https://example.com', script_name: '', request_path: '/users/auth/provider', current_user:, on_request_path?: on_request_path?, request: mock_request)
+      allow(mock_request).to receive_messages(params: {}, query_string: '')
+      allow(OmniAuth::NonceStore).to receive(:add).with(current_user&.id).and_return('nonce')
+      strategy.with_settings(&test_proc)
+    end
+
+    it 'sets name_identifier_format option to persistent' do
+      expect(strategy.options[:name_identifier_format]).to eq('urn:oasis:names:tc:SAML:2.0:nameid-format:persistent')
+    end
+
+    it 'sets sp_entity_id option to sso_path' do
+      expect(strategy.options[:sp_entity_id]).to eq(strategy.sso_path)
+    end
+
+    it 'sets single_logout_service_url option to slo_path' do
+      expect(strategy.options[:single_logout_service_url]).to eq(strategy.slo_path)
+    end
+
+    it 'sets slo_default_relay_state option to full_host' do
+      expect(strategy.options[:slo_default_relay_state]).to eq(strategy.full_host)
+    end
+
+    it 'yields the block and returns a Settings object' do
+      expect(strategy.with_settings(&test_proc)).to be_an_instance_of(OneLogin::RubySaml::Settings)
+    end
+
+    context 'when on request path and current user exists' do
+      it 'sets relay_state param to nonce' do
+        expect(strategy.request.params['relay_state']).to eq('nonce')
+      end
+    end
+
+    context 'when not on request path' do
+      let(:on_request_path?) { false }
+
+      it 'does not set relay_state param' do
+        expect(strategy.request.params['relay_state']).to be_nil
+      end
+    end
+
+    context 'when current user does not exist' do
+      let(:current_user) { nil }
+
+      it 'does not set relay_state param' do
+        expect(strategy.request.params['relay_state']).to be_nil
+      end
+    end
+  end
+
+  describe '#sso_path' do
+    before do
+      allow(strategy).to receive_messages(full_host: 'https://example.com', script_name: '/auth', request_path: '/saml')
+    end
+
+    it 'returns the correct sso path' do
+      expect(strategy.sso_path).to eq('https://example.com/auth/saml')
+    end
+  end
+
+  describe '#slo_path' do
+    before do
+      allow(strategy).to receive_messages(full_host: 'https://example.com', script_name: '/auth', request_path: '/saml')
+    end
+
+    it 'returns the correct slo path' do
+      expect(strategy.slo_path).to eq('https://example.com/auth/saml/slo')
+    end
+  end
+
+  describe '#current_user' do
+    let(:warden) { instance_double(Warden::Proxy) }
+    let(:user) { instance_double(User) }
+
+    before do
+      allow(strategy).to receive(:env).and_return({'warden' => warden})
+      allow(warden).to receive(:user).and_return(user)
+    end
+
+    it 'returns the current user' do
+      expect(strategy.current_user).to eq(user)
+    end
+  end
+
+  describe '.desired_bindings' do
+    it 'returns the correct desired bindings' do
+      expect(described_class.desired_bindings).to eq({
+        sso_binding: 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
+        slo_binding: 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
+      })
+    end
+  end
+end
diff --git a/spec/lib/omni_auth/strategies/bird_spec.rb b/spec/lib/omni_auth/strategies/bird_spec.rb
new file mode 100644
index 000000000..7e9e14072
--- /dev/null
+++ b/spec/lib/omni_auth/strategies/bird_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# Since the OmniAuth::Strategies::Bird class is only loaded if enabled, we need some workaround to test it.
+# Once enabled in the before block, we require the file to load the class.
+# Then, we test the class as usual.
+RSpec.describe 'OmniAuth::Strategies::Bird' do
+  let(:described_class) { OmniAuth::Strategies::Bird }
+
+  let(:strategy) { described_class.new(nil) }
+  let(:idp_metadata_parser) { instance_double(OneLogin::RubySaml::IdpMetadataParser) }
+  let(:idp_metadata) { {issuer: 'https://example.com', idp_sso_target_url: 'https://example.com/login'} }
+
+  before do
+    allow(Settings.omniauth.bird).to receive_messages(enable: true, certificate: 'certificate path', private_key: 'private key path')
+    allow(File).to receive(:read).and_return('certificate content', 'private key content')
+    allow(OneLogin::RubySaml::IdpMetadataParser).to receive(:new).and_return(idp_metadata_parser)
+    allow(idp_metadata_parser).to receive(:parse_remote_to_hash).and_return(idp_metadata)
+    require 'omni_auth/strategies/bird'
+  end
+
+  it 'configures the strategy with the idp metadata' do
+    expect(strategy.options.issuer).to eq('https://example.com')
+    expect(strategy.options.idp_sso_target_url).to eq('https://example.com/login')
+  end
+end
diff --git a/spec/lib/omni_auth/strategies/mock_saml_spec.rb b/spec/lib/omni_auth/strategies/mock_saml_spec.rb
new file mode 100644
index 000000000..61b6943d3
--- /dev/null
+++ b/spec/lib/omni_auth/strategies/mock_saml_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# Since the OmniAuth::Strategies::MockSaml class is only loaded if enabled, we need some workaround to test it.
+# Once enabled in the before block, we require the file to load the class.
+# Then, we test the class as usual.
+RSpec.describe 'OmniAuth::Strategies::MockSaml' do
+  let(:described_class) { OmniAuth::Strategies::MockSaml }
+
+  let(:strategy) { described_class.new(nil) }
+  let(:idp_metadata_parser) { instance_double(OneLogin::RubySaml::IdpMetadataParser) }
+  let(:idp_metadata) { {issuer: 'https://example.com', idp_sso_target_url: 'https://example.com/login'} }
+
+  before do
+    allow(Settings.omniauth.mocksaml).to receive(:enable).and_return(true)
+    allow(OneLogin::RubySaml::IdpMetadataParser).to receive(:new).and_return(idp_metadata_parser)
+    allow(idp_metadata_parser).to receive(:parse_remote_to_hash).and_return(idp_metadata)
+    require 'omni_auth/strategies/mock_saml'
+  end
+
+  it 'configures the strategy with the idp metadata' do
+    expect(strategy.options.issuer).to eq('https://example.com')
+    expect(strategy.options.idp_sso_target_url).to eq('https://example.com/login')
+  end
+
+  context 'when uid is present' do
+    before do
+      strategy.instance_variable_set(:@attributes, {
+        'id' => '123',
+      })
+    end
+
+    it 'returns the uid from the attributes' do
+      expect(strategy.uid).to eq('123')
+    end
+  end
+
+  context 'when uid is not present' do
+    before do
+      strategy.instance_variable_set(:@attributes, {})
+      strategy.instance_variable_set(:@name_id, 'abc')
+    end
+
+    it 'returns the name_id as the uid' do
+      expect(strategy.uid).to eq('abc')
+    end
+  end
+
+  context 'when user info is present' do
+    before do
+      strategy.instance_variable_set(:@attributes, {
+        'email' => 'test@example.com',
+        'firstName' => 'John',
+        'lastName' => 'Doe',
+      })
+    end
+
+    it 'returns the correct user info' do
+      expect(strategy.info).to eq({
+        email: 'test@example.com',
+        name: 'John Doe',
+        first_name: 'John',
+        last_name: 'Doe',
+        display_name: 'John',
+      })
+    end
+  end
+end
diff --git a/spec/lib/omni_auth/strategies/nbp_spec.rb b/spec/lib/omni_auth/strategies/nbp_spec.rb
new file mode 100644
index 000000000..e32c9d522
--- /dev/null
+++ b/spec/lib/omni_auth/strategies/nbp_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# Since the OmniAuth::Strategies::Nbp class is only loaded if enabled, we need some workaround to test it.
+# Once enabled in the before block, we require the file to load the class.
+# Then, we test the class as usual.
+RSpec.describe 'OmniAuth::Strategies::Nbp' do
+  let(:described_class) { OmniAuth::Strategies::Nbp }
+
+  let(:strategy) { described_class.new(nil) }
+  let(:idp_metadata_parser) { instance_double(OneLogin::RubySaml::IdpMetadataParser) }
+  let(:idp_metadata) { {issuer: 'https://example.com', idp_sso_target_url: 'https://example.com/login'} }
+
+  before do
+    allow(Settings.omniauth.nbp).to receive_messages(enable: true, certificate: 'certificate path', private_key: 'private key path')
+    allow(File).to receive(:read).and_return('certificate content', 'private key content')
+    allow(OneLogin::RubySaml::IdpMetadataParser).to receive(:new).and_return(idp_metadata_parser)
+    allow(idp_metadata_parser).to receive(:parse_remote_to_hash).and_return(idp_metadata)
+    require 'omni_auth/strategies/nbp'
+  end
+
+  it 'configures the strategy with the idp metadata' do
+    expect(strategy.options.issuer).to eq('https://example.com')
+    expect(strategy.options.idp_sso_target_url).to eq('https://example.com/login')
+  end
+end