From 7cc78a985ee27d5b9a3c08aa3c818203cd4d1746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 25 Jul 2023 07:29:35 +0200 Subject: [PATCH] Allow azure providers to use a configured tenant and graph API (#12852) * Add tenant to azure form * Use graph_api and tenant options to azure * bump providers gem --------- Co-authored-by: Markus Kahl --- Gemfile.lock | 4 ++-- Gemfile.modules | 2 +- .../authentication/openid-providers/README.md | 8 ++++++- .../openid-connect-providers.controller.ts | 18 ++++++++++++++++ .../openid_connect/providers_controller.rb | 10 ++++++--- .../app/models/openid_connect/provider.rb | 3 +++ .../providers/_azure_form.html.erb | 21 +++++++++++++++++++ .../openid_connect/providers/_form.html.erb | 15 +++++++------ .../openid_connect/providers/new.html.erb | 3 +++ modules/openid_connect/config/locales/en.yml | 5 +++++ 10 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 frontend/src/stimulus/controllers/dynamic/admin/openid-connect-providers.controller.ts create mode 100644 modules/openid_connect/app/views/openid_connect/providers/_azure_form.html.erb diff --git a/Gemfile.lock b/Gemfile.lock index 01bf5f9005c..97f771dc231 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -44,8 +44,8 @@ GIT GIT remote: https://github.com/opf/omniauth-openid_connect-providers.git - revision: a6c0c3ed78fac79cf4d007e40d4029e524ec7751 - ref: a6c0c3ed78fac79cf4d007e40d4029e524ec7751 + revision: 7559f44e70203f94572a90e1b4d1d1f8279cd40f + ref: 7559f44e70203f94572a90e1b4d1d1f8279cd40f specs: omniauth-openid_connect-providers (0.2.0) omniauth-openid-connect (>= 0.2.1) diff --git a/Gemfile.modules b/Gemfile.modules index 0b00b4c6932..94b8bca78ae 100644 --- a/Gemfile.modules +++ b/Gemfile.modules @@ -10,7 +10,7 @@ end gem 'omniauth-openid_connect-providers', git: 'https://github.com/opf/omniauth-openid_connect-providers.git', - ref: 'a6c0c3ed78fac79cf4d007e40d4029e524ec7751' + ref: '7559f44e70203f94572a90e1b4d1d1f8279cd40f' gem 'omniauth-openid-connect', git: 'https://github.com/opf/omniauth-openid-connect.git', diff --git a/docs/system-admin-guide/authentication/openid-providers/README.md b/docs/system-admin-guide/authentication/openid-providers/README.md index 813b4ab0ded..9465c22bd56 100644 --- a/docs/system-admin-guide/authentication/openid-providers/README.md +++ b/docs/system-admin-guide/authentication/openid-providers/README.md @@ -165,7 +165,13 @@ At the end of this step, you should have a copy of the Application client ID as ### Step 2: Configure OpenProject -Now, head over to OpenProject > Administration > OpenID providers. Click on "New OpenID provider", select the Azure type, enter the client ID and client Secret and then Save. +Now, head over to OpenProject > Administration > OpenID providers. Click on "New OpenID provider", select the Azure type, enter the client ID and client Secret. + +By default, OpenProject will use the Microsoft Graph API endpoint to perform userinfo requests. +For that, you will need to enter the correct tenant identifier for your Azure instance. +To find the correct value for your instance, [please see this guide](https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-protocols-oidc#find-your-apps-openid-configuration-document-uri). + +Once you filled out the form, hit save and the Azure provider has been created. You can now log out, and see that the login form displays a badge for authenticating with Azure. If you click on that badge, you will be redirected to Azure to enter your credentials and allow the App to access your Azure profile, and you should then be automatically logged in. diff --git a/frontend/src/stimulus/controllers/dynamic/admin/openid-connect-providers.controller.ts b/frontend/src/stimulus/controllers/dynamic/admin/openid-connect-providers.controller.ts new file mode 100644 index 00000000000..4ace8197068 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/admin/openid-connect-providers.controller.ts @@ -0,0 +1,18 @@ +import { Controller } from '@hotwired/stimulus'; + +export default class OpenidConnectProvidersController extends Controller { + static targets = [ + 'azureForm', + ]; + + declare readonly azureFormTarget:HTMLElement; + + public updateTypeForm(evt:InputEvent) { + const name = (evt.target as HTMLInputElement).value; + this.azureFormTarget.hidden = name !== 'azure'; + this + .azureFormTarget + .querySelectorAll('input') + .forEach((el) => (el.disabled = this.azureFormTarget.hidden)); + } +} diff --git a/modules/openid_connect/app/controllers/openid_connect/providers_controller.rb b/modules/openid_connect/app/controllers/openid_connect/providers_controller.rb index 24f0eac2676..38c504544b9 100644 --- a/modules/openid_connect/app/controllers/openid_connect/providers_controller.rb +++ b/modules/openid_connect/app/controllers/openid_connect/providers_controller.rb @@ -13,7 +13,7 @@ module OpenIDConnect if openid_connect_providers_available_for_configure.none? redirect_to action: :index else - @provider = ::OpenIDConnect::Provider.initialize_with({}) + @provider = ::OpenIDConnect::Provider.initialize_with({ use_graph_api: true }) end end @@ -62,11 +62,15 @@ module OpenIDConnect end def create_params - params.require(:openid_connect_provider).permit(:name, :display_name, :identifier, :secret, :limit_self_registration) + params + .require(:openid_connect_provider) + .permit(:name, :display_name, :identifier, :secret, :limit_self_registration, :tenant, :use_graph_api) end def update_params - params.require(:openid_connect_provider).permit(:display_name, :identifier, :secret, :limit_self_registration) + params + .require(:openid_connect_provider) + .permit(:display_name, :identifier, :secret, :limit_self_registration, :tenant, :use_graph_api) end def find_provider diff --git a/modules/openid_connect/app/models/openid_connect/provider.rb b/modules/openid_connect/app/models/openid_connect/provider.rb index 35edacbef0f..c97bd8d3438 100644 --- a/modules/openid_connect/app/models/openid_connect/provider.rb +++ b/modules/openid_connect/app/models/openid_connect/provider.rb @@ -21,6 +21,9 @@ module OpenIDConnect delegate :scope, to: :omniauth_provider, allow_nil: true delegate :to_h, to: :omniauth_provider, allow_nil: false + delegate :tenant, to: :omniauth_provider, allow_nil: false + delegate :use_graph_api, to: :omniauth_provider, allow_nil: false + ## # Controls whether or not self registration shall be limited for this provider. # diff --git a/modules/openid_connect/app/views/openid_connect/providers/_azure_form.html.erb b/modules/openid_connect/app/views/openid_connect/providers/_azure_form.html.erb new file mode 100644 index 00000000000..09c208f6744 --- /dev/null +++ b/modules/openid_connect/app/views/openid_connect/providers/_azure_form.html.erb @@ -0,0 +1,21 @@ +<% if (@provider.new_record? && !providers.map(&:name).include?('azure')) || @provider.name == 'azure' %> + <%= content_tag :fieldset, + class: 'form--fieldset', + data: { + 'admin--openid-connect-providers-target': 'azureForm', + }, + hidden: @provider.name.present? && @provider.name != 'azure' do %> +
+ <%= f.text_field :tenant, required: true, container_class: '-middle' %> +
+ <%= t('openid_connect.setting_instructions.azure_tenant_html') %> +
+
+
+ <%= f.check_box :use_graph_api, container_class: '-middle' %> +
+ <%= t('openid_connect.setting_instructions.azure_graph_api') %> +
+
+ <% end %> +<% end %> diff --git a/modules/openid_connect/app/views/openid_connect/providers/_form.html.erb b/modules/openid_connect/app/views/openid_connect/providers/_form.html.erb index 86d0a2804c8..7361267e018 100644 --- a/modules/openid_connect/app/views/openid_connect/providers/_form.html.erb +++ b/modules/openid_connect/app/views/openid_connect/providers/_form.html.erb @@ -1,12 +1,14 @@
- <% unless f.object.persisted? -%> + <% unless @provider.persisted? -%>
<%= f.collection_select :name, - openid_connect_providers_available_for_configure, - :to_s, - :capitalize, - required: true, - container_class: '-middle' + openid_connect_providers_available_for_configure, + :to_s, + :capitalize, + { container_class: '-middle', required: true }, + data: { + 'action': 'admin--openid-connect-providers#updateTypeForm' + } %>
<% end -%> @@ -30,3 +32,4 @@
+<%= render partial: 'azure_form', locals: { f: } %> diff --git a/modules/openid_connect/app/views/openid_connect/providers/new.html.erb b/modules/openid_connect/app/views/openid_connect/providers/new.html.erb index 9bf97d8c3ad..2ffecfae30b 100644 --- a/modules/openid_connect/app/views/openid_connect/providers/new.html.erb +++ b/modules/openid_connect/app/views/openid_connect/providers/new.html.erb @@ -5,6 +5,9 @@ <%= error_messages_for @provider %> +<% content_controller 'admin--openid-connect-providers', + dynamic: true %> + <%= labelled_tabular_form_for @provider, html: { class: 'form', autocomplete: 'off' } do |f| %> <%= render partial: "form", locals: { f: f } %> diff --git a/modules/openid_connect/config/locales/en.yml b/modules/openid_connect/config/locales/en.yml index c056fee2d26..00609db06eb 100644 --- a/modules/openid_connect/config/locales/en.yml +++ b/modules/openid_connect/config/locales/en.yml @@ -20,5 +20,10 @@ en: plural: OpenID providers singular: OpenID provider setting_instructions: + azure_graph_api: > + Use the graph.microsoft.com userinfo endpoint to request userdata. This should be the default unless you have an older azure application. + azure_tenant_html: > + Set the tenant of your Azure endpoint. This will control who gets access to the OpenProject instance. + For more information, please see our user guide on Azure OpenID connect. limit_self_registration: > If enabled users can only register using this provider if the self registration setting allows for it.