move API key to server-side

Author: riceyrice

flagsmith-jira-app/src/backend/flagsmith.ts

@@ -1,6 +1,7 @@
 import api, { APIResponse, Route, assumeTrustedRoute, route } from "@forge/api";
 
 import { ApiArgs, ApiError, FLAGSMITH_API_V1 } from "../common";
+import { readApiKey } from "./storage";
 
 type Model = {
   id: number;
@@ -53,7 +54,7 @@ export type EnvironmentFeatureState = Model & {
 const flagsmithApi = async (
   apiKey: string,
   route: Route,
-  { method = "GET", headers, body, codes = [], jsonResponse = true }: ApiArgs = {}
+  { method = "GET", headers, body, codes = [], jsonResponse = true }: ApiArgs = {},
 ): Promise<unknown> => {
   try {
     const url = `${FLAGSMITH_API_V1}${route.value}`;
@@ -84,13 +85,16 @@ const flagsmithApi = async (
 const checkResponse = (response: APIResponse, ...codes: number[]): void => {
   if (!response.ok && !codes.includes(response.status)) {
     console.warn(response.status, response.statusText);
-    throw new ApiError("Unexpected Flagsmith API response:", response.status);
+    throw new ApiError(
+      `Unexpected Flagsmith API response: ${response.statusText}`,
+      response.status,
+    );
   }
 };
 
 const unpaginate = async <TModel extends Model>(
   apiKey: string,
-  data: PaginatedModels<TModel>
+  data: PaginatedModels<TModel>,
 ): Promise<TModel[]> => {
   let pageData = data;
   const results = pageData?.results ?? [];
@@ -103,13 +107,14 @@ const unpaginate = async <TModel extends Model>(
 };
 
 const checkApiKey = (apiKey: string): void => {
-  if (!apiKey) throw new ApiError("Flagsmith API Key not configured", 400);
+  if (!apiKey) throw new ApiError("Flagsmith API Key not set", 400);
 };
 
-export type ReadOrganisations = (args: { apiKey: string }) => Promise<Organisation[]>;
+export type ReadOrganisations = (args: { apiKey?: string }) => Promise<Organisation[]>;
 
-/** Read Flagsmith Organisations for given API Key */
-export const readOrganisations: ReadOrganisations = async ({ apiKey }) => {
+/** Read Flagsmith Organisations for stored/given API Key */
+export const readOrganisations: ReadOrganisations = async ({ apiKey: givenApiKey }) => {
+  const apiKey = givenApiKey ?? (await readApiKey());
   checkApiKey(apiKey);
   const path = route`/organisations/`;
   const data = (await flagsmithApi(apiKey, path)) as PaginatedModels<Organisation>;
@@ -118,15 +123,13 @@ export const readOrganisations: ReadOrganisations = async ({ apiKey }) => {
   return results;
 };
 
-export type ReadProjects = (args: {
-  apiKey: string;
-  organisationId: string | undefined;
-}) => Promise<Project[]>;
+export type ReadProjects = (args: { organisationId: string | undefined }) => Promise<Project[]>;
 
-/** Read Flagsmith Projects for given API Key and Organisation ID */
-export const readProjects: ReadProjects = async ({ apiKey, organisationId }) => {
+/** Read Flagsmith Projects for stored API Key and given Organisation ID */
+export const readProjects: ReadProjects = async ({ organisationId }) => {
+  const apiKey = await readApiKey();
   checkApiKey(apiKey);
-  if (!organisationId) throw new ApiError("Flagsmith organisation not configured", 400);
+  if (!organisationId) throw new ApiError("Flagsmith organisation not connected", 400);
   const path = route`/projects/?organisation=${organisationId}`;
   const data = (await flagsmithApi(apiKey, path)) as Project[];
   // do not unpaginate as this API does not do pagination
@@ -135,31 +138,27 @@ export const readProjects: ReadProjects = async ({ apiKey, organisationId }) =>
   return results;
 };
 
-export type ReadEnvironments = (args: {
-  apiKey: string;
-  projectId: string | undefined;
-}) => Promise<Environment[]>;
+export type ReadEnvironments = (args: { projectId: string | undefined }) => Promise<Environment[]>;
 
-/** Read Flagsmith Environments for given API Key and Project ID */
-export const readEnvironments: ReadEnvironments = async ({ apiKey, projectId }) => {
+/** Read Flagsmith Environments for stored API Key and given Project ID */
+export const readEnvironments: ReadEnvironments = async ({ projectId }) => {
+  const apiKey = await readApiKey();
   checkApiKey(apiKey);
-  if (!projectId) throw new ApiError("Flagsmith project not configured", 400);
+  if (!projectId) throw new ApiError("Flagsmith project not connected", 400);
   const path = route`/environments/?project=${projectId}`;
   const data = (await flagsmithApi(apiKey, path)) as PaginatedModels<Environment>;
   const results = await unpaginate(apiKey, data);
   if (results.length === 0) throw new ApiError("Flagsmith project has no environments", 404);
   return results;
 };
 
-export type ReadFeatures = (args: {
-  apiKey: string;
-  projectId: string | undefined;
-}) => Promise<Feature[]>;
+export type ReadFeatures = (args: { projectId: string | undefined }) => Promise<Feature[]>;
 
-/** Read Flagsmith Features for given API Key and Project ID */
-export const readFeatures: ReadFeatures = async ({ apiKey, projectId }) => {
+/** Read Flagsmith Features for stored API Key and given Project ID */
+export const readFeatures: ReadFeatures = async ({ projectId }) => {
+  const apiKey = await readApiKey();
   checkApiKey(apiKey);
-  if (!projectId) throw new ApiError("Flagsmith project not configured", 400);
+  if (!projectId) throw new ApiError("Flagsmith project not connected", 400);
   const path = route`/projects/${projectId}/features/`;
   const data = (await flagsmithApi(apiKey, path)) as PaginatedModels<Feature>;
   // ignore archived features
@@ -169,19 +168,18 @@ export const readFeatures: ReadFeatures = async ({ apiKey, projectId }) => {
 };
 
 export type ReadEnvironmentFeatureState = (args: {
-  apiKey: string;
   envApiKey: string;
   featureName: string;
 }) => Promise<EnvironmentFeatureState>;
 
-/** Read Flagsmith Feature State for given API Key, Environment API Key and Feature Name */
+/** Read Flagsmith Feature State for stored API Key and given Environment API Key and Feature Name */
 export const readEnvironmentFeatureState: ReadEnvironmentFeatureState = async ({
-  apiKey,
   envApiKey,
   featureName,
 }) => {
+  const apiKey = await readApiKey();
   checkApiKey(apiKey);
-  if (!envApiKey) throw new ApiError("Flagsmith environment not configured", 400);
+  if (!envApiKey) throw new ApiError("Flagsmith environment not connected", 400);
   const path = route`/environments/${envApiKey}/featurestates/?feature_name=${featureName}`;
   const data = (await flagsmithApi(apiKey, path)) as PaginatedModels<EnvironmentFeatureState>;
   const results = await unpaginate(apiKey, data);

flagsmith-jira-app/src/frontend/components/AppSettingsPage.tsx

@@ -19,19 +19,22 @@ import {
 import React, { Fragment, useEffect, useId, useMemo, useState } from "react";
 
 import { ApiError, usePromise } from "../../common";
-import { readOrganisations } from "../flagsmith";
+import { readOrganisations, readProjects } from "../flagsmith";
 import {
   deleteApiKey,
   deleteOrganisationId,
-  readApiKey,
+  readHasApiKey,
   readOrganisationId,
   writeApiKey,
   writeOrganisationId,
 } from "../storage";
 import { WrappableComponentProps } from "./ErrorWrapper";
 
+// 40 chars, same length as API key
+const SENTINEL = "****************************************";
+
 type AppSettingsFormProps = WrappableComponentProps & {
-  apiKey: string | null;
+  hasApiKey: boolean;
   saveApiKey: (apiKey: string) => Promise<void>;
   organisationId: string | null;
   saveOrganisationId: (organisationId: string) => Promise<void>;
@@ -44,10 +47,10 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
   ...props
 }) => {
   // set form state from props
-  const [apiKey, setApiKey] = useState<string | null>(props.apiKey);
+  const [apiKey, setApiKey] = useState<string | null>(props.hasApiKey ? SENTINEL : null);
   useEffect(() => {
-    setApiKey(props.apiKey);
-  }, [props.apiKey]);
+    setApiKey(props.hasApiKey ? SENTINEL : null);
+  }, [props.hasApiKey]);
 
   const [organisationId, setOrganisationId] = useState<string | null>(props.organisationId);
   useEffect(() => {
@@ -59,7 +62,8 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
     async () => {
       try {
         if (apiKey && apiKey.length === 40) {
-          return await readOrganisations({ apiKey });
+          // use stored API key if current value is sentinel i.e. unchanged
+          return await readOrganisations(apiKey === SENTINEL ? {} : { apiKey });
         }
       } catch (error) {
         // ignore 401 (invalid API key) and 404 (no organisations) as that is handled by the form
@@ -74,11 +78,12 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
       return [];
     },
     [apiKey],
-    setError
+    setError,
   );
+
   const organisation = organisations?.find((each) => String(each.id) === String(organisationId));
   const currentOrganisation = organisations?.find(
-    (each) => String(each.id) === String(props.organisationId)
+    (each) => String(each.id) === String(props.organisationId),
   );
 
   // update organisationId when organisations change
@@ -95,6 +100,21 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
     }
   }, [organisations, organisation]);
 
+  // get projects for current organisation from Flagsmith API
+  const [projects] = usePromise(async () => {
+    try {
+      if (props.organisationId) {
+        return await readProjects({ organisationId: props.organisationId });
+      } else {
+        return undefined;
+      }
+    } catch (error) {
+      // treat errors as "no projects"
+      console.error(error);
+      return [];
+    }
+  }, [props.organisationId]);
+
   const apiKeyInputId = useId();
 
   const organisationInputId = useId();
@@ -104,7 +124,7 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
         label: each.name,
         value: String(each.id),
       })),
-    [organisations]
+    [organisations],
   );
 
   const organisationValue =
@@ -115,7 +135,7 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
   }
 
   const onSave = async () => {
-    await saveApiKey(apiKey ?? "");
+    if (apiKey !== SENTINEL) await saveApiKey(apiKey ?? "");
     await saveOrganisationId(organisationId ?? "");
   };
 
@@ -128,21 +148,28 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
     setOrganisationId("");
   };
 
-  const apiKeyInvalid = props.apiKey?.length === 40 && organisations?.length === 0;
-  const configured = !!props.apiKey && !!currentOrganisation;
+  // this status is a bit confusing as it reflects the current form state
+  // rather than keeping track of whether the saved API key and organisation ID match each other
+  const apiKeyInvalid = props.hasApiKey && organisations?.length === 0;
+  const configured = props.hasApiKey && !!currentOrganisation && projects && projects.length > 0;
 
   return (
     <Fragment>
       <Box xcss={{ marginBottom: "space.300" }}>
         <Inline space="space.050" alignBlock="center">
           <Strong>Organisation:</Strong>{" "}
           {!!currentOrganisation && <Text>{currentOrganisation.name}</Text>}
-          {!configured && !apiKeyInvalid && <Lozenge appearance="moved">Not connected</Lozenge>}
+          {!apiKeyInvalid && !currentOrganisation && (
+            <Lozenge appearance="moved">Not connected</Lozenge>
+          )}
           {apiKeyInvalid && (
             <Lozenge appearance="removed" maxWidth={"15rem"}>
               Invalid key or no organisations
             </Lozenge>
           )}
+          {!apiKeyInvalid && !!currentOrganisation && !!projects && !configured && (
+            <Lozenge appearance="removed">No projects to connect</Lozenge>
+          )}
           {configured && <Lozenge appearance="success">Connected</Lozenge>}
         </Inline>
       </Box>
@@ -194,7 +221,13 @@ const AppSettingsForm: React.FC<AppSettingsFormProps> = ({
 
 const AppSettingsPage: React.FC<WrappableComponentProps> = ({ setError }) => {
   // get configuration from storage
-  const [apiKey, setApiKey] = usePromise(readApiKey, [], setError);
+  const [hasApiKey, setHasApiKey] = usePromise(
+    async () => {
+      return await readHasApiKey();
+    },
+    [],
+    setError,
+  );
   const [organisationId, setOrganisationId] = usePromise(readOrganisationId, [], setError);
 
   /** Write API Key to storage and update form state */
@@ -204,7 +237,7 @@ const AppSettingsPage: React.FC<WrappableComponentProps> = ({ setError }) => {
     } else {
       await deleteApiKey();
     }
-    setApiKey(apiKey);
+    setHasApiKey(!!apiKey);
   };
 
   /** Write Organisation ID to storage and update form state */
@@ -217,12 +250,12 @@ const AppSettingsPage: React.FC<WrappableComponentProps> = ({ setError }) => {
     setOrganisationId(organisationId);
   };
 
-  const ready = apiKey !== undefined && organisationId !== undefined;
+  const ready = hasApiKey !== undefined && organisationId !== undefined;
 
   return ready ? (
     <AppSettingsForm
       setError={setError}
-      apiKey={apiKey}
+      hasApiKey={hasApiKey}
       saveApiKey={saveApiKey}
       organisationId={organisationId}
       saveOrganisationId={saveOrganisationId}

flagsmith-jira-app/src/frontend/components/ErrorWrapper.tsx

@@ -47,7 +47,7 @@ const ErrorWrapper: React.FC<ErrorWrapperProps> = ({
       <SectionMessage title={error.message ?? "Something went wrong"} appearance={appearance}>
         <Text>{advice}</Text>
       </SectionMessage>
-      <Box xcss={{ marginTop: "space.300" }}>
+      <Box xcss={{ marginTop: "space.200" }}>
         <Stack alignInline="end">
           <ButtonGroup label="Actions">
             <Button onClick={() => setError(undefined)}>Retry</Button>