Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#61178] Update RemoteIdentity after token obtained. #18033

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/models/oauth_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

class OAuthClient < ApplicationRecord
belongs_to :integration, polymorphic: true
has_many :remote_identities, dependent: :destroy
has_many :remote_identities, as: :auth_source, dependent: :destroy
has_many :oauth_client_tokens, dependent: :destroy

def redirect_uri
Expand Down
5 changes: 1 addition & 4 deletions app/models/oauth_client_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,10 @@
# See COPYRIGHT and LICENSE files for more details.
#++

# OAuthClientToken stores the OAuth2 Bearer+Refresh tokens that
# an OAuth2 server (Nextcloud or similar) provides after a user
# has granted access.
class OAuthClientToken < ApplicationRecord
# OAuthClientToken sits between User and OAuthClient
belongs_to :user, optional: false
belongs_to :oauth_client, optional: false
alias_method :auth_source, :oauth_client

validates :user, uniqueness: { scope: :oauth_client }

Expand Down
7 changes: 4 additions & 3 deletions app/models/remote_identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@

class RemoteIdentity < ApplicationRecord
belongs_to :user
belongs_to :oauth_client, class_name: "OAuthClient"
belongs_to :auth_source, polymorphic: true
belongs_to :integration, polymorphic: true

validates :user, uniqueness: { scope: :oauth_client }
validates :origin_user_id, :user, :oauth_client, presence: true
validates :user, uniqueness: { scope: %i[auth_source integration] }
validates :origin_user_id, :user, :auth_source, :integration, presence: true
end
2 changes: 1 addition & 1 deletion app/services/oauth_clients/connection_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def code_to_token(code)
oauth_client_token.save!

RemoteIdentities::CreateService
.call(user: @user, oauth_config: @config, oauth_client_token:)
.call(user: @user, integration: @oauth_client.integration, token: oauth_client_token)
.on_failure { raise ActiveRecord::Rollback }
end

Expand Down
38 changes: 18 additions & 20 deletions app/services/remote_identities/create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,33 @@ module RemoteIdentities
class CreateService
attr_reader :user, :model

def self.call(user:, oauth_config:, oauth_client_token:)
new(user:, oauth_config:, oauth_client_token:).call
def self.call(user:, integration:, token:)
new(user:, integration:, token:).call
end

def initialize(user:, oauth_config:, oauth_client_token:)
def initialize(user:, integration:, token:)
@user = user
@oauth_config = oauth_config
@oauth_client_token = oauth_client_token
@integration = integration
@token = token

@model = RemoteIdentity.find_or_initialize_by(user:, oauth_client: oauth_config.oauth_client)
@result = ServiceResult.success(result: @model, errors: @model.errors)
@model = RemoteIdentity.find_or_initialize_by(user:, auth_source: token.auth_source, integration:)
end

def call
@model.origin_user_id = @oauth_config.extract_origin_user_id(@oauth_client_token)
if @model.save
emit_event(@oauth_config.oauth_client.integration)
@model.origin_user_id = @integration.extract_origin_user_id(@token)
if @model.changed?
if @model.save
OpenProject::Notifications.send(
OpenProject::Events::REMOTE_IDENTITY_CREATED,
integration: @integration
)
ServiceResult.success(result: @model)
else
ServiceResult.failure(result: @model, errors: @model.errors)
end
else
@result.success = false
ServiceResult.success(result: @model)
end

@result
end

def emit_event(integration)
OpenProject::Notifications.send(
OpenProject::Events::REMOTE_IDENTITY_CREATED,
integration:
)
end
end
end
4 changes: 4 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,10 @@ en:
is_milestone: "Is milestone"
color: "Color"
patterns: "Patterns"
remote_identity:
auth_source: "Auth Source"
integration: "Integration"
user: "User"
user:
admin: "Administrator"
auth_source: "Authentication source"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# frozen_string_literal: true

#-- 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 AddPolymorphicAuthSourceAndIntegrationToRemoteIdentities < ActiveRecord::Migration[7.1]
def up
remove_foreign_key :remote_identities, :oauth_clients

rename_column :remote_identities, :oauth_client_id, :auth_source_id
add_column :remote_identities, :auth_source_type, :string
remove_index :remote_identities, :auth_source_id
remove_index :remote_identities, %i[user_id auth_source_id]
add_index :remote_identities, %i[auth_source_type auth_source_id]

