Add a flag to disable API key readback

Author: bduffany

app/components/banner/BUILD

@@ -12,5 +12,6 @@ ts_library(
         "//:node_modules/lucide-react",
         "//:node_modules/react",
         "//:node_modules/tslib",
+        "//app/components/button",
     ],
 )

app/components/banner/banner.css

@@ -5,6 +5,10 @@
   align-items: start;
 }
 
+.banner .banner-content {
+  flex-grow: 1;
+}
+
 .banner.banner-info {
   background: var(--color-banner-info-bg);
   color: var(--color-banner-info-text);

app/components/banner/banner.tsx

@@ -1,5 +1,6 @@
-import { AlertCircle, CheckCircle, Info, XCircle } from "lucide-react";
+import { AlertCircle, CheckCircle, Info, X, XCircle } from "lucide-react";
 import React from "react";
+import { OutlinedButton } from "../button/button";
 
 const ICONS = {
   info: <Info className="icon blue" />,
@@ -13,18 +14,25 @@ type BannerType = keyof typeof ICONS;
 export type BannerProps = JSX.IntrinsicElements["div"] & {
   /** The banner type. */
   type: BannerType;
+  /** Called when the dismiss button is clicked. If null/undefined, no dismiss button is shown. */
+  onDismiss?: (() => void) | null;
 };
 
 /**
  * A banner shows a message that draws the attention of the user, using a
  * colorful icon and background.
  */
 export const Banner = React.forwardRef((props: BannerProps, ref: React.Ref<HTMLDivElement>) => {
-  const { type = "info", className, children, ...rest } = props;
+  const { type = "info", className, children, onDismiss, ...rest } = props;
   return (
     <div className={`banner banner-${type} ${className || ""}`} {...rest} ref={ref}>
       {ICONS[type]}
       <div className="banner-content">{children}</div>
+      {onDismiss && (
+        <OutlinedButton className="icon-button" onClick={onDismiss} title="Dismiss" type="button">
+          <X className="icon" />
+        </OutlinedButton>
+      )}
     </div>
   );
 });

enterprise/app/api_keys/BUILD

@@ -15,6 +15,7 @@ ts_library(
         "//app/alert:alert_service",
         "//app/auth:auth_service",
         "//app/capabilities",
+        "//app/components/banner",
         "//app/components/button",
         "//app/components/dialog",
         "//app/components/input",

enterprise/app/api_keys/api_keys.css

@@ -20,6 +20,12 @@
   padding: 8px 0;
 }
 
+.api-keys .api-key-created-banner-content {
+  display: flex;
+  flex-direction: column;
+  gap: 8px;
+}
+
 .api-keys .api-keys-list > :not(:last-child) {
   margin-bottom: 8px;
 }

enterprise/app/api_keys/api_keys.tsx

@@ -3,6 +3,7 @@ import React from "react";
 import alert_service from "../../../app/alert/alert_service";
 import { User } from "../../../app/auth/auth_service";
 import capabilities from "../../../app/capabilities/capabilities";
+import Banner from "../../../app/components/banner/banner";
 import FilledButton, { OutlinedButton } from "../../../app/components/button/button";
 import Dialog, {
   DialogBody,
@@ -42,6 +43,8 @@ interface State {
 
   updateForm: FormState<api_key.UpdateApiKeyRequest>;
 
+  createdApiKey: api_key.ApiKey | null;
+
   keyToDelete: api_key.ApiKey | null;
   isDeleteModalOpen: boolean;
   isDeleteModalSubmitting: boolean;
@@ -55,6 +58,8 @@ const INITIAL_STATE: State = {
 
   updateForm: newFormState(api_key.UpdateApiKeyRequest.create()),
 
+  createdApiKey: null,
+
   keyToDelete: null,
   isDeleteModalOpen: false,
   isDeleteModalSubmitting: false,
@@ -161,11 +166,14 @@ export default class ApiKeysComponent extends React.Component<ApiKeysComponentPr
 
     try {
       this.setState({ createForm: { ...this.state.createForm, isSubmitting: true } });
-      await this.props.create(
+      const response = await this.props.create(
         new api_key.CreateApiKeyRequest({
           ...this.state.createForm.request,
         })
       );
+      if (!this.apiKeyValueReadbackEnabled()) {
+        this.setState({ createdApiKey: response.apiKey ?? null });
+      }
     } catch (e) {
       this.setState({ createForm: { ...this.state.createForm, isSubmitting: false } });
       errorService.handleError(e);
@@ -255,6 +263,10 @@ export default class ApiKeysComponent extends React.Component<ApiKeysComponentPr
     onChange(e.target.name, e.target.value);
   }
 
+  private onDismissCreatedApiKey() {
+    this.setState({ createdApiKey: null });
+  }
+
   private onSelectReadOnly(onChange: (name: string, value: any) => any) {
     onChange("capability", []);
   }
@@ -301,6 +313,10 @@ export default class ApiKeysComponent extends React.Component<ApiKeysComponentPr
     return this.props.userOwnedOnly || this.props.user.canCall("updateApiKey");
   }
 
+  private apiKeyValueReadbackEnabled(): boolean {
+    return capabilities.config.apiKeyValueReadbackEnabled !== false;
+  }
+
   private renderModal<T extends ApiKeyFields>({
     title,
     submitLabel,
@@ -458,7 +474,16 @@ export default class ApiKeysComponent extends React.Component<ApiKeysComponentPr
   render() {
     if (!this.props.user) return <></>;
 
-    const { keyToDelete, createForm, updateForm, getApiKeysResponse, isDeleteModalOpen, initialLoadError } = this.state;
+    const {
+      keyToDelete,
+      createForm,
+      updateForm,
+      createdApiKey,
+      getApiKeysResponse,
+      isDeleteModalOpen,
+      initialLoadError,
+    } = this.state;
+    const apiKeyValueReadbackEnabled = this.apiKeyValueReadbackEnabled();
 
     if (!getApiKeysResponse) {
       return (
@@ -484,6 +509,17 @@ export default class ApiKeysComponent extends React.Component<ApiKeysComponentPr
             </FilledButton>
           </div>
         )}
+        {createdApiKey?.value && (
+          <Banner type="info" className="api-key-created-banner" onDismiss={this.onDismissCreatedApiKey.bind(this)}>
+            <div className="api-key-created-banner-content">
+              <div>
+                Created API key <b title={createdApiKey.label || undefined}>{createdApiKey.label || "Untitled key"}</b>
+                {apiKeyValueReadbackEnabled ? "." : " (save this value - you will not be able to access it later)."}
+              </div>
+              <ApiKeyField apiKey={createdApiKey} />
+            </div>
+          </Banner>
+        )}
 
         {this.renderModal({
           title: "New API key",
@@ -524,7 +560,7 @@ export default class ApiKeysComponent extends React.Component<ApiKeysComponentPr
                 title={key.visibleToDevelopers ? "Visible to non-admin members of this organization" : undefined}>
                 <span>{describeCapabilities(key)}</span>
               </div>
-              <ApiKeyField apiKey={key} />
+              {apiKeyValueReadbackEnabled && <ApiKeyField apiKey={key} />}
               {this.props.user.canCall(this.props.userOwnedOnly ? "updateUserApiKey" : "updateApiKey") && (
                 <OutlinedButton className="api-key-edit-button" onClick={this.onClickUpdate.bind(this, key)}>
                   Edit

enterprise/app/settings/settings.tsx

@@ -104,6 +104,7 @@ export default class SettingsComponent extends React.Component<SettingsProps> {
     }
 
     const activeTabId = this.getActiveTabId();
+    const apiKeyValueReadbackEnabled = capabilities.config.apiKeyValueReadbackEnabled !== false;
 
     return (
       <div className="settings">
@@ -305,6 +306,12 @@ export default class SettingsComponent extends React.Component<SettingsProps> {
                       <div className="settings-option-description">
                         API keys grant access to your BuildBuddy organization.
                       </div>
+                      {!apiKeyValueReadbackEnabled && (
+                        <div className="settings-option-description">
+                          API keys cannot be viewed from this page. If you've lost access to an API key value, you can
+                          delete and recreate the key with the same capabilities.
+                        </div>
+                      )}
                       {this.isCLILogin() && (
                         <>
                           <div className="settings-option-description" debug-id="cli-login-settings-banner">
@@ -340,6 +347,12 @@ export default class SettingsComponent extends React.Component<SettingsProps> {
                           Personal API keys associate builds with both your individual user account and your
                           organization.
                         </div>
+                        {!apiKeyValueReadbackEnabled && (
+                          <div className="settings-option-description">
+                            API keys cannot be viewed from this page. If you've lost access to an API key value, you can
+                            delete and recreate the key with the same capabilities.
+                          </div>
+                        )}
                         {this.isCLILogin() && (
                           <div className="settings-option-description">
                             <Banner type="info">

enterprise/server/backends/authdb/authdb.go

@@ -55,11 +55,12 @@ const (
 )
 
 var (
-	userOwnedKeysEnabled = flag.Bool("app.user_owned_keys_enabled", false, "If true, enable user-owned API keys.")
-	apiKeyGroupCacheTTL  = flag.Duration("auth.api_key_group_cache_ttl", 5*time.Minute, "TTL for API Key to Group caching. Set to '0' to disable cache.")
-	apiKeyEncryptionKey  = flag.String("auth.api_key_encryption.key", "", "Base64-encoded 256-bit encryption key for API keys.", flag.Secret)
-	encryptNewKeys       = flag.Bool("auth.api_key_encryption.encrypt_new_keys", false, "If enabled, all new API keys will be written in an encrypted format.")
-	encryptOldKeys       = flag.Bool("auth.api_key_encryption.encrypt_old_keys", false, "If enabled, all existing unencrypted keys will be encrypted on startup. The unencrypted keys will remain in the database and will need to be cleared manually after verifying the success of the migration.")
+	userOwnedKeysEnabled      = flag.Bool("app.user_owned_keys_enabled", false, "If true, enable user-owned API keys.")
+	apiKeyValueReadbackConfig = flag.Bool("app.api_key_value_readback_enabled", true, "If true, API key values can be retrieved after creation via API key read methods.")
+	apiKeyGroupCacheTTL       = flag.Duration("auth.api_key_group_cache_ttl", 5*time.Minute, "TTL for API Key to Group caching. Set to '0' to disable cache.")
+	apiKeyEncryptionKey       = flag.String("auth.api_key_encryption.key", "", "Base64-encoded 256-bit encryption key for API keys.", flag.Secret)
+	encryptNewKeys            = flag.Bool("auth.api_key_encryption.encrypt_new_keys", false, "If enabled, all new API keys will be written in an encrypted format.")
+	encryptOldKeys            = flag.Bool("auth.api_key_encryption.encrypt_old_keys", false, "If enabled, all existing unencrypted keys will be encrypted on startup. The unencrypted keys will remain in the database and will need to be cleared manually after verifying the success of the migration.")
 )
 
 type apiKeyGroupCacheEntry struct {
@@ -361,6 +362,10 @@ func (d *AuthDB) fillDecryptedAPIKey(ak *tables.APIKey) error {
 	return nil
 }
 
+func apiKeyValueReadbackEnabled() bool {
+	return *apiKeyValueReadbackConfig
+}
+
 func (d *AuthDB) fillChildGroupIDs(ctx context.Context, akg *apiKeyGroup) error {
 	// If the group is not designated as a parent then don't bother doing a
 	// query.
@@ -839,8 +844,10 @@ func (d *AuthDB) GetAPIKey(ctx context.Context, apiKeyID string) (*tables.APIKey
 		}
 	}
 
-	if err := d.fillDecryptedAPIKey(key); err != nil {
-		return nil, err
+	if apiKeyValueReadbackEnabled() {
+		if err := d.fillDecryptedAPIKey(key); err != nil {
+			return nil, err
+		}
 	}
 	return key, nil
 }
@@ -917,8 +924,10 @@ func (d *AuthDB) GetAPIKeys(ctx context.Context, groupID string) ([]*tables.APIK
 
 	keys := make([]*tables.APIKey, 0)
 	err = db.ScanEach(rq, func(ctx context.Context, k *tables.APIKey) error {
-		if err := d.fillDecryptedAPIKey(k); err != nil {
-			return err
+		if apiKeyValueReadbackEnabled() {
+			if err := d.fillDecryptedAPIKey(k); err != nil {
+				return err
+			}
 		}
 		keys = append(keys, k)
 		return nil
@@ -1064,8 +1073,10 @@ func (d *AuthDB) GetUserAPIKeys(ctx context.Context, userID, groupID string) ([]
 	rq := d.h.NewQuery(ctx, "authdb_get_user_api_keys").Raw(queryStr, args...)
 	var keys []*tables.APIKey
 	err = db.ScanEach(rq, func(ctx context.Context, k *tables.APIKey) error {
-		if err := d.fillDecryptedAPIKey(k); err != nil {
-			return err
+		if apiKeyValueReadbackEnabled() {
+			if err := d.fillDecryptedAPIKey(k); err != nil {
+				return err
+			}
 		}
 		keys = append(keys, k)
 		return nil
@@ -1076,3 +1087,9 @@ func (d *AuthDB) GetUserAPIKeys(ctx context.Context, userID, groupID string) ([]
 func (d *AuthDB) GetUserOwnedKeysEnabled() bool {
 	return *userOwnedKeysEnabled
 }
+
+// GetAPIKeyValueReadbackEnabled returns whether API key values can be fetched
+// via API key read methods after creation.
+func (d *AuthDB) GetAPIKeyValueReadbackEnabled() bool {
+	return apiKeyValueReadbackEnabled()
+}

enterprise/server/backends/authdb/authdb_test.go

@@ -281,6 +281,71 @@ func TestUserKeyExpiration(t *testing.T) {
 	require.NotContains(t, apiKeyIDs(keys), rsp.GetApiKey().GetId())
 }
 
+func TestAPIKeyValueReadbackDisabled(t *testing.T) {
+	flags.Set(t, "app.api_key_value_readback_enabled", false)
+
+	ctx := context.Background()
+	env := setupEnv(t)
+
+	users := enterprise_testauth.CreateRandomGroups(t, env)
+	// Get a random admin user.
+	var admin *tables.User
+	for _, u := range users {
+		if u.Groups[0].HasCapability(cappb.Capability_ORG_ADMIN) {
+			admin = u
+			break
+		}
+	}
+	require.NotNil(t, admin)
+
+	auth := env.GetAuthenticator().(*testauth.TestAuthenticator)
+	adminCtx, err := auth.WithAuthenticatedUser(ctx, admin.UserID)
+	require.NoError(t, err)
+
+	groupID := admin.Groups[0].Group.GroupID
+
+	orgKeyRsp, err := env.GetBuildBuddyServer().CreateApiKey(adminCtx, &akpb.CreateApiKeyRequest{
+		RequestContext: &ctxpb.RequestContext{GroupId: groupID},
+		Label:          "org-key",
+	})
+	require.NoError(t, err)
+	require.NotEmpty(t, orgKeyRsp.GetApiKey().GetValue())
+	// Key remains usable for authentication even when API key value readback is disabled.
+	_, err = env.GetAuthDB().GetAPIKeyGroupFromAPIKey(ctx, orgKeyRsp.GetApiKey().GetValue())
+	require.NoError(t, err)
+
+	orgKeyGetRsp, err := env.GetBuildBuddyServer().GetApiKey(adminCtx, &akpb.GetApiKeyRequest{
+		ApiKeyId: orgKeyRsp.GetApiKey().GetId(),
+	})
+	require.NoError(t, err)
+	// Read APIs should still return metadata, but the key value should be omitted.
+	require.Empty(t, orgKeyGetRsp.GetApiKey().GetValue())
+
+	// Enable user-owned keys.
+	g, err := env.GetUserDB().GetGroupByID(adminCtx, groupID)
+	require.NoError(t, err)
+	g.UserOwnedKeysEnabled = true
+	_, err = env.GetUserDB().UpdateGroup(adminCtx, g)
+	require.NoError(t, err)
+
+	userKeyRsp, err := env.GetBuildBuddyServer().CreateUserApiKey(adminCtx, &akpb.CreateApiKeyRequest{
+		RequestContext: &ctxpb.RequestContext{GroupId: groupID},
+		Label:          "user-key",
+	})
+	require.NoError(t, err)
+	require.NotEmpty(t, userKeyRsp.GetApiKey().GetValue())
+	// User-owned key remains usable for authentication even when API key value readback is disabled.
+	_, err = env.GetAuthDB().GetAPIKeyGroupFromAPIKey(ctx, userKeyRsp.GetApiKey().GetValue())
+	require.NoError(t, err)
+
+	userKeyGetRsp, err := env.GetBuildBuddyServer().GetUserApiKey(adminCtx, &akpb.GetApiKeyRequest{
+		ApiKeyId: userKeyRsp.GetApiKey().GetId(),
+	})
+	require.NoError(t, err)
+	// Same behavior for user-owned keys: metadata is returned, value is omitted.
+	require.Empty(t, userKeyGetRsp.GetApiKey().GetValue())
+}
+
 func TestGetAPIKeyGroupFromAPIKey(t *testing.T) {
 	for _, encrypt := range []bool{false, true} {
 		t.Run(fmt.Sprintf("encrypt_%t", encrypt), func(t *testing.T) {

proto/config.proto

@@ -196,6 +196,12 @@ message FrontendConfig {
 
   // Whether dark mode UI is enabled.
   bool dark_mode_enabled = 65;
+
+  // Whether API key values can be retrieved after they are created.
+  // If false, API key metadata should still be accessible, but values should
+  // be omitted from read APIs.
+  // If this field is absent, clients should treat it as true.
+  optional bool api_key_value_readback_enabled = 66;
 }
 
 message Region {