Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow adding and removing OmniAuth account links #1316

Merged
merged 4 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'omni_auth/nonce_store'

module Users
class OmniauthCallbacksController < Devise::OmniauthCallbacksController
skip_before_action :require_user!
Expand All @@ -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
Expand All @@ -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)
kkoehn marked this conversation as resolved.
Show resolved Hide resolved
# 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)
kkoehn marked this conversation as resolved.
Show resolved Hide resolved
# 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:
Expand Down
10 changes: 7 additions & 3 deletions app/controllers/users/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 32 additions & 0 deletions app/controllers/users/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
42 changes: 27 additions & 15 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
kkoehn marked this conversation as resolved.
Show resolved Hide resolved
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

Expand Down
34 changes: 26 additions & 8 deletions app/views/users/registrations/edit.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
8 changes: 8 additions & 0 deletions config/locales/de/controllers/users.yml
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions config/locales/de/models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ de:
user:
first_name: Vorname
last_name: Nachname
password_set: Passwort gesetzt
role: Rolle
user_identity:
omniauth_provider: OmniAuth-Anbieter
Expand Down
5 changes: 5 additions & 0 deletions config/locales/de/views/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions config/locales/en/controllers/users.yml
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions config/locales/en/models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions config/locales/en/views/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions config/settings/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
7 changes: 7 additions & 0 deletions db/migrate/20240329215957_add_password_set_to_users.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions lib/omni_auth/nonce_store.rb
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading