Skip to content

Commit

Permalink
Merge pull request #17376 from NobodysNightmare/select-keycloak-provider
Browse files Browse the repository at this point in the history
Allow editing and auto-discovering supported grant types of OIDC providers
  • Loading branch information
NobodysNightmare authored Dec 16, 2024
2 parents 4971c8a + 630712e commit 81b7579
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 13 deletions.
5 changes: 5 additions & 0 deletions config/initializers/feature_decisions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,8 @@

OpenProject::FeatureDecisions.add :stages_and_gates,
description: "Enables the under construction feature of stages and gates."

OpenProject::FeatureDecisions.add :oidc_token_exchange,
description: "Enables the under construction OAuth2 token exchange, allowing " \
"users to interact with storage providers without consenting " \
"in OAuth screens before first use."
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,22 @@ module OpenIDConnect
module Providers
class MetadataDetailsForm < BaseForm
form do |f|
OpenIDConnect::Provider::DISCOVERABLE_ATTRIBUTES_ALL.each do |attr|
OpenIDConnect::Provider::DISCOVERABLE_STRING_ATTRIBUTES_ALL.each do |attr|
f.text_field(
name: attr,
label: I18n.t("activemodel.attributes.openid_connect/provider.#{attr}"),
disabled: provider.seeded_from_env?,
required: OpenIDConnect::Provider::DISCOVERABLE_ATTRIBUTES_MANDATORY.include?(attr),
required: OpenIDConnect::Provider::DISCOVERABLE_STRING_ATTRIBUTES_MANDATORY.include?(attr),
input_width: :large
)
end

if OpenProject::FeatureDecisions.oidc_token_exchange_active?
f.text_field(
name: :grant_types_supported,
label: I18n.t("activemodel.attributes.openid_connect/provider.grant_types_supported"),
disabled: provider.seeded_from_env?,
required: false,
input_width: :large
)
end
Expand Down
24 changes: 16 additions & 8 deletions modules/openid_connect/app/models/openid_connect/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,28 @@ class Provider < AuthProvider
include HashBuilder

OIDC_PROVIDERS = %w[google microsoft_entra custom].freeze
DISCOVERABLE_ATTRIBUTES_MANDATORY = %i[authorization_endpoint
userinfo_endpoint
token_endpoint
issuer].freeze
DISCOVERABLE_ATTRIBUTES_OPTIONAL = %i[end_session_endpoint jwks_uri].freeze
DISCOVERABLE_ATTRIBUTES_ALL = DISCOVERABLE_ATTRIBUTES_MANDATORY + DISCOVERABLE_ATTRIBUTES_OPTIONAL
DISCOVERABLE_STRING_ATTRIBUTES_MANDATORY = %i[authorization_endpoint
userinfo_endpoint
token_endpoint
issuer].freeze
DISCOVERABLE_STRING_ATTRIBUTES_OPTIONAL = %i[end_session_endpoint jwks_uri].freeze
DISCOVERABLE_STRING_ATTRIBUTES_ALL = DISCOVERABLE_STRING_ATTRIBUTES_MANDATORY + DISCOVERABLE_STRING_ATTRIBUTES_OPTIONAL

MAPPABLE_ATTRIBUTES = %i[login email first_name last_name admin].freeze

store_attribute :options, :oidc_provider, :string
store_attribute :options, :metadata_url, :string
store_attribute :options, :icon, :string

DISCOVERABLE_ATTRIBUTES_ALL.each do |attribute|
DISCOVERABLE_STRING_ATTRIBUTES_ALL.each do |attribute|
store_attribute :options, attribute, :string
end
MAPPABLE_ATTRIBUTES.each do |attribute|
store_attribute :options, "mapping_#{attribute}", :string
end

store_attribute :options, :grant_types_supported, :json, default: ["authorization_code", "implicit"]

store_attribute :options, :client_id, :string
store_attribute :options, :client_secret, :string
store_attribute :options, :post_logout_redirect_uri, :string
Expand Down Expand Up @@ -55,7 +57,7 @@ def advanced_details_configured?
def metadata_configured?
return true if google? || entra_id?

DISCOVERABLE_ATTRIBUTES_MANDATORY.all? do |mandatory_attribute|
DISCOVERABLE_STRING_ATTRIBUTES_MANDATORY.all? do |mandatory_attribute|
public_send(mandatory_attribute).present?
end
end
Expand All @@ -78,6 +80,12 @@ def configured?
display_name.present? && advanced_details_configured? && metadata_configured?
end

def token_exchange_capable?
return false if grant_types_supported.blank?

grant_types_supported.include?("urn:ietf:params:oauth:grant-type:token-exchange")
end

def icon
case oidc_provider
when "google"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,19 @@ def set_attributes(params)

super

update_grant_types_supported if params.key?(:grant_types_supported)
update_available_state
end

# This is a workaround for a non-ideal UI
# We only offer users to edit the supported grant types in a text input field,
# though they are indeed editing a list of grants.
def update_grant_types_supported
return unless params[:grant_types_supported].is_a? String

model.grant_types_supported = params[:grant_types_supported].split
end

def update_available_state
model.change_by_system do
model.available = model.configured?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ module OpenIDConnect
module Providers
class UpdateService < BaseServices::Update
class AttributesContract < Dry::Validation::Contract
params do
OpenIDConnect::Provider::DISCOVERABLE_ATTRIBUTES_MANDATORY.each do |attribute|
json do
OpenIDConnect::Provider::DISCOVERABLE_STRING_ATTRIBUTES_MANDATORY.each do |attribute|
required(attribute).filled(:string)
end
OpenIDConnect::Provider::DISCOVERABLE_ATTRIBUTES_OPTIONAL.each do |attribute|
OpenIDConnect::Provider::DISCOVERABLE_STRING_ATTRIBUTES_OPTIONAL.each do |attribute|
optional(attribute).filled(:string)
end

optional(:grant_types_supported).array(:string)
end
end

Expand Down
1 change: 1 addition & 0 deletions modules/openid_connect/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ en:
client_secret: Client secret
secret: Secret
scope: Scope
grant_types_supported: Supported grant types
limit_self_registration: Limit self registration
authorization_endpoint: Authorization endpoint
userinfo_endpoint: User information endpoint
Expand Down
55 changes: 55 additions & 0 deletions modules/openid_connect/spec/models/provider_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#-- 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.
#++
require "spec_helper"

RSpec.describe OpenIDConnect::Provider do
let(:provider) do
create(:oidc_provider, options: {
"grant_types_supported" => supported_grant_types
})
end
let(:supported_grant_types) { %w[authorization_code implicit] }

describe "#token_exchange_capable?" do
subject { provider.token_exchange_capable? }

it { is_expected.to be_falsey }

context "when the provider supports the token exchange grant" do
let(:supported_grant_types) { %w[authorization_code implicit urn:ietf:params:oauth:grant-type:token-exchange] }

it { is_expected.to be_truthy }
end

context "when supported grant types are nil (legacy providers)" do
let(:supported_grant_types) { nil }

it { is_expected.to be_falsey }
end
end
end

0 comments on commit 81b7579

Please sign in to comment.