diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 1a16883b4..ee759fb74 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,43 @@ 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, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + identity = current_user.identities.find_by(omniauth_provider: provider) + 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') + 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/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 575a34551..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 @@ -57,23 +58,34 @@ 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.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. + 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..579e1cde5 100644 --- a/app/views/users/registrations/edit.html.slim +++ b/app/views/users/registrations/edit.html.slim @@ -50,14 +50,32 @@ = 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') + + + - 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) + - 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 .form-group.py-3 .btn-group[role='group'] diff --git a/config/locales/de/controllers/users.yml b/config/locales/de/controllers/users.yml new file mode 100644 index 000000000..5095d7843 --- /dev/null +++ b/config/locales/de/controllers/users.yml @@ -0,0 +1,8 @@ +--- +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/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/de/views/users.yml b/config/locales/de/views/users.yml index a834a5e1c..081e5f1ed 100644 --- a/config/locales/de/views/users.yml +++ b/config/locales/de/views/users.yml @@ -7,6 +7,11 @@ de: registrations: avatar_form: 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: 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..4529aa630 --- /dev/null +++ b/config/locales/en/controllers/users.yml @@ -0,0 +1,8 @@ +--- +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/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/config/locales/en/views/users.yml b/config/locales/en/views/users.yml index d4306a62b..debef15fb 100644 --- a/config/locales/en/views/users.yml +++ b/config/locales/en/views/users.yml @@ -7,6 +7,11 @@ en: registrations: avatar_form: 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: 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/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/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/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..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, { @@ -24,13 +25,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 +72,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 +98,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..d53d0c365 --- /dev/null +++ b/spec/controllers/users/omniauth_callbacks_controller_spec.rb @@ -0,0 +1,308 @@ +# 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 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) + 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 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) + 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 password exists but 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 + + 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/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..05bada0ae --- /dev/null +++ b/spec/controllers/users/registrations_controller_spec.rb @@ -0,0 +1,250 @@ +# 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 '#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') } + + 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 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/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 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