Allow adding and removing OmniAuth account links (#1316)

* Allow adding and removing OmniAuth account links

* 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.

* Prevent deletion of last OmniAuth identity

* Add specs for OmniAuth::Strategies (#1318)

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:

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
 

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

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
 

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']

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.

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

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:

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.

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

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:

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

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: [email protected]

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

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

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