add_column :remote_identities, :integration_type, :string
add_column :remote_identities, :integration_id, :bigint
add_index :remote_identities, %i[integration_type integration_id]

execute <<~SQL.squish
UPDATE remote_identities
SET auth_source_type = 'OAuthClient',
integration_id = oauth_clients.integration_id,
integration_type = oauth_clients.integration_type
FROM oauth_clients
WHERE remote_identities.auth_source_id = oauth_clients.id;
SQL

change_column_null(:remote_identities, :integration_id, false)
change_column_null(:remote_identities, :integration_type, false)
change_column_null(:remote_identities, :auth_source_type, false)

add_index(:remote_identities,
%i[user_id auth_source_type auth_source_id integration_id integration_type],
unique: true)
end

def down
remove_index(:remote_identities,
%i[user_id auth_source_type auth_source_id integration_id integration_type])
rename_column :remote_identities, :auth_source_id, :oauth_client_id
add_foreign_key :remote_identities, :oauth_clients
add_index :remote_identities, :oauth_client_id
add_index :remote_identities, %i[user_id oauth_client_id], unique: true
remove_column :remote_identities, :auth_source_type

remove_column :remote_identities, :integration_id
remove_column :remote_identities, :integration_type
end
end
4 changes: 4 additions & 0 deletions modules/openid_connect/app/models/openid_connect/provider.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
# frozen_string_literal: true

module OpenIDConnect
class Provider < AuthProvider
include HashBuilder

has_many :remote_identities, as: :auth_source, dependent: :destroy

