Call authorized hook in service

Gemfile.lock

@@ -9,8 +9,8 @@ GIT
 
 GIT
   remote: https://github.com/opf/omniauth-openid-connect.git
-  revision: cb40e233f8c31a196af86d8c002e6005d5ffea58
-  ref: cb40e233f8c31a196af86d8c002e6005d5ffea58
+  revision: 057b167aed44b44714baa4e3c0091c7f17df7dfe
+  ref: 057b167aed44b44714baa4e3c0091c7f17df7dfe
   specs:
     omniauth-openid-connect (0.4.0)
       addressable (~> 2.5)

Gemfile.modules

@@ -14,7 +14,7 @@ gem 'omniauth-openid_connect-providers',
 
 gem 'omniauth-openid-connect',
     git: 'https://github.com/opf/omniauth-openid-connect.git',
-    ref: 'cb40e233f8c31a196af86d8c002e6005d5ffea58'
+    ref: '057b167aed44b44714baa4e3c0091c7f17df7dfe'
 
 group :opf_plugins do
     # included so that engines can reference OpenProject::Version

app/services/authentication/omniauth_service.rb

@@ -96,6 +96,8 @@ module Authentication
     # After login flow
     def tap_service_result(call)
       if call.success? && user.active?
+        OpenProject::Hook.call_hook :omniauth_user_authorized, { auth_hash:, controller: }
+        # Call deprecated login hook
         OpenProject::OmniAuth::Authorization.after_login! user, auth_hash, self
       end
 

modules/openid_connect/lib/open_project/openid_connect/engine.rb

@@ -78,7 +78,7 @@ module OpenProject::OpenIDConnect
     end
 
     config.to_prepare do
-      ::OpenProject::OpenIDConnect::Hooks::SessionMapperHook
+      ::OpenProject::OpenIDConnect::Hooks::Hook
     end
   end
 end

modules/openid_connect/lib/open_project/openid_connect/hooks/hook.rb

@@ -28,7 +28,7 @@
 
 module OpenProject::OpenIDConnect
   module Hooks
-    class SessionMapperHook < OpenProject::Hook::Listener
+    class Hook < OpenProject::Hook::Listener
       ##
       # Once the user has signed in and has an oidc session
       # we want to map that to the internal session
@@ -60,4 +60,3 @@ module OpenProject::OpenIDConnect
     end
   end
 end
-

modules/openid_connect/lib/open_project/openid_connect/session_mapper.rb

@@ -9,7 +9,7 @@ module OpenProject::OpenIDConnect
 
     def self.handle_login(oidc_session, session)
       if oidc_session.blank?
-        Rails.logger.info { "No OIDC session returned from provider. Cannot map session for later logouts. "}
+        Rails.logger.info { "No OIDC session returned from provider. Cannot map session for later logouts." }
         return
       end
 
@@ -20,6 +20,7 @@ module OpenProject::OpenIDConnect
     end
 
     attr_reader :session_link
+
     delegate :oidc_session, to: :session_link
 
     def initialize(link)
@@ -43,7 +44,7 @@ module OpenProject::OpenIDConnect
     end
 
     def delete_old_links!
-      ::OpenIDConnect::UserSessionLink.where(oidc_session: oidc_session).delete_all
+      ::OpenIDConnect::UserSessionLink.where(oidc_session:).delete_all
     end
 
     def find_user_session(session)
@@ -67,7 +68,7 @@ module OpenProject::OpenIDConnect
     def remove_linked_session!
       if session_link.session
         Rails.logger.debug { "Deleting linked session for #{oidc_session}" }
-        session_link.session.destroy!
+        session_link.session.delete
       else
         Rails.logger.debug { "Found session link, but no active user session for #{oidc_session}." }
       end

modules/openid_connect/spec/lib/session_mapper_spec.rb

@@ -41,7 +41,7 @@ describe OpenProject::OpenIDConnect::SessionMapper do
 
   describe 'handle_login' do
     let(:session) { mock_session.new('foo') }
