Merge pull request #351 from DigitalCurationCentre/vyruss/bugfixes

Vyruss/bugfixes

Author: jollopre

app/controllers/application_controller.rb

@@ -106,4 +106,15 @@ def errors_to_s(obj)
         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

app/controllers/phases_controller.rb

@@ -56,7 +56,7 @@ def edit
           end
           theme_guidance[title] << {
             text: guidance.text,
-            org: guidance_group.name
+            org: guidance_group.name + ':'
           }
         end
       end

app/controllers/users/omniauth_callbacks_controller.rb

@@ -61,6 +61,7 @@ def handle_omniauth(scheme)
   end
   # -------------------------------------------------------------
 
+
   def failure
     redirect_to root_path
   end

app/views/devise/registrations/_external_identifier.html.erb

@@ -1,6 +1,4 @@
-<div class="user-identifier" 
-     style="background-image: url(<%= "#{scheme.logo_url}" %>); background-size: 16px;">
-     
+<div>
   <% 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 '<i class="fa fa-fw fa-times-circle" aria-hidden="false"></i>'.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 %>
 </div>

app/views/devise/registrations/edit.html.erb

@@ -63,8 +63,14 @@
 
         <% @identifier_schemes.each do |scheme| %>
           <div>
-            <label><%= scheme.name %></label>
-            <div class="identifier-scheme identifier-scheme-indent">
+            <label>
+                <% if scheme.logo_url.nil? %>
+                    <i class="fa fa-user" title="<%= scheme.name %>" aria-hidden="true" style="font-size:16px; width:16px; text-align:center;"></i>
+                <% else %>
+                    <img title="<%= scheme.name %>" src=<%= "#{scheme.logo_url}" %> size="16px" />
+                <% end %>
+           </label>
+            <div class="identifier-scheme">
               <%= render partial: 'external_identifier', 
                          locals: {scheme: scheme, 
                                   id: current_user.identifier_for(scheme)} %>

config/application.rb

@@ -91,12 +91,12 @@ class Application < Rails::Application
 
     # 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'

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'

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? }
 

test/functional/sessions_controller_test.rb

@@ -53,7 +53,11 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest
     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

test/functional/users/omniauth_callbacks_controller_test.rb

@@ -50,8 +50,8 @@ class OmniauthCallbacksControllerTest < ActionDispatch::IntegrationTest
 
       ### 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}"