diff --git a/app/services/authentication/omniauth_service.rb b/app/services/authentication/omniauth_service.rb index 47f18d600691..567275414f59 100644 --- a/app/services/authentication/omniauth_service.rb +++ b/app/services/authentication/omniauth_service.rb @@ -165,7 +165,7 @@ def find_existing_user def remap_existing_user return unless Setting.oauth_allow_remapping_of_existing_users? - User.not_builtin.find_by_login(user_attributes[:login]) # rubocop:disable Rails/DynamicFindBy + User.not_builtin.find_by_login(user_attributes[:login]) end ## @@ -285,7 +285,7 @@ def identity_url_from_omniauth # Try to provide some context of the auth_hash in case of errors def auth_uid hash = auth_hash || {} - hash.dig(:info, :uid) || hash.dig(:uid) || "unknown" + hash.dig(:info, :uid) || hash[:uid] || "unknown" end end end diff --git a/modules/openid_connect/app/models/openid_connect/user_session_link.rb b/modules/openid_connect/app/models/openid_connect/user_session_link.rb index baa3caab442a..3315ca939638 100644 --- a/modules/openid_connect/app/models/openid_connect/user_session_link.rb +++ b/modules/openid_connect/app/models/openid_connect/user_session_link.rb @@ -1,3 +1,31 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + module OpenIDConnect class UserSessionLink < ::ApplicationRecord self.table_name = "oidc_user_session_links" diff --git a/modules/openid_connect/db/migrate/20240806174815_add_tokens_to_oidc_user_session_links.rb b/modules/openid_connect/db/migrate/20240806174815_add_tokens_to_oidc_user_session_links.rb new file mode 100644 index 000000000000..7976c0ab7dd8 --- /dev/null +++ b/modules/openid_connect/db/migrate/20240806174815_add_tokens_to_oidc_user_session_links.rb @@ -0,0 +1,34 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class AddTokensToOidcUserSessionLinks < ActiveRecord::Migration[7.1] + def change + add_column :oidc_user_session_links, :access_token, :string + add_column :oidc_user_session_links, :refresh_token, :string + end +end diff --git a/modules/openid_connect/db/migrate/20241212104658_make_oidc_session_optional.rb b/modules/openid_connect/db/migrate/20241212104658_make_oidc_session_optional.rb new file mode 100644 index 000000000000..bce1f9a86c65 --- /dev/null +++ b/modules/openid_connect/db/migrate/20241212104658_make_oidc_session_optional.rb @@ -0,0 +1,33 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class MakeOidcSessionOptional < ActiveRecord::Migration[7.1] + def change + change_column_null :oidc_user_session_links, :oidc_session, true + end +end diff --git a/modules/openid_connect/lib/open_project/openid_connect/engine.rb b/modules/openid_connect/lib/open_project/openid_connect/engine.rb index bde77b42ab7f..ced2a8447617 100644 --- a/modules/openid_connect/lib/open_project/openid_connect/engine.rb +++ b/modules/openid_connect/lib/open_project/openid_connect/engine.rb @@ -25,6 +25,8 @@ class Engine < ::Rails::Engine openid_connect/auth_provider-custom.png ) + patches %i[Sessions::UserSession] + class_inflection_override("openid_connect" => "OpenIDConnect") register_auth_providers do @@ -49,7 +51,11 @@ class Engine < ::Rails::Engine end # Remember oidc session values when logging in user - h[:retain_from_session] = %w[omniauth.oidc_sid] + h[:retain_from_session] = %w[ + omniauth.oidc_sid + omniauth.oidc_access_token + omniauth.oidc_refresh_token + ] h[:backchannel_logout_callback] = ->(logout_token) do ::OpenProject::OpenIDConnect::SessionMapper.handle_logout(logout_token) diff --git a/modules/openid_connect/lib/open_project/openid_connect/hooks/hook.rb b/modules/openid_connect/lib/open_project/openid_connect/hooks/hook.rb index 20894a869de6..519655e2f393 100644 --- a/modules/openid_connect/lib/open_project/openid_connect/hooks/hook.rb +++ b/modules/openid_connect/lib/open_project/openid_connect/hooks/hook.rb @@ -33,17 +33,21 @@ 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 def user_logged_in(context) - session = context[:session] - oidc_sid = session["omniauth.oidc_sid"] - return if oidc_sid.nil? - - ::OpenProject::OpenIDConnect::SessionMapper.handle_login(oidc_sid, session) + session = context.fetch(:session) + ::OpenProject::OpenIDConnect::SessionMapper.handle_login(session) end ## # Called once omniauth has returned with an auth hash - # NOTE: It's a passthrough as we no longer persist the access token into the cookie - def omniauth_user_authorized(_context); end + def omniauth_user_authorized(context) + controller = context.fetch(:controller) + session = controller.session + + session["omniauth.oidc_access_token"] = context.dig(:auth_hash, :credentials, :token) + session["omniauth.oidc_refresh_token"] = context.dig(:auth_hash, :credentials, :refresh_token) + + nil + end end end end diff --git a/modules/openid_connect/lib/open_project/openid_connect/patches.rb b/modules/openid_connect/lib/open_project/openid_connect/patches.rb new file mode 100644 index 000000000000..ff3c89768f2e --- /dev/null +++ b/modules/openid_connect/lib/open_project/openid_connect/patches.rb @@ -0,0 +1,30 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module OpenProject::OpenIDConnect::Patches +end diff --git a/modules/openid_connect/lib/open_project/openid_connect/patches/sessions/user_session_patch.rb b/modules/openid_connect/lib/open_project/openid_connect/patches/sessions/user_session_patch.rb new file mode 100644 index 000000000000..e24bfc7544d9 --- /dev/null +++ b/modules/openid_connect/lib/open_project/openid_connect/patches/sessions/user_session_patch.rb @@ -0,0 +1,44 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module OpenProject::OpenIDConnect::Patches::Sessions::UserSessionPatch + def self.included(base) # :nodoc: + base.extend(ClassMethods) + base.include(InstanceMethods) + + base.class_eval do + has_one :oidc_session_link, class_name: "OpenIDConnect::UserSessionLink", foreign_key: "session_id" + end + end + + module ClassMethods + end + + module InstanceMethods + end +end diff --git a/modules/openid_connect/lib/open_project/openid_connect/session_mapper.rb b/modules/openid_connect/lib/open_project/openid_connect/session_mapper.rb index 13107bcffd60..8089ea8b9879 100644 --- a/modules/openid_connect/lib/open_project/openid_connect/session_mapper.rb +++ b/modules/openid_connect/lib/open_project/openid_connect/session_mapper.rb @@ -8,13 +8,10 @@ def self.handle_logout(logout_token) raise e end - 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." } - return - end - - link = ::OpenIDConnect::UserSessionLink.new(oidc_session:) + def self.handle_login(session) + link = ::OpenIDConnect::UserSessionLink.new(oidc_session: session["omniauth.oidc_sid"], + access_token: session["omniauth.oidc_access_token"], + refresh_token: session["omniauth.oidc_refresh_token"]) new(link).link_to_internal!(session) rescue StandardError => e Rails.logger.error { "Failed to map OIDC session to internal: #{e.message}" } diff --git a/modules/openid_connect/spec/lib/session_mapper_spec.rb b/modules/openid_connect/spec/lib/session_mapper_spec.rb index 96bab9c1d940..0c83bf4756af 100644 --- a/modules/openid_connect/spec/lib/session_mapper_spec.rb +++ b/modules/openid_connect/spec/lib/session_mapper_spec.rb @@ -28,23 +28,32 @@ require "spec_helper" RSpec.describe OpenProject::OpenIDConnect::SessionMapper do - let(:mock_session) do - Class.new(Rack::Session::Abstract::SessionHash) do - def initialize(id) - super(nil, nil) - @id = Rack::Session::SessionId.new(id) - @data = {} - @loaded = true - end - end + let(:session) do + instance_double(ActionDispatch::Request::Session, + id: instance_double(Rack::Session::SessionId, private_id: 42)) + end + + let(:session_data) do + { + "omniauth.oidc_sid" => oidc_session_id, + "omniauth.oidc_access_token" => access_token, + "omniauth.oidc_refresh_token" => refresh_token + } + end + + let(:oidc_session_id) { "oidc_sid_foo" } + let(:access_token) { "access_token_bar" } + let(:refresh_token) { "refresh_token_baz" } + + before do + allow(session).to receive(:[]) { |k| session_data[k] } end describe "handle_login" do - let(:session) { mock_session.new("foo") } 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 } + subject { described_class.handle_login session } it "creates a user link object" do expect { subject }.to change(OpenIDConnect::UserSessionLink, :count).by(1) @@ -52,7 +61,25 @@ def initialize(id) expect(link).to be_present expect(link.session).to eq user_session - expect(link.oidc_session).to eq "oidc_sid_foo" + expect(link.access_token).to eq access_token + expect(link.oidc_session).to eq oidc_session_id + expect(link.refresh_token).to eq refresh_token + end + + context "when there is only an access token" do + let(:oidc_session_id) { nil } + let(:refresh_token) { nil } + + it "creates a user link object" do + expect { subject }.to change(OpenIDConnect::UserSessionLink, :count).by(1) + link = OpenIDConnect::UserSessionLink.find_by(session_id: user_session.id) + + expect(link).to be_present + expect(link.session).to eq user_session + expect(link.access_token).to eq access_token + expect(link.oidc_session).to be_nil + expect(link.refresh_token).to be_nil + end end end diff --git a/modules/openid_connect/spec/requests/openid_connect_spec.rb b/modules/openid_connect/spec/requests/openid_connect_spec.rb index 88a9fc4bdd00..9c5246b3c659 100644 --- a/modules/openid_connect/spec/requests/openid_connect_spec.rb +++ b/modules/openid_connect/spec/requests/openid_connect_spec.rb @@ -46,23 +46,26 @@ family_name: "Wurst" } end + let(:access_token) { "foo-bar-baz" } + let(:refresh_token) { "refreshing-foo-bar-baz" } + let(:oidc_sid) { "oidc-session-id-42" } before do # The redirect will include an authorisation code. # Since we don't actually get a valid code in the test we will stub the resulting AccessToken. allow_any_instance_of(OpenIDConnect::Client).to receive(:access_token!) do - OpenIDConnect::AccessToken.new client: self, access_token: "foo bar baz" + instance_double(OpenIDConnect::AccessToken, + access_token:, + refresh_token:, + userinfo!: OpenIDConnect::ResponseObject::UserInfo.new(user_info), + id_token: "not-nil").as_null_object end - # Using the granted AccessToken the client then performs another request to the OpenID Connect - # provider to retrieve user information such as name and email address. - # Since the test is not supposed to make an actual call it is be stubbed too. - allow_any_instance_of(OpenIDConnect::AccessToken).to receive(:userinfo!).and_return( - OpenIDConnect::ResponseObject::UserInfo.new(user_info) + # We are also stubbing the way that an ID token would be decoded, so that the omniauth-openid-connect + # strategy can fill the session id as well + allow(OpenIDConnect::ResponseObject::IdToken).to receive(:decode).and_return( + instance_double(OpenIDConnect::ResponseObject::IdToken, sid: oidc_sid).as_null_object ) - - # Enable storing the access token in a cookie is not necessary since it is currently hard wired to always - # be true. end describe "sign-up and login" do @@ -95,6 +98,14 @@ expect(user).not_to be_nil expect(user.active?).to be true + session = Sessions::UserSession.for_user(user).first + session_link = session.oidc_session_link + + expect(session_link).not_to be_nil + expect(session_link.oidc_session).to eq oidc_sid + expect(session_link.access_token).to eq access_token + expect(session_link.refresh_token).to eq refresh_token + ## # it should redirect to the provider again upon clicking on sign-in when the user has been activated user = User.find_by(mail: user_info[:email])