From c1479ce459d61b66cbd0456af9a8aeacdc88a210 Mon Sep 17 00:00:00 2001 From: Yinzi Xie Date: Thu, 19 Dec 2024 22:27:29 +0000 Subject: [PATCH] OEQ-2351 - refactor: OIDC rest module --- oeq-ts-rest-api/src/Oidc.ts | 55 ++++---- oeq-ts-rest-api/test/Oidc.test.ts | 4 +- .../Integrations/oidc/OidcSettings.tsx | 130 +++++++----------- .../Integrations/oidc/OidcSettingsHelper.tsx | 63 ++------- 4 files changed, 93 insertions(+), 159 deletions(-) diff --git a/oeq-ts-rest-api/src/Oidc.ts b/oeq-ts-rest-api/src/Oidc.ts index 39f0b7ae37..70f076ac42 100644 --- a/oeq-ts-rest-api/src/Oidc.ts +++ b/oeq-ts-rest-api/src/Oidc.ts @@ -33,7 +33,7 @@ export type IdentityProviderPlatform = 'AUTH0' | 'ENTRA_ID' | 'OKTA'; * Data structure for the common details of an Identity Provider, containing fields * with primitive types. **/ -interface IdentityProviderBase { +interface CommonDetailsBase { /** * One of the supported Identity Provider: {@link IdentityProviderPlatform} */ @@ -78,7 +78,7 @@ interface IdentityProviderBase { /** * Full structure of an Identity Provider where types are looser in order to be handled by axios. */ -interface IdentityProviderRaw extends IdentityProviderBase { +interface CommonDetailsRaw extends CommonDetailsBase { /** * A list of default OEQ roles to assign to the logged-in user. */ @@ -101,7 +101,7 @@ interface IdentityProviderRaw extends IdentityProviderBase { /** * Full structure of an Identity Provider, including default roles and custom role configuration. */ -export interface IdentityProvider extends IdentityProviderBase { +export interface CommonDetails extends CommonDetailsBase { /** * A list of default OEQ roles to assign to the logged-in user. */ @@ -122,50 +122,53 @@ export interface IdentityProvider extends IdentityProviderBase { } /** - * Full structure for an Generic Identity Provider, including all the common details and specific - * fields related to interacting with the Identity Provider's API. + * API details configured for the use of an Identity Provider's REST APIs. */ -export interface GenericIdentityProvider extends IdentityProvider { - platform: 'AUTH0' | 'ENTRA_ID'; +export interface RestApiDetails { /** - * The API endpoint for the Identity Provider, use for operations such as search for users + * The API endpoint for the Identity Provider, use for operations such as search for users. */ apiUrl: string; /** - * Client ID used to get an Authorisation Token to use with the Identity Provider's API + * Client ID used to get an Authorisation Token to use with the Identity Provider's API. */ apiClientId: string; /** - * Client Secret used with `apiClientId` to get an Authorization Token to use with the Identity Provider's API + * Client Secret used with `apiClientId` to get an Authorization Token to use with the Identity Provider's API. */ apiClientSecret?: string; } +interface EntraId extends CommonDetails, RestApiDetails { + platform: 'ENTRA_ID'; +} + +interface Auth0 extends CommonDetails, RestApiDetails { + platform: 'AUTH0'; +} + +interface Okta extends CommonDetails, RestApiDetails { + platform: 'OKTA'; +} + +export type IdentityProvider = EntraId | Auth0 | Okta; + /** - * Full structure for the configuration of Okta, including all the common details for OIDC and API - * details for interacting with Core Okta API. + * Data structure for the response of an Identity Provider where common details are consolidated in one field and secret values are excluded. */ -export interface Okta extends IdentityProvider { - platform: 'OKTA'; +interface IdentityProviderResponse { + commonDetails: CommonDetailsRaw; /** - * The base endpoint of Core Okta API, use for operations such as search for users + * The API endpoint for the Identity Provider, use for operations such as search for users. */ apiUrl: string; /** - * Client ID used to request an access token to use with Core Okta API + * Client ID used to get an Authorisation Token to use with the Identity Provider's API. */ apiClientId: string; } -/** - * Data structure for the response of an Identity Provider, which is slightly different - * as all the common fields are centralised into a single field 'commonDetails'. - */ -interface IdentityProviderResponse { - commonDetails: IdentityProviderRaw; -} - -const toIdentityProviderRaw = (idp: IdentityProvider): IdentityProviderRaw => ({ +const toIdentityProviderRaw = (idp: IdentityProvider): CommonDetailsRaw => ({ ...idp, defaultRoles: toStringArray(idp.defaultRoles), roleConfig: pipe( @@ -195,6 +198,8 @@ const toIdentityProvider = ({ O.toUndefined ), ...platformSpecific, + // Because server does not return the client secret, set it to undefined. + apiClientSecret: undefined, }); /** diff --git a/oeq-ts-rest-api/test/Oidc.test.ts b/oeq-ts-rest-api/test/Oidc.test.ts index 528d7799ed..aecbf71c5c 100644 --- a/oeq-ts-rest-api/test/Oidc.test.ts +++ b/oeq-ts-rest-api/test/Oidc.test.ts @@ -18,13 +18,13 @@ import * as OEQ from '../src'; import { IdentityProviderCodec } from '../src/gen/Oidc'; import { - GenericIdentityProvider, + IdentityProvider, getIdentityProvider, updateIdentityProvider, } from '../src/Oidc'; import * as TC from './TestConfig'; -const auth0: GenericIdentityProvider = { +const auth0: IdentityProvider = { platform: 'AUTH0', issuer: 'https://dev-cqchwn4hfdb1p8xr.au.auth0.com', authCodeClientId: 'C5tvBaB7svqjLPe0dDPBicgPcVPDJumZ', diff --git a/react-front-end/tsrc/settings/Integrations/oidc/OidcSettings.tsx b/react-front-end/tsrc/settings/Integrations/oidc/OidcSettings.tsx index 082c95d416..c90c64f75b 100644 --- a/react-front-end/tsrc/settings/Integrations/oidc/OidcSettings.tsx +++ b/react-front-end/tsrc/settings/Integrations/oidc/OidcSettings.tsx @@ -58,14 +58,12 @@ import { import * as S from "fp-ts/string"; import SelectRoleControl from "../lti13/components/SelectRoleControl"; import { - defaultGeneralDetails, - defaultEntraIdApiDetails, - defaultApiDetailsMap, + defaultConfig, generateGeneralDetails, generateApiDetails, - ApiDetails, generatePlatform, oeqDetailsList, + defaultApiDetails, } from "./OidcSettingsHelper"; import * as O from "fp-ts/Option"; import * as R from "fp-ts/Record"; @@ -150,11 +148,8 @@ const OidcSettings = ({ const [showValidationErrors, setShowValidationErrors] = useState(false); // States for values displayed in different sections. - const [generalDetails, setGeneralDetails] = - useState(defaultGeneralDetails); - const [apiDetails, setApiDetails] = useState( - defaultEntraIdApiDetails, - ); + const [config, setConfig] = + useState(defaultConfig); const [defaultRoles, setDefaultRoles] = useState>( new Set(), ); @@ -168,29 +163,29 @@ const OidcSettings = ({ customRoles: undefined, }); + // TODO: remove this in OEQ-2359. // Build final submit values for OIDC. - const currentOidcValue = { - ...generalDetails, - ...apiDetails, + const currentConfig = { + ...config, defaultRoles: pipe(roleIds(defaultRoles), RS.toSet), - roleConfig: generalDetails.roleConfig?.roleClaim + roleConfig: config.roleConfig?.roleClaim ? { - roleClaim: generalDetails.roleConfig.roleClaim, + roleClaim: config.roleConfig.roleClaim, customRoles: transformCustomRoleMapping(customRoles), } : undefined, }; - // Initial configuration retrieved from server, but before the config is returned, use the defaults values of each state listed above. - const [initialIdpDetails, setInitialIdpDetails] = - useState(currentOidcValue); + // Initial configuration retrieved from server, but before the config is returned, use the defaults values of "config". + const [initialConfig, setInitialConfig] = + useState(currentConfig); // Flag to indicate if there is an existing configuration in server. const [serverHasConfiguration, setServerHasConfiguration] = useState(false); const configurationChanged = hasConfigurationChanged( - initialIdpDetails, - currentOidcValue, + initialConfig, + currentConfig, ); useEffect(() => { @@ -253,24 +248,8 @@ const OidcSettings = ({ flow( O.fold(constVoid, (idp) => { setServerHasConfiguration(true); - setInitialIdpDetails(idp); - setGeneralDetails(idp); - - if (OEQ.Codec.Oidc.GenericIdentityProviderCodec.is(idp)) { - setApiDetails({ - platform: idp.platform, - apiUrl: idp.apiUrl, - apiClientId: idp.apiClientId, - apiClientSecret: idp.apiClientSecret, - }); - } else if (OEQ.Codec.Oidc.OktaCodec.is(idp)) { - setApiDetails({ - platform: idp.platform, - apiUrl: idp.apiUrl, - apiClientId: idp.apiClientId, - }); - } - + setInitialConfig(idp); + setConfig(idp); // process role mappings value to display existing settings setRoles(idp); }), @@ -279,46 +258,39 @@ const OidcSettings = ({ )(); }, [appErrorHandler, resolveRolesProvider]); - // Update the corresponding value in generalDetails state based on the provided key. - const onGeneralDetailsChange = (key: string, newValue: unknown) => - setGeneralDetails({ - ...generalDetails, + // Update the corresponding value in idpConfigurations state based on the provided key. + const onConfigChange = (key: string, newValue: unknown) => + setConfig({ + ...config, [key]: newValue, }); const onPlatformChange = (newValue: string) => { - // update platform - onGeneralDetailsChange("platform", newValue); - // Also clear the previous API configurations on platform change. - pipe( - defaultApiDetailsMap, - R.lookup(newValue), - O.map(setApiDetails), - O.getOrElseW(() => { - appErrorHandler(`Unsupported platform ${newValue}`); - }), - ); - }; - - const onApiDetailsChange = (key: string, newValue: unknown) => - setApiDetails({ - ...apiDetails, - [key]: newValue, + if (!OEQ.Codec.Oidc.IdentityProviderPlatformCodec.is(newValue)) { + throw new Error(`Unsupported platform ${newValue}`); + } + + // Update platform and reset the API details. + setConfig({ + ...config, + platform: newValue, + ...defaultApiDetails, }); + }; const generalDetailsRenderOptions = generateGeneralDetails( - generalDetails, - onGeneralDetailsChange, + config, + onConfigChange, showValidationErrors, serverHasConfiguration, ); const apiDetailsRenderOptions = generateApiDetails( - apiDetails, - onApiDetailsChange, + config, + onConfigChange, showValidationErrors, // OKTA platform doesn't have client secret field which means there is no secret configuration in the server. - serverHasConfiguration && initialIdpDetails.platform !== "OKTA", + serverHasConfiguration && initialConfig.platform !== "OKTA", ); const handleOnSave = async () => { @@ -328,11 +300,11 @@ const OidcSettings = ({ pipe( checkValidations( generalDetailsRenderOptions, - R.fromEntries(Object.entries(generalDetails)), + R.fromEntries(Object.entries(config)), ) && checkValidations( apiDetailsRenderOptions, - R.fromEntries(Object.entries(apiDetails)), + R.fromEntries(Object.entries(config)), ) ? TE.right(undefined) : TE.left(checkForm), @@ -341,21 +313,15 @@ const OidcSettings = ({ const validateStructure = (): TE.TaskEither< string, OEQ.Oidc.IdentityProvider - > => { - const codec = - currentOidcValue.platform === "OKTA" - ? OEQ.Codec.Oidc.OktaCodec - : OEQ.Codec.Oidc.GenericIdentityProviderCodec; - - return pipe( - currentOidcValue, + > => + pipe( + currentConfig, E.fromPredicate( - codec.is, + OEQ.Codec.Oidc.IdentityProviderCodec.is, () => `Validation for the structure of OIDC configuration failed.`, ), TE.fromEither, ); - }; const submit = ( oidcValue: OEQ.Oidc.IdentityProvider, @@ -368,7 +334,7 @@ const OidcSettings = ({ TE.chain(submit), TE.match(appErrorHandler, () => { // Reset the initial values since user has saved the settings. - setInitialIdpDetails(currentOidcValue); + setInitialConfig(currentConfig); setShowSnackBar(true); }), )(); @@ -400,7 +366,7 @@ const OidcSettings = ({ title={apiDetailsTitle} desc={apiDetailsDesc} fields={{ - ...generatePlatform(generalDetails.platform, onPlatformChange), + ...generatePlatform(config.platform, onPlatformChange), ...apiDetailsRenderOptions, }} /> @@ -431,23 +397,23 @@ const OidcSettings = ({ secondaryText={roleClaimDesc} control={plainTextFiled({ name: roleClaimTitle, - value: generalDetails.roleConfig?.roleClaim, + value: config.roleConfig?.roleClaim, disabled: false, required: true, onChange: (value) => - setGeneralDetails({ - ...generalDetails, + setConfig({ + ...config, roleConfig: { roleClaim: value, customRoles: - generalDetails.roleConfig?.customRoles ?? new Map(), + config.roleConfig?.customRoles ?? new Map(), }, }), showValidationErrors, })} /> - {generalDetails.roleConfig?.roleClaim && ( + {config.roleConfig?.roleClaim && ( <> ([ ["OKTA", "Okta"], ]); -// Use 'platform' as the discriminator. -export interface GenericApiDetails - extends Pick< - OEQ.Oidc.GenericIdentityProvider, - "platform" | "apiUrl" | "apiClientId" | "apiClientSecret" - > {} - -export interface OkatApiDetails - extends Pick {} - -export type ApiDetails = GenericApiDetails | OkatApiDetails; +export const defaultApiDetails: OEQ.Oidc.RestApiDetails = { + apiUrl: "", + apiClientId: "", +}; -export const defaultGeneralDetails: OEQ.Oidc.IdentityProvider = { +export const defaultConfig: OEQ.Oidc.IdentityProvider = { enabled: false, platform: "ENTRA_ID", issuer: "", @@ -107,33 +100,7 @@ export const defaultGeneralDetails: OEQ.Oidc.IdentityProvider = { keysetUrl: "", tokenUrl: "", defaultRoles: new Set(), -}; - -export const defaultAuth0ApiDetails: GenericApiDetails = { - platform: "AUTH0", - apiUrl: "", - apiClientId: "", - apiClientSecret: "", -}; - -export const defaultEntraIdApiDetails: GenericApiDetails = { - ...defaultAuth0ApiDetails, - platform: "ENTRA_ID", -}; - -export const defaultOktaApiDetails: OkatApiDetails = { - platform: "OKTA", - apiUrl: "", - apiClientId: "", -}; - -export const defaultApiDetailsMap: Record< - OEQ.Oidc.IdentityProviderPlatform, - ApiDetails -> = { - ENTRA_ID: defaultEntraIdApiDetails, - AUTH0: defaultAuth0ApiDetails, - OKTA: defaultOktaApiDetails, + ...defaultApiDetails, }; const platformSelector = ( @@ -305,16 +272,12 @@ export const generateGeneralDetails = ( }, }); -const commonApiDetails = ( +const apiDetails = ( onChange: (key: string, value: unknown) => void, showValidationErrors: boolean, - apiDetails: ApiDetails, + { apiUrl, apiClientId, apiClientSecret }: OEQ.Oidc.IdentityProvider, isSecretConfigured: boolean, ): Record => { - const { platform, apiUrl, apiClientId } = apiDetails; - // apiClientSecret is not exist in OktaApiDetails. - const apiClientSecret = platform === "OKTA" ? "" : apiDetails.apiClientSecret; - return { apiUrl: { label: apiUrlLabel, @@ -390,23 +353,23 @@ export const generatePlatform = ( /** * Generate the render options for the API configuration of the selected identity providers. * - * @param apiDetails The value of the platform specific details. + * @param idpDetails Contains the value of the platform specific details. * @param apiDetailsOnChange Function to be called when a platform specific field is changed. * @param showValidationErrors Whether to show validation errors for each field. * @param isSecretConfigured Whether the server already has the API secrete. */ export const generateApiDetails = ( - apiDetails: ApiDetails, + idpDetails: OEQ.Oidc.IdentityProvider, apiDetailsOnChange: (key: string, value: unknown) => void, showValidationErrors: boolean, isSecretConfigured: boolean, ): Record => { - const platform = apiDetails.platform; + const platform = idpDetails.platform; - const apiCommonFields = commonApiDetails( + const apiCommonFields = apiDetails( apiDetailsOnChange, showValidationErrors, - apiDetails, + idpDetails, isSecretConfigured, ); const { apiUrl, apiClientId, apiClientSecret } = apiCommonFields;