diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 9ad13ce..9f7f372 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -21,8 +21,7 @@
# -------------------------------------------------------------
def handle_omniauth(scheme)
user = User.from_omniauth(request.env["omniauth.auth"].nil? ? request.env : request.env["omniauth.auth"])
- identifier = UserIdentifier.where(identifier: request.env["omniauth.auth"].uid).first
-
+
# If the user isn't logged in
if current_user.nil?
# If the uid didn't have a match in the system send them to register
@@ -38,7 +37,7 @@
set_flash_message(:notice, :success, kind: scheme.description) if is_navigational_format?
sign_in_and_redirect user, event: :authentication
else
- flash[:notice] = t('identifier_schemes.new_login_success')
+ flash[:notice] = _('Successfully signed in')
redirect_to new_user_registration_url
end
end
@@ -53,12 +52,19 @@
flash[:notice] = _('Your account has been successfully linked to %{scheme}.') % { scheme: scheme.description }
else
- flash[:notice] = _('Unable to link your account to %{scheme}.') % { scheme: scheme.description }
+ flash[:alert] = _('Unable to link your account to %{scheme}.') % { scheme: scheme.description }
end
- end
-
- if identifier.user.id != current_user.id
- flash[:notice] = _("The current #{scheme.description} iD has been already linked to a user with email #{identifier.user.email}")
+
+ else
+ # If a user was found but does NOT match the current user then the identifier has
+ # already been attached to another account (likely the user has 2 accounts)
+ identifier = UserIdentifier.where(identifier: request.env["omniauth.auth"].uid).first
+ if identifier.user.id != current_user.id
+ flash[:alert] = _("The current #{scheme.description} iD has been already linked to a user with email #{identifier.user.email}")
+ end
+
+ # Otherwise, the identifier was found and it matches the one already associated
+ # with the current user so nothing else needs to be done
end
# Redirect to the User Profile page
diff --git a/app/views/devise/registrations/_external_identifier_orcid.html.erb b/app/views/devise/registrations/_external_identifier_orcid.html.erb
index 2ba5cf0..ca1db01 100644
--- a/app/views/devise/registrations/_external_identifier_orcid.html.erb
+++ b/app/views/devise/registrations/_external_identifier_orcid.html.erb
@@ -1,6 +1,6 @@
<% if id.nil? || id.identifier == '' %>
- <%= link_to 'Create or Connect your Orcid iD', Rails.application.routes.url_helpers.send("user_#{scheme.name.downcase}_omniauth_authorize_path"), id:"connect-orcid-button", target: '_blank', title: t("identifier_schemes.schemes.#{scheme.name}.connect_tooltip", default: "") %>
+ <%= link_to 'Create or connect your ORCID iD', Rails.application.routes.url_helpers.send("user_#{scheme.name.downcase}_omniauth_authorize_path"), id:"connect-orcid-button", target: '_blank', title: t("identifier_schemes.schemes.#{scheme.name}.connect_tooltip", default: "") %>
<% else %>
<% if scheme.user_landing_url.nil? %>
<%= _("Your account has been linked to #{scheme.description}.") %>
diff --git a/lib/assets/stylesheets/dmproadmap/tabs.scss b/lib/assets/stylesheets/dmproadmap/tabs.scss
index 5570e5a..b327b21 100644
--- a/lib/assets/stylesheets/dmproadmap/tabs.scss
+++ b/lib/assets/stylesheets/dmproadmap/tabs.scss
@@ -17,9 +17,14 @@
left: 1px;
color: $white;
background-color: $primary-color;
- border-bottom: 2px solid $primary-color;
text-decoration: none;
font-weight: bold;
+ box-sizing: border-box;
+ border-width: 2px;
+ border-style: solid;
+ border-color: inherit;
+ border-bottom: 0px;
+ border-radius: 0px;
}
}
li a:hover,
diff --git a/test/functional/users/omniauth_callbacks_controller_test.rb b/test/functional/users/omniauth_callbacks_controller_test.rb
index d4f7e58..f902709 100644
--- a/test/functional/users/omniauth_callbacks_controller_test.rb
+++ b/test/functional/users/omniauth_callbacks_controller_test.rb
@@ -54,7 +54,7 @@
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}"
+ assert_equal _('Successfully signed in'), flash[:notice], "Expected a success message when simulating a valid callback from #{scheme.name}"
assert @response.redirect_url.include?(new_user_registration_url), "Expected a redirect to the registration page when the user is not logged in and we received a valid callback from #{scheme.name}"
assert_not "#user_identifiers[#{scheme.name}]".nil?
end
@@ -78,5 +78,32 @@
assert_equal usr.user_identifiers.find_by(identifier_scheme: scheme).identifier, 'foo:bar'
end
end
+
+ # -------------------------------------------------------------
+ test "An omniauth identifier cannot be tied to multiple accounts" do
+ new_account = User.create!(firstname: 'Tester', surname: 'Duplicate',
+ email: 'tester.duplicate@somewhereelse.org',
+ org: Org.first, accept_terms: true,
+ password: "password123", password_confirmation: "password123")
+
+ @schemes.each do |scheme|
+ @user.user_identifiers << UserIdentifier.new(identifier_scheme: scheme,
+ identifier: "foo:bar")
+ sign_in new_account
+
+ post @callback_uris[scheme.name]
+
+ assert_equal _("The current #{scheme.description} iD has been already linked to a user with email #{@user.email}"), flash[:alert], "Expected that we could not attach #{scheme.name} to an account if it has already been associated with another account"
+
+ assert_redirected_to "#{edit_user_registration_path}", "Expected a redirect to the edit profile page, #{edit_user_registration_path}, when omniauth returns with a valid identifier for a user that is already signed in!"
+
+ # reload the user record and make sure the omniauth value NOT attached to their record
+ # and still associated with the original account
+ @user.reload
+ assert_equal @user.user_identifiers.find_by(identifier_scheme: scheme).identifier, 'foo:bar'
+ new_account.reload
+ assert new_account.user_identifiers.find_by(identifier_scheme: scheme).nil?, "Expected the new account to NOT be associated with the #{scheme.name}"
+ end
+ end
end