Upgrade secrets manager (#75)

* Use the field visibility property from the secrets manager

* Remove the settings to use the secret manager if not provided

Author: brichet

package.json

@@ -83,7 +83,7 @@
         "@rjsf/utils": "^5.18.4",
         "@rjsf/validator-ajv8": "^5.18.4",
         "json5": "^2.2.3",
-        "jupyter-secrets-manager": "^0.3.0",
+        "jupyter-secrets-manager": "^0.4.0",
         "react": "^18.2.0",
         "react-dom": "^18.2.0"
     },

schema/provider-registry.json

@@ -11,12 +11,6 @@
       "description": "Whether to use or not the secrets manager. If not, secrets will be stored in the browser (local storage)",
       "default": true
     },
-    "HideSecretFields": {
-      "type": "boolean",
-      "title": "Hide secret fields",
-      "description": "Whether to hide the secret fields in the UI or not",
-      "default": true
-    },
     "AIprovider": {
       "type": "object",
       "title": "AI provider",

src/index.ts

@@ -191,6 +191,9 @@ const providerRegistryPlugin: JupyterFrontEndPlugin<IAIProviderRegistry> =
       settingRegistry
         .load(providerRegistryPlugin.id)
         .then(settings => {
+          if (!secretsManager) {
+            delete settings.schema.properties?.['UseSecretsManager'];
+          }
           const updateProvider = () => {
             // Update the settings to the AI providers.
             const providerSettings = (settings.get('AIprovider').composite ?? {

src/settings/panel.tsx

@@ -72,10 +72,10 @@ export class AiSettings extends React.Component<
     this._settingConnector = props.formContext.settingConnector ?? null;
     this._settings = props.formContext.settings;
 
-    this._useSecretsManager =
+    const useSecretsManagerSetting =
       (this._settings.get('UseSecretsManager').composite as boolean) ?? true;
-    this._hideSecretFields =
-      (this._settings.get('HideSecretFields').composite as boolean) ?? true;
+    this._useSecretsManager =
+      useSecretsManagerSetting && this._secretsManager !== null;
 
     // Initialize the providers schema.
     const providerSchema = JSONExt.deepCopy(baseSettings) as any;
@@ -124,19 +124,11 @@ export class AiSettings extends React.Component<
     // Update the setting registry.
     this.saveSettingsToRegistry();
 
-    this._settings.changed.connect(() => {
-      const useSecretsManager =
-        (this._settings.get('UseSecretsManager').composite as boolean) ?? true;
-      if (useSecretsManager !== this._useSecretsManager) {
-        this._updateUseSecretsManager(useSecretsManager);
-      }
-      const hideSecretFields =
-        (this._settings.get('HideSecretFields').composite as boolean) ?? true;
-      if (hideSecretFields !== this._hideSecretFields) {
-        this._hideSecretFields = hideSecretFields;
-        this._updateSchema();
-      }
-    });
+    this._secretsManager?.fieldVisibilityChanged.connect(
+      this._fieldVisibilityChanged
+    );
+
+    this._settings.changed.connect(this._settingsChanged);
   }
 
   async componentDidUpdate(): Promise<void> {
@@ -165,6 +157,10 @@ export class AiSettings extends React.Component<
   }
 
   componentWillUnmount(): void {
+    this._settings.changed.disconnect(this._settingsChanged);
+    this._secretsManager?.fieldVisibilityChanged.disconnect(
+      this._fieldVisibilityChanged
+    );
     if (!this._secretsManager || !this._useSecretsManager) {
       return;
     }
@@ -199,11 +195,11 @@ export class AiSettings extends React.Component<
   /**
    * Save settings in local storage for a given provider.
    */
-  saveSettingsToLocalStorage(value: IDict<any>) {
-    const currentSettings = { ...value };
+  saveSettingsToLocalStorage() {
+    const currentSettings = { ...this._currentSettings };
     const settings = JSON.parse(localStorage.getItem(STORAGE_NAME) ?? '{}');
     // Do not save secrets in local storage if using the secrets manager.
-    if (this._secretsManager && this._useSecretsManager) {
+    if (this._useSecretsManager) {
       this._secretFields.forEach(field => delete currentSettings[field]);
     }
     settings[this._provider] = currentSettings;
@@ -219,21 +215,44 @@ export class AiSettings extends React.Component<
       .catch(console.error);
   }
 
+  /**
+   * Triggered when the settings has changed.
+   */
+  private _settingsChanged = (settings: ISettingRegistry.ISettings) => {
+    this._updateUseSecretsManager(
+      (this._settings.get('UseSecretsManager').composite as boolean) ?? true
+    );
+  };
+
+  /**
+   * Triggered when the secret fields visibility has changed.
+   */
+  private _fieldVisibilityChanged = (
+    _: ISecretsManager,
+    value: boolean
+  ): void => {
+    if (this._useSecretsManager) {
+      this._updateSchema();
+    }
+  };
+
   /**
    * Update the settings whether the secrets manager is used or not.
    *
    * @param value - whether to use the secrets manager or not.
    */
   private _updateUseSecretsManager = (value: boolean) => {
+    // No-op if the value did not change or the secrets manager has not been provided.
+    if (value === this._useSecretsManager || this._secretsManager === null) {
+      return;
+    }
+
+    // Update the secrets manager.
     this._useSecretsManager = value;
     if (!value) {
       // Detach all the password inputs attached to the secrets manager, and save the
       // current settings to the local storage to save the password.
-      this._secretsManager?.detachAll(Private.getToken(), SECRETS_NAMESPACE);
-      if (this._settingConnector instanceof SettingConnector) {
-        this._settingConnector.doNotSave = [];
-      }
-      this.saveSettingsToLocalStorage(this._currentSettings);
+      this._secretsManager.detachAll(Private.getToken(), SECRETS_NAMESPACE);
     } else {
       // Remove all the keys stored locally.
       const settings = JSON.parse(localStorage.getItem(STORAGE_NAME) || '{}');
@@ -245,13 +264,9 @@ export class AiSettings extends React.Component<
           });
       });
       localStorage.setItem(STORAGE_NAME, JSON.stringify(settings));
-      // Update the fields not to save in settings.
-      if (this._settingConnector instanceof SettingConnector) {
-        this._settingConnector.doNotSave = this._secretFields;
-      }
-      // Attach the password inputs to the secrets manager.
-      this.componentDidUpdate();
     }
+    this._updateSchema();
+    this.saveSettingsToLocalStorage();
     this.saveSettingsToRegistry();
   };
 
@@ -271,9 +286,16 @@ export class AiSettings extends React.Component<
       Object.entries(settingsSchema).forEach(([key, value]) => {
         if (key.toLowerCase().includes('key')) {
           this._secretFields.push(key);
-          if (this._hideSecretFields) {
+
+          // If the secrets manager is not used, do not show the secrets fields.
+          // If the secrets manager is used, check if the fields should be visible.
+          const showSecretFields =
+            !this._useSecretsManager ||
+            (this._secretsManager?.secretFieldsVisibility ?? true);
+          if (!showSecretFields) {
             return;
           }
+
           this._uiSchema[key] = { 'ui:widget': 'password' };
         }
         schema.properties[key] = value;
@@ -284,12 +306,12 @@ export class AiSettings extends React.Component<
     }
 
     // Do not save secrets in settings if using the secrets manager.
-    if (
-      this._secretsManager &&
-      this._useSecretsManager &&
-      this._settingConnector instanceof SettingConnector
-    ) {
-      this._settingConnector.doNotSave = this._secretFields;
+    if (this._settingConnector instanceof SettingConnector) {
+      if (this._useSecretsManager) {
+        this._settingConnector.doNotSave = this._secretFields;
+      } else {
+        this._settingConnector.doNotSave = [];
+      }
     }
     return schema as JSONSchema7;
   }
@@ -389,7 +411,7 @@ export class AiSettings extends React.Component<
   private _onFormChanged = (e: IChangeEvent): void => {
     const { formData } = e;
     const isModified = this._updatedFormData(formData);
-    this.saveSettingsToLocalStorage(this._currentSettings);
+    this.saveSettingsToLocalStorage();
     this.saveSettingsToRegistry();
     if (isModified !== this.state.isModified) {
       this.setState({ isModified });
@@ -407,7 +429,7 @@ export class AiSettings extends React.Component<
       ...this._currentSettings,
       ...this._defaultFormData
     };
-    this.saveSettingsToLocalStorage(this._currentSettings);
+    this.saveSettingsToLocalStorage();
     this.saveSettingsToRegistry();
     this.setState({ isModified: false });
   };
@@ -459,7 +481,6 @@ export class AiSettings extends React.Component<
   private _provider: string;
   private _providerSchema: JSONSchema7;
   private _useSecretsManager: boolean;
-  private _hideSecretFields: boolean;
   private _rmRegistry: IRenderMimeRegistry | null;
   private _secretsManager: ISecretsManager | null;
   private _settingConnector: ISettingConnector | null;

yarn.lock

@@ -1439,7 +1439,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@jupyterlab/settingregistry@npm:^4.4.0, @jupyterlab/settingregistry@npm:^4.4.1":
+"@jupyterlab/settingregistry@npm:^4.0.0, @jupyterlab/settingregistry@npm:^4.4.0, @jupyterlab/settingregistry@npm:^4.4.1":
   version: 4.4.1
   resolution: "@jupyterlab/settingregistry@npm:4.4.1"
   dependencies:
@@ -1598,7 +1598,7 @@ __metadata:
     eslint-config-prettier: ^8.8.0
     eslint-plugin-prettier: ^5.0.0
     json5: ^2.2.3
-    jupyter-secrets-manager: ^0.3.0
+    jupyter-secrets-manager: ^0.4.0
     npm-run-all: ^4.1.5
     prettier: ^3.0.0
     react: ^18.2.0
@@ -5862,16 +5862,18 @@ __metadata:
   languageName: node
   linkType: hard
 
-"jupyter-secrets-manager@npm:^0.3.0":
-  version: 0.3.0
-  resolution: "jupyter-secrets-manager@npm:0.3.0"
+"jupyter-secrets-manager@npm:^0.4.0":
+  version: 0.4.0
+  resolution: "jupyter-secrets-manager@npm:0.4.0"
   dependencies:
     "@jupyterlab/application": ^4.0.0
     "@jupyterlab/coreutils": ^6.0.0
+    "@jupyterlab/settingregistry": ^4.0.0
     "@jupyterlab/statedb": ^4.0.0
     "@lumino/algorithm": ^2.0.0
     "@lumino/coreutils": ^2.1.2
-  checksum: 8e0b9dd4acf746c3e8383a8de2621745297f7e96a323b7a5469e9cb487d4ddf0ef36af9b716a53e85868e922dfae3632dc4a961b8c059b2b79b5a0b93f2146d5
+    "@lumino/signaling": ^2.1.2
+  checksum: 1cf0399df4bc9c16e91b60d2526e32f6690b5a3b4293d5ff727ab992b49d08d172e3fba6f3d95ad97eef44b1f934d2db56823c886c47bca2fb1216e4a9b4f140
   languageName: node
   linkType: hard