-    let!(:plain_session) { create :user_session, session_id: session.id.private_id }
+    let!(:plain_session) { create(:user_session, session_id: session.id.private_id) }
     let!(:user_session) { Sessions::UserSession.find_by(session_id: plain_session.session_id) }
 
     subject { described_class.handle_login 'oidc_sid_foo', session }
@@ -59,12 +59,12 @@ describe OpenProject::OpenIDConnect::SessionMapper do
   describe 'handle_logout' do
     let(:token) { instance_double(OmniAuth::OpenIDConnect::LogoutToken, sid: 'oidc_foobar') }
 
-    subject { described_class.handle_logout token}
+    subject { described_class.handle_logout token }
 
     context 'when an unrelated session exists' do
-      let!(:plain_session) { create :user_session, session_id: 'internal_foobar' }
+      let!(:plain_session) { create(:user_session, session_id: 'internal_foobar') }
       let!(:user_session) { Sessions::UserSession.find_by(session_id: 'internal_foobar') }
-      let!(:link) { create :user_session_link, oidc_session: 'other_oidc_sid', session: user_session }
+      let!(:link) { create(:user_session_link, oidc_session: 'other_oidc_sid', session: user_session) }
 
       it 'does not delete it' do
         expect { subject }.not_to change(OpenIDConnect::UserSessionLink, :count)
@@ -75,9 +75,9 @@ describe OpenProject::OpenIDConnect::SessionMapper do
     end
 
     context 'when a linked session exists' do
-      let!(:plain_session) { create :user_session, session_id: 'internal_foobar' }
+      let!(:plain_session) { create(:user_session, session_id: 'internal_foobar') }
       let!(:user_session) { Sessions::UserSession.find_by(session_id: 'internal_foobar') }
-      let!(:link) { create :user_session_link, oidc_session: 'oidc_foobar', session: user_session }
+      let!(:link) { create(:user_session_link, oidc_session: 'oidc_foobar', session: user_session) }
 
       it 'deletes the linked session' do
         expect { subject }.to change(OpenIDConnect::UserSessionLink, :count).by(-1)

modules/openid_connect/spec/models/user_session_link_spec.rb

@@ -29,9 +29,9 @@ require 'spec_helper'
 
 describe OpenIDConnect::UserSessionLink do
   describe 'session' do
-    let(:plain_session) { create :user_session }
+    let(:plain_session) { create(:user_session) }
     let(:user_session) { Sessions::UserSession.find_by(session_id: plain_session.session_id) }
-    let(:link) { create :user_session_link, session: user_session }
+    let(:link) { create(:user_session_link, session: user_session) }
 
     it 'gets deleted when session is deleted' do
       expect(link).to be_present

spec/services/users/login_service_spec.rb

@@ -31,10 +31,11 @@ require 'spec_helper'
 describe ::Users::LoginService, type: :model do
   let(:input_user) { build_stubbed(:user) }
   let(:controller) { double('ApplicationController') }
+  let(:request) { {} }
   let(:session) { {} }
   let(:flash) { ActionDispatch::Flash::FlashHash.new }
 
-  let(:instance) { described_class.new(controller:) }
+  let(:instance) { described_class.new(controller:, request:) }
 
   subject { instance.call(input_user) }
 
@@ -49,7 +50,8 @@ describe ::Users::LoginService, type: :model do
 
       before do
         allow(::OpenProject::Plugins::AuthPlugin)
-          .to(receive(:login_provider_for))
+          .to(receive(:find_provider_by_name))
+          .with('provider_name')
           .and_return sso_provider
 
         allow(controller)
@@ -74,6 +76,7 @@ describe ::Users::LoginService, type: :model do
         let(:retained_values) { %i[foo bar] }
 
         it 'retains present session values' do
+          session[:omniauth_provider] = 'provider_name'
           session[:foo] = 'foo value'
           session[:what] = 'should be cleared'