Merge pull request #907 from portagenetwork/aaron/issues/sso-link

Update Handling of SSO Linking

Author: aaronskiba

CHANGELOG.md

@@ -1,9 +1,11 @@
 # Changelog
 
-## [Unreleased]
+## [4.1.1+portage-4.2.2] - 2024-09-18
 
 ### Changed
 
+ - Update Handling of SSO Linking [#907](https://github.com/portagenetwork/roadmap/pull/907)
+ 
  - Updated SSO button string [portagenetwork/roadmap#903](https://github.com/portagenetwork/roadmap/issues/903)
 
 ## [4.1.1+portage-4.2.1] - 2024-09-12

app/controllers/users/omniauth_callbacks_controller.rb

@@ -5,7 +5,7 @@ module Users
   class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     # This is for the OpenidConnect CILogon
 
-    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
+    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
     def openid_connect
       # First or create
       auth = request.env['omniauth.auth']
@@ -22,11 +22,18 @@ def openid_connect
 
       identifier_scheme = IdentifierScheme.find_by(name: auth.provider)
 
-      if current_user.nil?
-        # We need to register
-        if user.nil?
-          # Register and sign in
+      if current_user.nil? # if user is not signed in (They clicked the SSO sign in button)
+        if user.nil? # If an entry does not exist in the identifiers table for the chosen SSO account
           user = User.create_from_provider_data(auth)
+          if user.nil? # if a user was NOT created (a match was found for User.find_by(email: auth.info.email)
+            # Do not link SSO credentials for the signed out, existing user
+            flash[:alert] = _('That email appears to be associated with an existing account.<br>' \
+                              'Sign into your existing account, and you can link that ' \
+                              "account with SSO from the 'Edit Profile' page.")
+            redirect_to root_path
+            return
+          end
+          # A new user was created, link the SSO credentials (we can do this for a newly created user)
           user.identifiers << Identifier.create(identifier_scheme: identifier_scheme,
                                                 value: auth.uid,
                                                 attrs: auth,
@@ -51,7 +58,7 @@ def openid_connect
         redirect_to edit_user_registration_path
       end
     end
-    # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
+    # rubocop:enable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
 
     def orcid
       handle_omniauth(IdentifierScheme.for_authentication.find_by(name: 'orcid'))

app/models/user.rb

@@ -186,11 +186,11 @@ def self.from_omniauth(auth)
   # Handle user creation from provider
   # rubocop:disable Metrics/AbcSize
   def self.create_from_provider_data(provider_data)
-    user = User.find_by email: provider_data.info.email
+    user = User.find_or_initialize_by(email: provider_data.info.email)
 
-    return user if user
+    return unless user.new_record?
 
-    User.create!(
+    user.update!(
       firstname: provider_data.info&.first_name.present? ? provider_data.info.first_name : _('First name'),
       surname: provider_data.info&.last_name.present? ? provider_data.info.last_name : _('Last name'),
       email: provider_data.info.email,
@@ -199,6 +199,7 @@ def self.create_from_provider_data(provider_data)
       accept_terms: true,
       password: Devise.friendly_token[0, 20]
     )
+    user
   end
   # rubocop:enable Metrics/AbcSize
 

spec/integration/openid_connect_sso_spec.rb

@@ -39,7 +39,19 @@
       expect(page).to have_content('John Doe')
     end
 
-    it 'links account from external credentails' do
+    it 'does not create SSO link when user is signed out and SSO email is an existing account email' do
+      # Hardcode user email to what we are mocking via OmniAuth.config.mock_auth[:openid_connect]
+      user = create(:user, :org_admin, org: @org, email: 'user@organization.ca', firstname: 'DMP Name',
+                                       surname: 'DMP Lastname')
+      expect(user.identifiers.count).to eql(0)
+      visit root_path
+      click_link 'Sign in with CILogon'
+      error_message = 'That email appears to be associated with an existing account'
+      expect(page).to have_content(error_message)
+      expect(user.identifiers.count).to eql(0)
+    end
+
+    xit 'links account from external credentails' do
       # Create existing user
       user = create(:user, :org_admin, org: @org, email: 'user@organization.ca', firstname: 'DMP Name',
                                        surname: 'DMP Lastname')