From 2172c102c95604d90476c9829caa40a490071477 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Tue, 31 May 2022 18:06:35 +0200 Subject: [PATCH] (bug) use arrays for OIDC scopes in front and DB. Send the scope as a string separated with spaces to the OIDC provider. --- .../openid-connect-form.tsx | 34 ++++++++++++------- .../components/form/form-multi-select.tsx | 4 +-- .../models/authentication-provider.ts | 2 +- app/models/open_id_connect_provider.rb | 4 +++ ...220531160223_change_oidc_scope_to_array.rb | 9 +++++ db/schema.rb | 4 +-- 6 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20220531160223_change_oidc_scope_to_array.rb diff --git a/app/frontend/src/javascript/components/authentication-provider/openid-connect-form.tsx b/app/frontend/src/javascript/components/authentication-provider/openid-connect-form.tsx index aad8aa199..4969c6842 100644 --- a/app/frontend/src/javascript/components/authentication-provider/openid-connect-form.tsx +++ b/app/frontend/src/javascript/components/authentication-provider/openid-connect-form.tsx @@ -10,6 +10,7 @@ import { OpenIdConnectProvider } from '../../models/authentication-provider'; import SsoClient from '../../api/external/sso'; import { FieldPathValue } from 'react-hook-form/dist/types/path'; import { FormMultiSelect } from '../form/form-multi-select'; +import { difference } from 'lodash'; interface OpenidConnectFormProps { register: UseFormRegister, @@ -60,6 +61,21 @@ export const OpenidConnectForm = ) => void): void => { + const current = currentFormValues.scope || []; + if (scopesAvailable) { + // add custom scopes to the list of available scopes + const unlisted = difference(current, scopesAvailable); + callback(scopesAvailable.concat(unlisted).map(scope => ({ value: scope, label: scope }))); + } else { + current.map(scope => ({ value: scope, label: scope })); + } + }; + /** * Callback that check for the existence of the .well-known/openid-configuration endpoint, for the given issuer. * This callback is triggered when the user changes the issuer field. @@ -102,18 +118,12 @@ export const OpenidConnectForm = - {!scopesAvailable && } />} - {scopesAvailable && } - options={scopesAvailable.map((scope) => ({ value: scope, label: scope }))} - delimiter={' '} - creatable - control={control} />} + } + loadOptions={loadScopes} + creatable + control={control} /> } diff --git a/app/frontend/src/javascript/components/form/form-multi-select.tsx b/app/frontend/src/javascript/components/form/form-multi-select.tsx index 2cac0970b..f19f1652c 100644 --- a/app/frontend/src/javascript/components/form/form-multi-select.tsx +++ b/app/frontend/src/javascript/components/form/form-multi-select.tsx @@ -16,7 +16,6 @@ interface CommonProps exten onChange?: (values: Array) => void, placeholder?: string, creatable?: boolean, - delimiter?: string } // we should provide either an array of options or a function that returns a promise, but not both @@ -36,7 +35,7 @@ type selectOption = { value: TOptionValue, label: string, select?: * This component is a wrapper around react-select to use with react-hook-form. * It is a multi-select component. */ -export const FormMultiSelect = ({ id, label, tooltip, className, control, placeholder, options, valuesDefault, error, rules, disabled, onChange, formState, warning, loadOptions, creatable, delimiter }: FormSelectProps) => { +export const FormMultiSelect = ({ id, label, tooltip, className, control, placeholder, options, valuesDefault, error, rules, disabled, onChange, formState, warning, loadOptions, creatable }: FormSelectProps) => { const { t } = useTranslation('shared'); const [isDisabled, setIsDisabled] = React.useState(false); @@ -121,7 +120,6 @@ export const FormMultiSelect = , prompt?: 'none' | 'login' | 'consent' | 'select_account', send_scope_to_token_endpoint?: string, client__identifier: string, diff --git a/app/models/open_id_connect_provider.rb b/app/models/open_id_connect_provider.rb index ccf2fee5d..c9db5020e 100644 --- a/app/models/open_id_connect_provider.rb +++ b/app/models/open_id_connect_provider.rb @@ -17,6 +17,10 @@ class OpenIdConnectProvider < ApplicationRecord validates :prompt, inclusion: { in: %w[none login consent select_account], allow_nil: true } validates :client_auth_method, inclusion: { in: %w[basic jwks] } + def scope + self[:scope].join(' ') + end + def config OpenIdConnectProvider.columns.map(&:name).filter { |n| !n.start_with?('client__') && n != 'profile_url' }.map do |n| [n, send(n)] diff --git a/db/migrate/20220531160223_change_oidc_scope_to_array.rb b/db/migrate/20220531160223_change_oidc_scope_to_array.rb new file mode 100644 index 000000000..323317c01 --- /dev/null +++ b/db/migrate/20220531160223_change_oidc_scope_to_array.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +# Previously, the OpenID Connect scope was a string, scopes were separated by commas. +# To be more fron-end friendly, we now use an array. +class ChangeOidcScopeToArray < ActiveRecord::Migration[5.2] + def change + change_column :open_id_connect_providers, :scope, "varchar[] USING (string_to_array(scope, ','))" + end +end diff --git a/db/schema.rb b/db/schema.rb index 6c6b96071..55a7cad70 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_05_17_140916) do +ActiveRecord::Schema.define(version: 2022_05_31_160223) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" @@ -415,7 +415,7 @@ ActiveRecord::Schema.define(version: 2022_05_17_140916) do t.string "issuer" t.boolean "discovery" t.string "client_auth_method" - t.string "scope" + t.string "scope", array: true t.string "response_type" t.string "response_mode" t.string "display"