diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c952b33..54dfc67 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -106,4 +106,15 @@ msg end end + + ## + # Sign out of Shibboleth SP local session too. + # ------------------------------------------------------------- + def after_sign_out_path_for(resource_or_scope) + if Rails.application.config.shibboleth_enabled + return Rails.application.config.shibboleth_logout_url + root_url + super + end + end + # ------------------------------------------------------------- end diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index eead0ca..99406e9 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -56,7 +56,7 @@ end theme_guidance[title] << { text: guidance.text, - org: guidance_group.name + org: guidance_group.name + ':' } end end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 0c6a600..d50e397 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -61,6 +61,7 @@ end # ------------------------------------------------------------- + def failure redirect_to root_path end diff --git a/app/views/devise/registrations/_external_identifier.html.erb b/app/views/devise/registrations/_external_identifier.html.erb index 93c045a..d1071f6 100644 --- a/app/views/devise/registrations/_external_identifier.html.erb +++ b/app/views/devise/registrations/_external_identifier.html.erb @@ -1,6 +1,4 @@ -
); background-size: 16px;"> - +
<% if id.nil? || id.identifier == '' %> <%= link_to "#{_("Create or Link your #{scheme.description} ID")}", Rails.application.routes.url_helpers.send( @@ -15,9 +13,9 @@ <%= link_to "#{_("Your account has been linked to #{scheme.description}.")}", "#{scheme.user_landing_url}/#{id.identifier}", target: '_blank', title: t("identifier_schemes.schemes.#{scheme.name}.connect_tooltip", default: "") %> <% end %> - <%= link_to image_tag('remove.png', height: '16px', width: '16px'), + <%= link_to ''.html_safe, destroy_user_identifier_path(id), method: :delete, title: _("Unlink your account from #{scheme.description}. You can link again at any time."), - data: {confirm: _("Are you sure you want to unlink your #{scheme.description} ID?")} %> + data: {confirm: _("Are you sure you want to unlink #{scheme.description} ID?")} %> <% end %>
diff --git a/app/views/devise/registrations/edit.html.erb b/app/views/devise/registrations/edit.html.erb index 22ac7ef..ec280ec 100644 --- a/app/views/devise/registrations/edit.html.erb +++ b/app/views/devise/registrations/edit.html.erb @@ -63,8 +63,14 @@ <% @identifier_schemes.each do |scheme| %>
- -
+ +
<%= render partial: 'external_identifier', locals: {scheme: scheme, id: current_user.identifier_for(scheme)} %> diff --git a/config/application.rb b/config/application.rb index ce4f957..8b20d7d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -91,12 +91,12 @@ # Enable shibboleth as an alternative authentication method # Requires server configuration and omniauth shibboleth provider configuration - # See config/initializers/omniauth.rb + # See config/initializers/devise.rb config.shibboleth_enabled = true - # Absolute path to Shibboleth SSO Login - config.shibboleth_login = '/Shibboleth.sso/Login' - + # Relative path to Shibboleth SSO Logout + config.shibboleth_logout_url = '/Shibboleth.sso/Logout?return=' + # Active Record will no longer suppress errors raised in after_rollback or after_commit # in the next version. Devise appears to be using those callbacks. # To accept the new behaviour use 'true' otherwise use 'false' diff --git a/config/routes.rb b/config/routes.rb index 27c1d67..8fee9ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,7 +46,7 @@ # fix for activeadmin signout bug devise_scope :user do - get '/users/sign_out' => 'devise/sessions#destroy' + delete '/users/sign_out' => 'devise/sessions#destroy' end delete '/users/identifiers/:id', to: 'user_identifiers#destroy', as: 'destroy_user_identifier' diff --git a/db/seeds.rb b/db/seeds.rb index 7da10a5..d722d06 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -8,7 +8,8 @@ {name: 'orcid', description: 'ORCID', active: true, logo_url:'http://orcid.org/sites/default/files/images/orcid_16x16.png', user_landing_url:'https://orcid.org' }, - {name: 'shibboleth', description: 'Shibboleth', active: true} + {name: 'shibboleth', description: 'your institutional credentials', active: true, + }, ] identifier_schemes.map{ |is| IdentifierScheme.create!(is) if IdentifierScheme.find_by(name: is[:name]).nil? } diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb index ac1b5dd..a41e540 100644 --- a/test/functional/sessions_controller_test.rb +++ b/test/functional/sessions_controller_test.rb @@ -53,7 +53,11 @@ delete destroy_user_session_path assert_equal nil, session[:locale], "expected the locale to have been deleted from the session" assert_response :redirect - assert_redirected_to root_path + if Rails.application.config.shibboleth_enabled + assert_redirected_to Rails.application.config.shibboleth_logout_url + root_url + else + assert_redirected_to root_path + end end end diff --git a/test/functional/users/omniauth_callbacks_controller_test.rb b/test/functional/users/omniauth_callbacks_controller_test.rb index 3dca1ca..d4f7e58 100644 --- a/test/functional/users/omniauth_callbacks_controller_test.rb +++ b/test/functional/users/omniauth_callbacks_controller_test.rb @@ -50,8 +50,8 @@ ### Until ORCID login becomes supported. if scheme.name == 'shibboleth' - assert [I18n.t('devise.omniauth_callbacks.user.success').gsub('%{kind}', scheme.name).downcase, - I18n.t('devise.omniauth_callbacks.success').gsub('%{kind}', scheme.name).downcase].include?(flash[:notice].downcase), "Expected a success message when simulating a valid callback from #{scheme.name}" + assert [I18n.t('devise.omniauth_callbacks.user.success').gsub('%{kind}', scheme.description).downcase, + I18n.t('devise.omniauth_callbacks.success').gsub('%{kind}', scheme.description).downcase].include?(flash[:notice].downcase), "Expected a success message when simulating a valid callback from #{scheme.name}" assert @response.redirect_url.include?(root_url), "Expected a redirect to the root page, #{root_url}, when omniauth returns with a valid identifier!" else assert_equal I18n.t('identifier_schemes.new_login_success'), flash[:notice], "Expected a success message when simulating a valid callback from #{scheme.name}" diff --git a/test/integration/authentication_test.rb b/test/integration/authentication_test.rb index 6f2fe24..c9d4c26 100644 --- a/test/integration/authentication_test.rb +++ b/test/integration/authentication_test.rb @@ -31,10 +31,14 @@ delete destroy_user_session_path assert_response :redirect - follow_redirect! + if Rails.application.config.shibboleth_enabled + assert_redirected_to Rails.application.config.shibboleth_logout_url + root_url + else + assert_redirected_to root_path + end + get root_path # Make sure that the user is sent to the page that lists their plans - assert_response :success assert_select '.welcome-message h2', _('Welcome.') end