OIDC_PROVIDERS = %w[google microsoft_entra custom].freeze
DISCOVERABLE_STRING_ATTRIBUTES_MANDATORY = %i[authorization_endpoint
userinfo_endpoint
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,9 @@ class UserToken < ::ApplicationRecord

scope :idp, -> { where(audience: IDP_AUDIENCE) }
scope :with_audience, ->(audience) { where("audiences ? :aud", aud: audience) }

def auth_source
user.authentication_provider
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,16 @@ def supported? = false
end
end

attr_reader :user

def initialize(user:)
@user = user
end

def call(audience)
return Failure("Provider does not support token exchange") unless supported?

idp_token = yield FetchService.new(user: @user, token_exchange: Disabled)
idp_token = yield FetchService.new(user:, token_exchange: Disabled)
.access_token_for(audience: UserToken::IDP_AUDIENCE)

json = yield TokenRequest.new(provider:).exchange(idp_token, audience)
Expand All @@ -65,6 +67,7 @@ def call(audience)
# A second reason is that at least Keycloak (an IDP we implement against), offers broken
# refresh tokens after token exchange (see https://github.com/keycloak/keycloak/issues/37016)
token = store_exchanged_token(audience:, access_token:, refresh_token: nil, expires_in:)

Success(token)
end

Expand All @@ -76,22 +79,22 @@ def supported?

def store_exchanged_token(audience:, access_token:, refresh_token:, expires_in:)
token_data = { access_token:, refresh_token:, expires_at: expires_in&.seconds&.from_now }
token = @user.oidc_user_tokens.where("audiences ? :audience", audience:).first
token = user.oidc_user_tokens.where("audiences ? :audience", audience:).first
if token
if token.audiences.size > 1
raise "Did not expect to update token with multiple audiences (#{token.audiences}) in-place."
end

token.update!(**token_data)
else
token = @user.oidc_user_tokens.create!(audiences: [audience], **token_data)
token = user.oidc_user_tokens.create!(audiences: [audience], **token_data)
end

token
end

def provider
@user.authentication_provider
user.authentication_provider
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class FetchService
include Dry::Monads[:result]
include Dry::Monads::Do.for(:access_token_for, :refreshed_access_token_for)

TOKEN_OBTAINED_EVENT = "access_token_obtained"

attr_reader :user

def initialize(user:,
jwt_parser: JwtParser.new(verify_audience: false, verify_expiration: false),
token_exchange: ExchangeService.new(user:),
Expand All @@ -63,6 +67,7 @@ def access_token_for(audience:)
token = yield token_with_audience(audience)
token = yield @token_refresh.call(token) if expired?(token)

emit_event(token, audience)
Success(token.access_token)
end

Expand All @@ -79,11 +84,21 @@ def access_token_for(audience:)
def refreshed_access_token_for(audience:)
token = yield token_with_audience(audience)
token = yield @token_refresh.call(token)

emit_event(token, audience)
Success(token.access_token)
end

private

def emit_event(token, audience)
OpenProject::Notifications.send(
TOKEN_OBTAINED_EVENT,
audience:,
token:
)
end

def token_with_audience(aud)
token = @user.oidc_user_tokens.with_audience(aud).first
return Success(token) if token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,38 @@
expect(result.value!).to eq(access_token)
end

it "emits appropriate event" do
allow(OpenProject::Notifications)
.to receive(:send).with(described_class::TOKEN_OBTAINED_EVENT, token: instance_of(OpenIDConnect::UserToken),
audience: queried_audience).once
expect(result.value!).to eq(access_token)
end

it "does not create RemoteIdentity if storage with appropriate audience is absent" do
create(:nextcloud_storage, storage_audience: "not-expected-audience")
expect { result }.not_to change(RemoteIdentity, :count)
end

context "when storage with appropriate audience is present", :storage_server_helpers, :webmock do
it "raises an error if auth source is not present" do
storage = create(:nextcloud_storage, storage_audience: existing_audience)
stub_nextcloud_user_query(storage.host)

expect { result }.to raise_error("RemoteIdentity creation failed: Auth Source can't be blank.")
end

it "creates remote identity if auth source is present" do
slug = create(:oidc_provider).slug
user.identity_url = "#{slug}:qweqweqweqwe"
user.save

storage = create(:nextcloud_storage, storage_audience: existing_audience)
stub_nextcloud_user_query(storage.host)

expect { result }.to change(RemoteIdentity, :count).from(0).to(1)
end
end

context "when the token doesn't expire and can't be parsed as JWT" do
let(:jwt_parser) { instance_double(OpenIDConnect::JwtParser, parse: Failure("Not a valid JWT")) }

Expand Down Expand Up @@ -142,6 +174,13 @@

it { is_expected.to be_success }

it "emits appropriate event" do
allow(OpenProject::Notifications)
.to receive(:send).with(described_class::TOKEN_OBTAINED_EVENT, token: instance_of(OpenIDConnect::UserToken),
audience: queried_audience).once
expect(result.value!).to eq("access-token-refreshed")
end

it "returns the refreshed access token" do
expect(result.value!).to eq("access-token-refreshed")
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ module Peripherals
namespace("authentication") do
register(:userless, StorageInteraction::AuthenticationStrategies::NextcloudStrategies::UserLess, call: false)
register(:user_bound, StorageInteraction::AuthenticationStrategies::NextcloudStrategies::UserBound)
register(:specific_bearer_token, StorageInteraction::AuthenticationStrategies::NextcloudStrategies::SpecificBearerToken)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,6 @@ def to_httpx_oauth_config = raise ::Storages::Errors::SubclassResponsibility
def authorization_uri(state: nil)
basic_rack_oauth_client.authorization_uri(scope:, state:)
end

def extract_origin_user_id(oauth_client_token)
auth_strategy = Registry
.resolve("#{@storage}.authentication.user_bound")
.call(user: oauth_client_token.user, storage: @storage)
Registry
.resolve("#{@storage}.queries.user")
.call(auth_strategy:, storage: @storage)
.match(
on_success: ->(user) { user[:id] },
on_failure: ->(error) { raise "UserQuery responed with #{error}" }
)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ module Peripherals
namespace("authentication") do
register(:userless, StorageInteraction::AuthenticationStrategies::OneDriveStrategies::UserLess, call: false)
register(:user_bound, StorageInteraction::AuthenticationStrategies::OneDriveStrategies::UserBound)
register(:specific_bearer_token, StorageInteraction::AuthenticationStrategies::OneDriveStrategies::SpecificBearerToken)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ def self.[](strategy)
AuthenticationStrategies::Failure.new
when :basic_auth
AuthenticationStrategies::BasicAuth.new
when :bearer_token
AuthenticationStrategies::SpecificBearerToken.new(strategy.token)
when :sso_user_token
AuthenticationStrategies::SsoUserToken.new(strategy.user)
when :oauth_user_token
Expand Down
Loading
Loading