diff --git a/Gemfile b/Gemfile index c9678d58eb34c8551601e31f5e9d6b9b6a02d2ff..091adfff6bb319247b24736322e9757f745ba007 100644 --- a/Gemfile +++ b/Gemfile @@ -33,7 +33,8 @@ gem 'kamifusen'#, path: '../kamifusen' gem 'kaminari' gem 'mini_magick' gem 'octokit' -gem 'omniauth-saml' +gem 'omniauth-rails_csrf_protection', '~> 1.0' +gem 'omniauth-saml', '~> 2.0' gem 'pg', '~> 1.1' gem 'puma' gem 'rails', '~> 6.1' diff --git a/Gemfile.lock b/Gemfile.lock index 7a8d96bb71c06f190cf877abac06ad6408937b8b..d75a15fd9f09afa158fcfefae57a0dbb3c21a274 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -302,18 +302,22 @@ GEM ruby2_keywords (~> 0.0.1) nesty (1.0.2) nio4r (2.5.8) - nokogiri (1.13.4) + nokogiri (1.13.5) mini_portile2 (~> 2.8.0) racc (~> 1.4) octokit (4.22.0) faraday (>= 0.9) sawyer (~> 0.8.0, >= 0.5.3) - omniauth (1.9.1) + omniauth (2.1.0) hashie (>= 3.4.6) - rack (>= 1.6.2, < 3) - omniauth-saml (1.10.3) - omniauth (~> 1.3, >= 1.3.2) - ruby-saml (~> 1.9) + rack (>= 2.2.3) + rack-protection + omniauth-rails_csrf_protection (1.0.1) + actionpack (>= 4.2) + omniauth (~> 2.0) + omniauth-saml (2.1.0) + omniauth (~> 2.0) + ruby-saml (~> 1.12) orm_adapter (0.5.0) pg (1.3.5) popper_js (2.9.3) @@ -368,7 +372,7 @@ GEM railties (>= 5.0) rexml (3.2.5) rotp (6.2.0) - ruby-saml (1.13.0) + ruby-saml (1.14.0) nokogiri (>= 1.10.5) rexml ruby-vips (2.1.4) @@ -499,7 +503,8 @@ DEPENDENCIES listen (~> 3.3) mini_magick octokit - omniauth-saml + omniauth-rails_csrf_protection (~> 1.0) + omniauth-saml (~> 2.0) pg (~> 1.1) puma rack-mini-profiler (~> 2.0) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index be8f1f99dfc7cd5b53c57a3c380f90d5ab0fdea8..47b36fc4d6bb9823d4f47ddda8486fe7f53d70e1 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -32,12 +32,12 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController def manage_user(user_infos) @user = User.from_omniauth(current_university, user_infos) - + if @user&.persisted? @user.remember_me = true sign_in_and_redirect @user, event: :authentication else - flash[:notice] = tt('devise.omniauth_callbacks.failure') + flash[:notice] = t('devise.omniauth_callbacks.failure') redirect_to new_user_session_url end end diff --git a/app/models/university/with_sso.rb b/app/models/university/with_sso.rb index 08dec1a53fc213d78f7a0515ba35688a90cd5e20..91ef0d463e0143666a8bdaf6deff7b936dc8b30e 100644 --- a/app/models/university/with_sso.rb +++ b/app/models/university/with_sso.rb @@ -5,7 +5,7 @@ module University::WithSso enum sso_provider: { saml: 0 }, _prefix: :with_sso_via validates :sso_cert, :sso_name_identifier_format, :sso_target_url, presence: true, if: :has_sso? - + validate :sso_mapping_should_have_email, if: :has_sso? end # Setter to serialize data as JSON @@ -18,4 +18,7 @@ module University::WithSso super(value) end + def sso_mapping_should_have_email + errors.add(:sso_mapping, :missing_email) unless (sso_mapping || []).detect { |sso_item| sso_item['internal_key'] == 'email' } + end end diff --git a/app/models/user/with_omniauth.rb b/app/models/user/with_omniauth.rb index 24d8361368f2e7c1cdceb2bdd808f89df7b45fa1..f4a6e3fd15189a2ec28d3acf067fdf8f13ce4627 100644 --- a/app/models/user/with_omniauth.rb +++ b/app/models/user/with_omniauth.rb @@ -4,16 +4,13 @@ module User::WithOmniauth included do def self.from_omniauth(university, attributes) - mapping = university.sso_mapping + mapping = university.sso_mapping || [] # first step: we find the email (we are supposed to have an email mapping) - email_sso_key = mapping.select { |elmt| elmt['internal_key'] == 'email' }&.first&.dig('sso_key') - email = attributes.dig(email_sso_key) + email = get_email_from_mapping(mapping, attributes) return unless email - email = email.first if email.is_a?(Array) - email = email.downcase - user = User.where(university: university, email: email).first_or_create do |u| + user = User.where(university: university, email: email.downcase).first_or_create do |u| u.password = "#{Devise.friendly_token[0,20]}!" # meets password complexity requirements end @@ -28,6 +25,13 @@ module User::WithOmniauth protected + def self.get_email_from_mapping(mapping, attributes) + email_sso_key = mapping.detect { |elmt| elmt['internal_key'] == 'email' }&.dig('sso_key') + email = attributes.dig(email_sso_key) + email = email.first if email.is_a?(Array) + email + end + def self.update_data_for_mapping_element(user, mapping_element, attributes) sso_key = mapping_element['sso_key'] return user if attributes[sso_key].nil? # if not provided by sso, just return diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index f22889e4730e48e6ae0633f5f47641eb51bf69c3..d1a68f946cbd72e5b546544b3566553940e47928 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -11,7 +11,7 @@ <div class="col-md-6"> <h2 class="mb-4"><%= t('login.already_registered') %></h2> <% if current_university.has_sso? %> - <p><%= link_to t('login.sign_in_with_sso'), omniauth_authorize_path(resource_name, current_university.sso_provider), class: 'btn btn-primary' %></p> + <p><%= link_to t('login.sign_in_with_sso'), omniauth_authorize_path(resource_name, current_university.sso_provider), method: :post, class: 'btn btn-primary' %></p> <p><%= t('login.or') %></p> <a href="#collapseLoginForm" class="btn btn-primary mb-3" data-bs-toggle="collapse"><%= t('login.sign_in_with_credentials') %></a> <% end %> diff --git a/app/views/server/universities/_form.html.erb b/app/views/server/universities/_form.html.erb index 5ef1508748f0d071e6782559b6453803da92fbbb..480d423f6eb1fe44660534be8d31013f30f6aca7 100644 --- a/app/views/server/universities/_form.html.erb +++ b/app/views/server/universities/_form.html.erb @@ -1,4 +1,6 @@ <%= simple_form_for [:server, university] do |f| %> + <%= f.error_notification %> + <div class="row"> <div class="col-md-4"> <%= f.input :name %> @@ -40,6 +42,7 @@ </div> <div class="col-md-6"> <h4 class="mb-4"><%= University.human_attribute_name('sso_mapping') %></h4> + <%= f.error_notification message: f.object.errors[:sso_mapping].to_sentence if f.object.errors[:sso_mapping].present? %> <%= render 'sso_mapping', university: university %> </div> </div> diff --git a/config/locales/en.yml b/config/locales/en.yml index 49071105af0eb9cde4fe42461b5a176d54940a54..413c3e8cf60e353600c964f9647c803282cc5671 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -74,6 +74,8 @@ en: two_factor_authentication_code: subject: "Two-factor authentication code" text_html: "Your two-factor authentication code for %{university} is: <br><br><b>%{code}</b><br><br>It will expire in 5 minutes." + omniauth_callbacks: + failure: "Failed to sign in." sessions: signed_in: '' shared: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index ab257fa73827bc04141c59c0edb70995dd5fcf76..1e8778ab47e657036fd405f8e669a9cf92f14fe9 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -74,6 +74,8 @@ fr: two_factor_authentication_code: subject: "Code d'authentification à deux facteurs" text_html: "Votre code d'authentification pour %{university} est :<br><br><b>%{code}</b><br><br>Il expirera dans 5 minutes." + omniauth_callbacks: + failure: "Échec de l'authentification." sessions: signed_in: '' shared: diff --git a/config/locales/university/en.yml b/config/locales/university/en.yml index 2e9ea4d69c0ae389d611c9f297dfe4f64dfc5fcd..821184b444454959fbb0fc717e722cf5888a6c48 100644 --- a/config/locales/university/en.yml +++ b/config/locales/university/en.yml @@ -81,6 +81,12 @@ en: university/role: description: Description people: People + errors: + models: + university: + attributes: + sso_mapping: + missing_email: doesn't handle the email models: university: one: University diff --git a/config/locales/university/fr.yml b/config/locales/university/fr.yml index bd2215aba23d0f3d6f36c0be7c102f408c688426..11e6e050ef25099db72514b2bfc904e0fe9ba2e5 100644 --- a/config/locales/university/fr.yml +++ b/config/locales/university/fr.yml @@ -81,6 +81,12 @@ fr: university/role: description: Description people: Personnes + errors: + models: + university: + attributes: + sso_mapping: + missing_email: ne gère pas l'adresse email models: university: one: Université