From 08cc5f6a9506848339328e61be1b2a4075f8afd1 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Tue, 25 Jul 2023 10:16:32 -0500 Subject: [PATCH 01/19] refactor #nyulibraries OAuth2 to #shibboleth --- .tool-versions | 2 + Gemfile | 3 +- Gemfile.lock | 11 +--- app/assets/config/manifest.js | 1 + .../users/omniauth_callbacks_controller.rb | 10 +-- app/models/user.rb | 2 +- config/initializers/devise.rb | 2 +- .../omniauth/strategies/shibboleth.rb | 65 +++++++++++++++++++ config/routes.rb | 4 +- spec/routing/user_routing_spec.rb | 20 +++--- 10 files changed, 91 insertions(+), 29 deletions(-) create mode 100644 .tool-versions create mode 100644 config/initializers/omniauth/strategies/shibboleth.rb diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 00000000..dc859079 --- /dev/null +++ b/.tool-versions @@ -0,0 +1,2 @@ +ruby 2.7.8 +nodejs 18.10.0 diff --git a/Gemfile b/Gemfile index f3cbd8e9..69ed6407 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,8 @@ gem 'jbuilder' gem 'jquery-rails' gem 'mimemagic', github: 'mimemagicrb/mimemagic', ref: '01f92d86d15d85cfd0f20dabd025dcbd36a8a60f' gem 'mysql2' -gem 'omniauth-nyulibraries', git: 'https://github.com/NYULibraries/omniauth-nyulibraries', branch: 'v2.2.0' +gem 'omniauth' +gem 'omniauth-oauth2' gem 'rails' gem 'rainbow' gem 'rsolr' diff --git a/Gemfile.lock b/Gemfile.lock index f180c7dd..3b494287 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,11 +1,3 @@ -GIT - remote: https://github.com/NYULibraries/omniauth-nyulibraries - revision: 2a77afefc346669e5a50ff5c84e48a012490dfbc - branch: v2.2.0 - specs: - omniauth-nyulibraries (2.2.0) - omniauth-oauth2 (~> 1.2.0) - GIT remote: https://github.com/mimemagicrb/mimemagic.git revision: 01f92d86d15d85cfd0f20dabd025dcbd36a8a60f @@ -434,7 +426,8 @@ DEPENDENCIES jquery-rails mimemagic! mysql2 - omniauth-nyulibraries! + omniauth + omniauth-oauth2 puma rails rainbow diff --git a/app/assets/config/manifest.js b/app/assets/config/manifest.js index cc6cf74e..0f67efa9 100644 --- a/app/assets/config/manifest.js +++ b/app/assets/config/manifest.js @@ -1,3 +1,4 @@ //= link_tree ../images //= link_tree ../javascripts +//= link_tree ../stylesheets //= link_directory ../stylesheets .css diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 467beab6..15ee6f81 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -2,19 +2,21 @@ module Users class OmniauthCallbacksController < Devise::OmniauthCallbacksController - before_action :require_valid_omniauth, only: :nyulibraries + before_action :require_valid_omniauth, only: :shibboleth - def nyulibraries + def shibboleth set_user if @user.persisted? sign_in_and_redirect @user, event: :authentication - logger.info(find_message(:success, kind: 'NYU Libraries')) + logger.info(find_message(:success, kind: 'NYU Shibboleth')) else session['devise.nyulibraries_data'] = request.env['omniauth.auth'] redirect_to root_path end end + private + def find_user @find_user ||= find_user_with_provider.present? ? find_user_with_provider : find_user_without_provider end @@ -95,8 +97,6 @@ def failure redirect_to root_path end - private - def set_user # Find existing or initialize new user, # and save new attributes each time diff --git a/app/models/user.rb b/app/models/user.rb index ae162b1b..22804246 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,7 +7,7 @@ class User < ActiveRecord::Base # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable - devise :omniauthable, omniauth_providers: [:nyulibraries] + devise :omniauthable, omniauth_providers: [:shibboleth] # Method added by Blacklight; Blacklight uses #to_s on your # user class to get a user-displayable login/identifier for diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 83212a83..c1403871 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -10,7 +10,7 @@ config.password_length = 8..128 config.reset_password_within = 60.minutes config.sign_out_via = :get - config.omniauth :nyulibraries, Settings.APP_ID, Settings.APP_SECRET, client_options: { + config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { site: Settings.LOGIN_URL, authorize_path: '/oauth/authorize' } diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb new file mode 100644 index 00000000..f8a8b38f --- /dev/null +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -0,0 +1,65 @@ +module OmniAuth + module Strategies + require 'omniauth-oauth2' + class Shibboleth < OmniAuth::Strategies::OAuth2 + if defined?(::Rails) && ::Rails.env.development? + silence_warnings do + OpenSSL::SSL::VERIFY_PEER = OpenSSL::SSL::VERIFY_NONE + end + end + option :name, :shibboleth + + option :client_options, { + site: (ENV['LOGIN_URL'] || "https://qa.auth.nyu.edu:443"), + authorize_path: "/oauth2/authorize" + } + + uid do + raw_info["username"] + end + + info do + { + name: raw_info["username"], + nickname: raw_info["username"], + email: raw_info["email"], + last_name: last_name, + first_name: first_name + } + end + extra do + { + provider: raw_info["provider"], + identities: raw_info["identities"], + institution_code: raw_info["institution_code"] + } + end + + def raw_info + response = access_token.get("/api/v1/user") + @raw_info ||= response.parsed + end + + # Pass querystring from client to provider + def authorize_params + querystring_hash = Rack::Utils.parse_nested_query(request.query_string[/institution=(\w+)/,0]) + super.merge(querystring_hash) + end + + # Get the provider's identity + def provider_identity + @provider_identity ||= raw_info["identities"].find {|id| id["provider"] == raw_info["provider"]} + end + + # Extract Last Name from identity + def last_name + @last_name ||= provider_identity["properties"]["last_name"] rescue nil + end + + # Extract First Name from identity + def first_name + @first_name ||= provider_identity["properties"]["first_name"] rescue nil + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 49ea1330..3c5bc147 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,7 +2,7 @@ devise_for :users, controllers: {omniauth_callbacks: 'users/omniauth_callbacks'} devise_scope :user do get 'logout', to: 'devise/sessions#destroy', as: :logout - get 'login', to: redirect { |params, request| "#{Rails.application.config.relative_url_root}/users/auth/nyulibraries?#{request.query_string}" }, as: :login + get 'login', to: redirect { |params, request| "#{Rails.application.config.relative_url_root}/users/auth/shibboleth?#{request.query_string}" }, as: :login resources :suggest, only: :index, defaults: {format: 'json'} @@ -51,4 +51,4 @@ end end end -end \ No newline at end of file +end diff --git a/spec/routing/user_routing_spec.rb b/spec/routing/user_routing_spec.rb index 884a82fc..1e0772f3 100644 --- a/spec/routing/user_routing_spec.rb +++ b/spec/routing/user_routing_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true describe 'routes for users' do - describe 'GET /users/auth/nyulibraries' do - subject { get('/users/auth/nyulibraries') } + describe 'GET /users/auth/shibboleth' do + subject { get('/users/auth/shibboleth') } it do should route_to({ controller: 'users/omniauth_callbacks', @@ -11,8 +11,8 @@ end end - describe 'POST /users/auth/nyulibraries' do - subject { post('/users/auth/nyulibraries') } + describe 'POST /users/auth/shibboleth' do + subject { post('/users/auth/shibboleth') } it do should route_to({ controller: 'users/omniauth_callbacks', @@ -21,13 +21,13 @@ end end - describe 'GET /users/auth/nyulibraries/callback' do - subject { get('/users/auth/nyulibraries/callback') } - it { should route_to({ controller: 'users/omniauth_callbacks', action: 'nyulibraries' }) } + describe 'GET /users/auth/shibboleth/callback' do + subject { get('/users/auth/shibboleth/callback') } + it { should route_to({ controller: 'users/omniauth_callbacks', action: 'shibboleth' }) } end - describe 'POST /users/auth/nyulibraries/callback' do - subject { post('/users/auth/nyulibraries/callback') } - it { should route_to({ controller: 'users/omniauth_callbacks', action: 'nyulibraries' }) } + describe 'POST /users/auth/shibboleth/callback' do + subject { post('/users/auth/shibboleth/callback') } + it { should route_to({ controller: 'users/omniauth_callbacks', action: 'shibboleth' }) } end end From 6c22d7635711e2d5ba879704cfdc99a38d618dae Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 15:24:54 -0500 Subject: [PATCH 02/19] refactor shibboleth strategy --- .../omniauth/strategies/shibboleth.rb | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index f8a8b38f..4cb69aee 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -15,14 +15,12 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 } uid do - raw_info["username"] + raw_info["sub"] end info do { - name: raw_info["username"], - nickname: raw_info["username"], - email: raw_info["email"], + email: raw_info["sub"], last_name: last_name, first_name: first_name } @@ -36,29 +34,18 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end def raw_info - response = access_token.get("/api/v1/user") + response = access_token.get("/oauth2/userinfo?schema=openid") @raw_info ||= response.parsed end - # Pass querystring from client to provider - def authorize_params - querystring_hash = Rack::Utils.parse_nested_query(request.query_string[/institution=(\w+)/,0]) - super.merge(querystring_hash) - end - - # Get the provider's identity - def provider_identity - @provider_identity ||= raw_info["identities"].find {|id| id["provider"] == raw_info["provider"]} - end - # Extract Last Name from identity def last_name - @last_name ||= provider_identity["properties"]["last_name"] rescue nil + @last_name ||= raw_info["lastname"] rescue nil end # Extract First Name from identity def first_name - @first_name ||= provider_identity["properties"]["first_name"] rescue nil + @first_name ||= raw_info["firstname"] rescue nil end end end From 175df5197d66496651c56c49c9f8354fd1b8fdad Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 15:46:02 -0500 Subject: [PATCH 03/19] fix OAuth controller and refactor shibboleth config to pull attributes that are actually there --- .../omniauth_callbacks_controller.rb | 71 ++++++++++++ .../users/omniauth_callbacks_controller.rb | 107 ------------------ .../omniauth/strategies/shibboleth.rb | 7 -- 3 files changed, 71 insertions(+), 114 deletions(-) create mode 100644 app/controllers/omniauth_callbacks_controller.rb delete mode 100644 app/controllers/users/omniauth_callbacks_controller.rb diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb new file mode 100644 index 00000000..8ed2c5b4 --- /dev/null +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +class OmniauthCallbacksController < Devise::OmniauthCallbacksController + before_action :require_valid_omniauth, only: :shibboleth + + def shibboleth + set_user + if @user.persisted? + sign_in_and_redirect @user, event: :authentication + logger.info(find_message(:success, kind: 'NYU Shibboleth')) + else + session['devise.shibboleth_data'] = request.env['omniauth.auth'] + redirect_to root_path + end + end + + private + + def require_valid_omniauth + head :bad_request unless valid_omniauth? + end + + def valid_omniauth? + omniauth.present? + end + + def omniauth + @omniauth ||= request.env['omniauth.auth'] + end + + def omniauth_provider + @omniauth_provider ||= omniauth.provider + end + + def attributes_from_omniauth + { + provider: 'nyulibraries', + username: omniauth.uid, + email: omniauth_email, + firstname: omniauth_firstname, + lastname: omniauth_lastname, + } + end + + def omniauth_email + @omniauth_email ||= omniauth.info.email + end + + def omniauth_firstname + @omniauth_firstname ||= omniauth.info.first_name + end + + def omniauth_lastname + @omniauth_lastname ||= omniauth.info.last_name + end + + def failure + redirect_to root_path + end + + def set_user + # Find existing or initialize new user, + # and save new attributes each time + @user = find_user + @user.update_attributes(attributes_from_omniauth) + end + + def find_user + @find_user ||= User.find_or_initialize_by(email: omniauth_email) + end +end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb deleted file mode 100644 index 15ee6f81..00000000 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ /dev/null @@ -1,107 +0,0 @@ -# frozen_string_literal: true - -module Users - class OmniauthCallbacksController < Devise::OmniauthCallbacksController - before_action :require_valid_omniauth, only: :shibboleth - - def shibboleth - set_user - if @user.persisted? - sign_in_and_redirect @user, event: :authentication - logger.info(find_message(:success, kind: 'NYU Shibboleth')) - else - session['devise.nyulibraries_data'] = request.env['omniauth.auth'] - redirect_to root_path - end - end - - private - - def find_user - @find_user ||= find_user_with_provider.present? ? find_user_with_provider : find_user_without_provider - end - - def find_user_with_provider - @find_user_with_provider ||= User.where(username: omniauth.uid, provider: omniauth.provider) - end - - def find_user_without_provider - @find_user_without_provider ||= User.where(username: omniauth.uid, provider: '') - end - - def require_valid_omniauth - head :bad_request unless valid_omniauth? - end - - def valid_omniauth? - omniauth.present? && omniauth.provider.to_s == 'nyulibraries' && !omniauth_aleph_identity.blank? - # Only accept users with an Aleph ID, authenticated via nyulibraries - end - - def omniauth - @omniauth ||= request.env['omniauth.auth'] - end - - def omniauth_provider - @omniauth_provider ||= omniauth.provider - end - - def attributes_from_omniauth - { - provider: omniauth_provider, - email: omniauth_email, - firstname: omniauth_firstname, - lastname: omniauth_lastname, - institution_code: omniauth_institution, - aleph_id: omniauth_aleph_id, - patron_status: omniauth_patron_status - } - end - - def omniauth_email - @omniauth_email ||= omniauth.info.email - end - - def omniauth_firstname - @omniauth_firstname ||= omniauth.info.first_name - end - - def omniauth_lastname - @omniauth_lastname ||= omniauth.info.last_name - end - - def omniauth_institution - @omniauth_institution ||= omniauth.extra.institution_code - end - - def omniauth_identities - # byebug - @omniauth_identities ||= omniauth.extra.identities - end - - def omniauth_aleph_identity - @omniauth_aleph_identity ||= omniauth_identities.find do |omniauth_identity| - omniauth_identity.provider == 'aleph' - end - end - - def omniauth_aleph_id - @omniauth_aleph_id ||= omniauth_aleph_identity.uid unless omniauth_aleph_identity.blank? - end - - def omniauth_patron_status - @omniauth_patron_status ||= omniauth_aleph_identity.properties.patron_status unless omniauth_aleph_identity.blank? - end - - def failure - redirect_to root_path - end - - def set_user - # Find existing or initialize new user, - # and save new attributes each time - @user = find_user.first_or_initialize(attributes_from_omniauth) - @user.update_attributes(attributes_from_omniauth) - end - end -end diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index 4cb69aee..84596b8e 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -25,13 +25,6 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 first_name: first_name } end - extra do - { - provider: raw_info["provider"], - identities: raw_info["identities"], - institution_code: raw_info["institution_code"] - } - end def raw_info response = access_token.get("/oauth2/userinfo?schema=openid") From 2507ee02df27e2336dc9277bbc6086c44b1d3836 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 15:52:19 -0500 Subject: [PATCH 04/19] make routes match config with IDM --- app/controllers/omniauth_callbacks_controller.rb | 8 ++++---- config/routes.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 8ed2c5b4..7b79eeaa 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -14,6 +14,10 @@ def shibboleth end end + def failure + redirect_to root_path + end + private def require_valid_omniauth @@ -54,10 +58,6 @@ def omniauth_lastname @omniauth_lastname ||= omniauth.info.last_name end - def failure - redirect_to root_path - end - def set_user # Find existing or initialize new user, # and save new attributes each time diff --git a/config/routes.rb b/config/routes.rb index 3c5bc147..986f0063 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,8 +1,8 @@ Rails.application.routes.draw do - devise_for :users, controllers: {omniauth_callbacks: 'users/omniauth_callbacks'} + devise_for :users, path: '', controllers: {omniauth_callbacks: 'omniauth_callbacks'} devise_scope :user do get 'logout', to: 'devise/sessions#destroy', as: :logout - get 'login', to: redirect { |params, request| "#{Rails.application.config.relative_url_root}/users/auth/shibboleth?#{request.query_string}" }, as: :login + get 'login', to: redirect { |params, request| "#{Rails.application.config.relative_url_root}/auth/shibboleth?#{request.query_string}" }, as: :login resources :suggest, only: :index, defaults: {format: 'json'} From e8aee4fcbb5195662f55cc9eda9a5184b8febbc9 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 16:16:17 -0500 Subject: [PATCH 05/19] use Setting over ENV --- config/initializers/omniauth/strategies/shibboleth.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index 84596b8e..ab7882e6 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -10,7 +10,7 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 option :name, :shibboleth option :client_options, { - site: (ENV['LOGIN_URL'] || "https://qa.auth.nyu.edu:443"), + site: (Settings.LOGIN_URL || "https://qa.auth.nyu.edu:443"), authorize_path: "/oauth2/authorize" } From 879c780172b298915807489f88edc19175d90247 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 16:21:02 -0500 Subject: [PATCH 06/19] you're killing me, smalls --- config/initializers/omniauth/strategies/shibboleth.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index ab7882e6..b1a8ea46 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -11,7 +11,7 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 option :client_options, { site: (Settings.LOGIN_URL || "https://qa.auth.nyu.edu:443"), - authorize_path: "/oauth2/authorize" + authorize_url: "/oauth2/authorize" } uid do From 968bd9d80a21516f758df154c7c4dd0205eb184d Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 16:57:34 -0500 Subject: [PATCH 07/19] can a brother get a log? --- app/controllers/omniauth_callbacks_controller.rb | 1 + config/initializers/omniauth/strategies/shibboleth.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 7b79eeaa..ce246e1d 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -9,6 +9,7 @@ def shibboleth sign_in_and_redirect @user, event: :authentication logger.info(find_message(:success, kind: 'NYU Shibboleth')) else + logger.error(find_message(:failure, kind: 'NYU Shibboleth', reason: 'User not persisted')) session['devise.shibboleth_data'] = request.env['omniauth.auth'] redirect_to root_path end diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index b1a8ea46..d3d2f0cb 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -27,7 +27,9 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end def raw_info + console response = access_token.get("/oauth2/userinfo?schema=openid") + Rails.logger.info("Shibboleth raw_info: #{response.parsed}") @raw_info ||= response.parsed end From d49462bf218a85dd0febad25b47128d635627f5e Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 16:59:48 -0500 Subject: [PATCH 08/19] this is the worst way to debug --- app/controllers/omniauth_callbacks_controller.rb | 1 + config/initializers/omniauth/strategies/shibboleth.rb | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index ce246e1d..f953f2e6 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -16,6 +16,7 @@ def shibboleth end def failure + logger.error("OmniAuth is failing for reasons? #{failure_message}") redirect_to root_path end diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index d3d2f0cb..3fbc3e90 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -27,7 +27,6 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end def raw_info - console response = access_token.get("/oauth2/userinfo?schema=openid") Rails.logger.info("Shibboleth raw_info: #{response.parsed}") @raw_info ||= response.parsed From 134e2b37bde66ce696a0f377cb345c9b66af34a2 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 17:23:24 -0500 Subject: [PATCH 09/19] Devise and OmniAuth colliding --- app/controllers/omniauth_callbacks_controller.rb | 7 ++++--- app/models/user.rb | 9 +++++++++ config/initializers/devise.rb | 5 +++-- config/initializers/omniauth/strategies/shibboleth.rb | 5 ----- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index f953f2e6..ed927fd8 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -7,6 +7,7 @@ def shibboleth set_user if @user.persisted? sign_in_and_redirect @user, event: :authentication + set_flash_message(:notice, :success, kind: 'NYU Shibboleth') logger.info(find_message(:success, kind: 'NYU Shibboleth')) else logger.error(find_message(:failure, kind: 'NYU Shibboleth', reason: 'User not persisted')) @@ -16,7 +17,7 @@ def shibboleth end def failure - logger.error("OmniAuth is failing for reasons? #{failure_message}") + logger.error("OmniAuth is failing - #{failure_message}") redirect_to root_path end @@ -40,7 +41,7 @@ def omniauth_provider def attributes_from_omniauth { - provider: 'nyulibraries', + provider: omniauth.provider, username: omniauth.uid, email: omniauth_email, firstname: omniauth_firstname, @@ -68,6 +69,6 @@ def set_user end def find_user - @find_user ||= User.find_or_initialize_by(email: omniauth_email) + @find_user ||= User.create_from_provider_data(request.env['omniauth.auth']) end end diff --git a/app/models/user.rb b/app/models/user.rb index 22804246..96f639ae 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,4 +15,13 @@ class User < ActiveRecord::Base def to_s email end + + def self.create_from_provider_data(provider_data) + where(provider: provider_data.provider, + username: provider_data.uid) + .first_or_create do |user| + user.email = provider_data.info.email + user.username = provider_data.uid + end + end end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index c1403871..156e51cf 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -11,7 +11,8 @@ config.reset_password_within = 60.minutes config.sign_out_via = :get config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { - site: Settings.LOGIN_URL, - authorize_path: '/oauth/authorize' + site: (Settings.LOGIN_URL || "https://qa.auth.nyu.edu:443"), + authorize_url: "/oauth2/authorize", + token_url: "/oauth2/token" } end diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index 3fbc3e90..ce10f4c2 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -9,11 +9,6 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end option :name, :shibboleth - option :client_options, { - site: (Settings.LOGIN_URL || "https://qa.auth.nyu.edu:443"), - authorize_url: "/oauth2/authorize" - } - uid do raw_info["sub"] end From 78330deefbe87a5e52a3a8ac5744fb1168600082 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 18:02:06 -0500 Subject: [PATCH 10/19] better logs --- app/controllers/omniauth_callbacks_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index ed927fd8..14e76d95 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -17,7 +17,7 @@ def shibboleth end def failure - logger.error("OmniAuth is failing - #{failure_message}") + logger.error("OmniAuth is failing - #{request.env['omniauth.auth']}") redirect_to root_path end From 28a5cb04f277c1d61e544bbe729d63532a604ad1 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 19:09:20 -0500 Subject: [PATCH 11/19] missing token params? --- config/initializers/omniauth/strategies/shibboleth.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index ce10f4c2..b2059ca2 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -8,6 +8,7 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end end option :name, :shibboleth + option :token_params, { client_id: Settings.APP_ID, client_secret: Settings.APP_SECRET } uid do raw_info["sub"] From a63b4c20707c845e6c508fbecbc7f16fa6796495 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 19:18:06 -0500 Subject: [PATCH 12/19] aLl tHe PaRaMz! --- config/initializers/devise.rb | 4 +++- config/initializers/omniauth/strategies/shibboleth.rb | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 156e51cf..f587da65 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -13,6 +13,8 @@ config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { site: (Settings.LOGIN_URL || "https://qa.auth.nyu.edu:443"), authorize_url: "/oauth2/authorize", - token_url: "/oauth2/token" + authorize_params: { scope: "openid", response_type: "code", client_id: Settings.APP_ID, redirect_uri: Settings.REDIRECT_URI }, + token_url: "/oauth2/token", + redirect_uri: Settings.REDIRECT_URI } end diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index b2059ca2..ce10f4c2 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -8,7 +8,6 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end end option :name, :shibboleth - option :token_params, { client_id: Settings.APP_ID, client_secret: Settings.APP_SECRET } uid do raw_info["sub"] From dc0068c16633362880221edf00a206d2591d7a4c Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 19:33:35 -0500 Subject: [PATCH 13/19] will this log now? --- config/initializers/devise.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index f587da65..69597d1f 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -15,6 +15,7 @@ authorize_url: "/oauth2/authorize", authorize_params: { scope: "openid", response_type: "code", client_id: Settings.APP_ID, redirect_uri: Settings.REDIRECT_URI }, token_url: "/oauth2/token", - redirect_uri: Settings.REDIRECT_URI + redirect_uri: Settings.REDIRECT_URI, + logger: Rails.logger } end From b60246d10e0b39ea1b2f0d1679d41d4997208b61 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 19:57:09 -0500 Subject: [PATCH 14/19] override #log --- config/initializers/devise.rb | 2 +- config/initializers/omniauth/strategies/shibboleth.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 69597d1f..a34eab14 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -11,7 +11,7 @@ config.reset_password_within = 60.minutes config.sign_out_via = :get config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { - site: (Settings.LOGIN_URL || "https://qa.auth.nyu.edu:443"), + site: (Settings.LOGIN_URL || "https://qa.auth.it.nyu.edu"), authorize_url: "/oauth2/authorize", authorize_params: { scope: "openid", response_type: "code", client_id: Settings.APP_ID, redirect_uri: Settings.REDIRECT_URI }, token_url: "/oauth2/token", diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index ce10f4c2..65dd8167 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -36,6 +36,10 @@ def last_name def first_name @first_name ||= raw_info["firstname"] rescue nil end + + def log(level, message) + Rails.logger.send(level, "(#{name}) #{message}") + end end end end From cce786283c05785732b3b800c61b2b1ddec06385 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 20:02:03 -0500 Subject: [PATCH 15/19] errors say we have repeated params? --- app/controllers/omniauth_callbacks_controller.rb | 1 - config/initializers/devise.rb | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 14e76d95..419c1550 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -17,7 +17,6 @@ def shibboleth end def failure - logger.error("OmniAuth is failing - #{request.env['omniauth.auth']}") redirect_to root_path end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index a34eab14..b77c5ff6 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -13,9 +13,7 @@ config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { site: (Settings.LOGIN_URL || "https://qa.auth.it.nyu.edu"), authorize_url: "/oauth2/authorize", - authorize_params: { scope: "openid", response_type: "code", client_id: Settings.APP_ID, redirect_uri: Settings.REDIRECT_URI }, + # authorize_params: { scope: "openid", response_type: "code", client_id: Settings.APP_ID, redirect_uri: Settings.REDIRECT_URI }, token_url: "/oauth2/token", - redirect_uri: Settings.REDIRECT_URI, - logger: Rails.logger } end From c1c72b81dd8353e21a73c8b0d9537c6dbf3fffdb Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 20:04:41 -0500 Subject: [PATCH 16/19] missing scope --- config/initializers/devise.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index b77c5ff6..786328d5 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -13,7 +13,7 @@ config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { site: (Settings.LOGIN_URL || "https://qa.auth.it.nyu.edu"), authorize_url: "/oauth2/authorize", - # authorize_params: { scope: "openid", response_type: "code", client_id: Settings.APP_ID, redirect_uri: Settings.REDIRECT_URI }, + authorize_params: { scope: "openid" }, token_url: "/oauth2/token", } end From a3d7f6faae64d2da8ecd8cb5e37a8512b9747e98 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 20:08:05 -0500 Subject: [PATCH 17/19] trying to get logs to log something valuable --- app/controllers/application_controller.rb | 2 +- app/controllers/omniauth_callbacks_controller.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f39937f4..77d7fb38 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -35,7 +35,7 @@ def after_sign_out_path_for(resource_or_scope) end def logout_path - 'https://login.library.nyu.edu/logout' + Settings.LOGOUT_URL || 'https://qa.auth.it.nyu.edu/oidc/logout' end private :logout_path end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 419c1550..d8ccc883 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -4,13 +4,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController before_action :require_valid_omniauth, only: :shibboleth def shibboleth + Rails.logger.info("OmniauthCallbacksController#shibboleth: #{omniauth.inspect}") set_user if @user.persisted? sign_in_and_redirect @user, event: :authentication set_flash_message(:notice, :success, kind: 'NYU Shibboleth') logger.info(find_message(:success, kind: 'NYU Shibboleth')) else - logger.error(find_message(:failure, kind: 'NYU Shibboleth', reason: 'User not persisted')) + Rails.logger.error(find_message(:failure, kind: 'NYU Shibboleth', reason: 'User not persisted')) session['devise.shibboleth_data'] = request.env['omniauth.auth'] redirect_to root_path end From bd80a73a635a71ac7ed7779af4ca12635950ec89 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Wed, 26 Jul 2023 20:17:49 -0500 Subject: [PATCH 18/19] missing scope --- config/initializers/devise.rb | 1 - config/initializers/omniauth/strategies/shibboleth.rb | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 786328d5..fb9c2474 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -13,7 +13,6 @@ config.omniauth :shibboleth, Settings.APP_ID, Settings.APP_SECRET, client_options: { site: (Settings.LOGIN_URL || "https://qa.auth.it.nyu.edu"), authorize_url: "/oauth2/authorize", - authorize_params: { scope: "openid" }, token_url: "/oauth2/token", } end diff --git a/config/initializers/omniauth/strategies/shibboleth.rb b/config/initializers/omniauth/strategies/shibboleth.rb index 65dd8167..9eb6136c 100644 --- a/config/initializers/omniauth/strategies/shibboleth.rb +++ b/config/initializers/omniauth/strategies/shibboleth.rb @@ -8,6 +8,7 @@ class Shibboleth < OmniAuth::Strategies::OAuth2 end end option :name, :shibboleth + option :authorize_params, { scope: "openid" } uid do raw_info["sub"] From 6e9c6efe647e1e8b02ac5e11f40103782eaf3f08 Mon Sep 17 00:00:00 2001 From: Michael Cain Date: Mon, 7 Aug 2023 12:10:33 -0500 Subject: [PATCH 19/19] rebase and update specs/routes --- .../omniauth_callbacks_controller.rb | 22 +++++++++++------ spec/routing/user_routing_spec.rb | 24 +++++++++---------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index d8ccc883..3d99ef2d 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -7,13 +7,9 @@ def shibboleth Rails.logger.info("OmniauthCallbacksController#shibboleth: #{omniauth.inspect}") set_user if @user.persisted? - sign_in_and_redirect @user, event: :authentication - set_flash_message(:notice, :success, kind: 'NYU Shibboleth') - logger.info(find_message(:success, kind: 'NYU Shibboleth')) + log_in else - Rails.logger.error(find_message(:failure, kind: 'NYU Shibboleth', reason: 'User not persisted')) - session['devise.shibboleth_data'] = request.env['omniauth.auth'] - redirect_to root_path + redirect_to_login end end @@ -23,6 +19,18 @@ def failure private + def redirect_to_login + Rails.logger.error(find_message(:failure, kind: 'NYU Shibboleth', reason: 'User not persisted')) + session['devise.shibboleth_data'] = request.env['omniauth.auth'] + redirect_to root_path + end + + def log_in + sign_in_and_redirect @user, event: :authentication + set_flash_message(:notice, :success, kind: 'NYU Shibboleth') + logger.info(find_message(:success, kind: 'NYU Shibboleth')) + end + def require_valid_omniauth head :bad_request unless valid_omniauth? end @@ -45,7 +53,7 @@ def attributes_from_omniauth username: omniauth.uid, email: omniauth_email, firstname: omniauth_firstname, - lastname: omniauth_lastname, + lastname: omniauth_lastname } end diff --git a/spec/routing/user_routing_spec.rb b/spec/routing/user_routing_spec.rb index 1e0772f3..b7e5eea4 100644 --- a/spec/routing/user_routing_spec.rb +++ b/spec/routing/user_routing_spec.rb @@ -1,33 +1,33 @@ # frozen_string_literal: true describe 'routes for users' do - describe 'GET /users/auth/shibboleth' do - subject { get('/users/auth/shibboleth') } + describe 'GET /auth/shibboleth' do + subject { get('/auth/shibboleth') } it do should route_to({ - controller: 'users/omniauth_callbacks', + controller: 'omniauth_callbacks', action: 'passthru' }) end end - describe 'POST /users/auth/shibboleth' do - subject { post('/users/auth/shibboleth') } + describe 'POST /auth/shibboleth' do + subject { post('/auth/shibboleth') } it do should route_to({ - controller: 'users/omniauth_callbacks', + controller: 'omniauth_callbacks', action: 'passthru' }) end end - describe 'GET /users/auth/shibboleth/callback' do - subject { get('/users/auth/shibboleth/callback') } - it { should route_to({ controller: 'users/omniauth_callbacks', action: 'shibboleth' }) } + describe 'GET /auth/shibboleth/callback' do + subject { get('/auth/shibboleth/callback') } + it { should route_to({ controller: 'omniauth_callbacks', action: 'shibboleth' }) } end - describe 'POST /users/auth/shibboleth/callback' do - subject { post('/users/auth/shibboleth/callback') } - it { should route_to({ controller: 'users/omniauth_callbacks', action: 'shibboleth' }) } + describe 'POST /auth/shibboleth/callback' do + subject { post('/auth/shibboleth/callback') } + it { should route_to({ controller: 'omniauth_callbacks', action: 'shibboleth' }) } end end