From 63f7e103af76a4d33bfa02a01a3448b7dc4c86ee Mon Sep 17 00:00:00 2001
From: Tung Wu <tung@oursky.com>
Date: Thu, 9 Jan 2025 12:26:59 +0800
Subject: [PATCH] Use index to reference existing sso provider because alias
 can be changed

---
 .../AddSingleSignOnConfigurationScreen.tsx    |  8 +--
 .../EditSingleSignOnConfigurationScreen.tsx   |  9 ++--
 .../SingleSignOnConfigurationWidget.tsx       | 54 ++++++++++---------
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/portal/src/graphql/portal/AddSingleSignOnConfigurationScreen.tsx b/portal/src/graphql/portal/AddSingleSignOnConfigurationScreen.tsx
index 25f2a5b0f0..ba0d2485ad 100644
--- a/portal/src/graphql/portal/AddSingleSignOnConfigurationScreen.tsx
+++ b/portal/src/graphql/portal/AddSingleSignOnConfigurationScreen.tsx
@@ -85,16 +85,16 @@ function generateNewAlias(
 }
 
 interface OAuthClientFormProps {
-  alias: string;
+  initialAlias: string;
   providerItemKey: OAuthSSOProviderItemKey;
   form: OAuthProviderFormModel;
 }
 
 const OAuthClientForm: React.VFC<OAuthClientFormProps> =
   function OAuthClientForm(props) {
-    const { alias, providerItemKey, form } = props;
+    const { initialAlias, providerItemKey, form } = props;
     const widgetProps = useSingleSignOnConfigurationWidget(
-      alias,
+      initialAlias,
       providerItemKey,
       form
     );
@@ -174,7 +174,7 @@ const AddSingleSignOnConfigurationContent: React.VFC =
           <ShowOnlyIfSIWEIsDisabled>
             {newAlias != null && selectedProviderKey != null ? (
               <OAuthClientForm
-                alias={newAlias}
+                initialAlias={newAlias}
                 form={form}
                 providerItemKey={selectedProviderKey}
               />
diff --git a/portal/src/graphql/portal/EditSingleSignOnConfigurationScreen.tsx b/portal/src/graphql/portal/EditSingleSignOnConfigurationScreen.tsx
index 697dc096d6..cc9e2583b8 100644
--- a/portal/src/graphql/portal/EditSingleSignOnConfigurationScreen.tsx
+++ b/portal/src/graphql/portal/EditSingleSignOnConfigurationScreen.tsx
@@ -41,7 +41,7 @@ function isLocationState(raw: unknown): raw is LocationState {
 }
 
 interface OAuthClientItemProps {
-  alias: string;
+  initialAlias: string;
   providerItemKey: OAuthSSOProviderItemKey;
   form: OAuthProviderFormModel;
   oauthSSOFeatureConfig?: OAuthSSOFeatureConfig;
@@ -49,9 +49,10 @@ interface OAuthClientItemProps {
 
 const OAuthClientItem: React.VFC<OAuthClientItemProps> =
   function OAuthClientItem(props) {
-    const { alias, providerItemKey, form, oauthSSOFeatureConfig } = props;
+    const { initialAlias, providerItemKey, form, oauthSSOFeatureConfig } =
+      props;
     const widgetProps = useSingleSignOnConfigurationWidget(
-      alias,
+      initialAlias,
       providerItemKey,
       form,
       oauthSSOFeatureConfig
@@ -107,7 +108,7 @@ const EditSingleSignOnConfigurationContent: React.VFC<EditSingleSignOnConfigurat
       >
         <ShowOnlyIfSIWEIsDisabled>
           <OAuthClientItem
-            alias={alias}
+            initialAlias={alias}
             providerItemKey={providerItemKey}
             form={form}
             oauthSSOFeatureConfig={oauthSSOFeatureConfig}
diff --git a/portal/src/graphql/portal/SingleSignOnConfigurationWidget.tsx b/portal/src/graphql/portal/SingleSignOnConfigurationWidget.tsx
index b67aff6fa2..58203de581 100644
--- a/portal/src/graphql/portal/SingleSignOnConfigurationWidget.tsx
+++ b/portal/src/graphql/portal/SingleSignOnConfigurationWidget.tsx
@@ -2,7 +2,7 @@ import { Checkbox, DirectionalHint, Label, Text } from "@fluentui/react";
 import { Context, FormattedMessage } from "@oursky/react-messageformat";
 import cn from "classnames";
 import { produce } from "immer";
-import React, { useCallback, useContext, useMemo } from "react";
+import React, { useCallback, useContext, useMemo, useState } from "react";
 import FormTextField from "../../FormTextField";
 import {
   createOAuthSSOProviderItemKey,
@@ -251,7 +251,7 @@ const OAuthClientIcon: React.VFC<OAuthClientIconProps> =
   };
 
 export function useSingleSignOnConfigurationWidget(
-  alias: string,
+  initialAlias: string,
   providerItemKey: OAuthSSOProviderItemKey,
   form: OAuthProviderFormModel,
   oauthSSOFeatureConfig?: OAuthSSOFeatureConfig
@@ -263,6 +263,19 @@ export function useSingleSignOnConfigurationWidget(
 
   const [providerType, appType] = parseOAuthSSOProviderItemKey(providerItemKey);
 
+  const [providerIndex] = useState<number>(() => {
+    const existingIndex = providers.findIndex((p) =>
+      isOAuthSSOProvider(p.config, providerType, initialAlias, appType)
+    );
+    if (existingIndex !== -1) {
+      return existingIndex;
+    }
+    // Insert at the end if it does not exist
+    return providers.length;
+  });
+
+  console.log("providerIndex", providerIndex);
+
   const disabled = useMemo(() => {
     const providersConfig = oauthSSOFeatureConfig?.providers ?? {};
     const providerConfig = providersConfig[
@@ -275,41 +288,32 @@ export function useSingleSignOnConfigurationWidget(
     const newConfig = {
       config: {
         type: providerType,
-        alias: alias,
+        alias: initialAlias,
         ...(appType && { app_type: appType }),
       },
       secret: {
         originalAlias: null,
-        newAlias: alias,
+        newAlias: initialAlias,
         newClientSecret: "",
       },
     } satisfies SSOProviderFormState;
-    return (
-      providers.find((p) =>
-        isOAuthSSOProvider(p.config, providerType, alias, appType)
-      ) ?? newConfig
-    );
-  }, [providers, providerType, alias, appType]);
+    return providers.length > providerIndex
+      ? providers[providerIndex]
+      : newConfig;
+  }, [providerType, initialAlias, appType, providers, providerIndex]);
 
-  const index = providers.findIndex((p) =>
-    isOAuthSSOProvider(p.config, providerType, alias, appType)
-  );
   const jsonPointer = useMemo(() => {
-    return index >= 0 ? `/identity/oauth/providers/${index}` : "";
-  }, [index]);
-  const clientSecretParentJsonPointer =
-    index >= 0
-      ? new RegExp(`/secrets/\\d+/data/items/${index}`)
-      : /placeholder/;
+    return `/identity/oauth/providers/${providerIndex}`;
+  }, [providerIndex]);
+  const clientSecretParentJsonPointer = new RegExp(
+    `/secrets/\\d+/data/items/${providerIndex}`
+  );
 
   const onChange = useCallback(
     (config: OAuthSSOProviderConfig, secret: SSOProviderFormSecretViewModel) =>
       setState((state) =>
         produce(state, (state) => {
-          const existingIdx = state.providers.findIndex((p) =>
-            isOAuthSSOProvider(p.config, providerType, alias, appType)
-          );
-          if (existingIdx === -1) {
+          if (providerIndex === -1) {
             state.providers.push({
               config,
               secret: {
@@ -319,7 +323,7 @@ export function useSingleSignOnConfigurationWidget(
               },
             });
           } else {
-            state.providers[existingIdx] = {
+            state.providers[providerIndex] = {
               config,
               secret: {
                 originalAlias: secret.originalAlias,
@@ -330,7 +334,7 @@ export function useSingleSignOnConfigurationWidget(
           }
         })
       ),
-    [setState, providerType, alias, appType]
+    [setState, providerIndex]
   );
 
   return {