feat: masking support for SingleLineEditor

[krummbar]

Description

Fixes #2217

This change adds masking capabilities to a SingleLineEditor

Issue
As of now, an environment variable becomes read-only once it is flagged as secret. This is due to the fact, that a div overlay is rendered instead of an input field. In order to change a secret value, one has to unsecret it, edit it, and flag as secret again. This could cause secrets leaks during screen sharing sessions whenever a environment secret needs to be changed.

Solution
Now it uses the SingleLineEditor even if the variable is marked as secret, but each rendered character is altered to a *. The actual value is preserved. The masking functionality is only activated and used if <SingleLineEditor isSecret={true} /> is set, otherwise the existing behaviour is preserved.

New environment settings behaviour

bruno-issue2217-masking-feat-toggle-aligned.mp4

UI Change

Changed alignment of environment variable toggles. Toggle for en/disabling variables was centered, slightly shift left. The toggle for secrets was left aligned.
Now both are centered in the column.

Contribution Checklist:

• The pull request only addresses one issue or adds one feature.
• The pull request does not introduce any breaking changes
• I have added screenshots or gifs to help explain the change if applicable.
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

[lizziemac]

Nice!! Instead of passing in the masking from the isSecret checkbox, I think it makes more sense to have the ability to mask/unmask be within the field itself (e.g. an eye open/closed icon in the input field where the masking occurs). This is mainly because by toggling the secret field, there's a risk that the secret is saved as a value instead, which by effect could accidentally be saved in a remote repository.

[krummbar]

Now it looks like this.

bruno-issue2217-masking-feat-visibility-toggle.mp4

After extending the visuals, I'm wondering if the maskable capabilities should be added as an individual component. Say MaskableSingleLineEditor derived from SingleLineEditor, which has the visuals integrated as well. I can't estimate if this an easy straight-forward implementation as I'm just starting out with the code base and learning about react.

Maybe someone who has experience can give feedback if it would be feasible to separate the logic here or if this can be merged as it is right now.

[lizziemac]

Looks great!

Maybe someone who has experience can give feedback if it would be feasible to separate the logic here or if this can be merged as it is right now.

I've provided a .patch of what you could do to make it solely a part of SingleLineEditor. Generally for features like this in my own projects, I prefer reducing code duplication as much as possible, which is why the patch file keeps the changes in SingleLineEditor, as opposed to a new module (e.g. MaskableSingleLineEditor ) that uses a lot of the same base utilities.

Some improvements could be made to this change, like removing logs and renaming some functions.

To apply to your own branch for testing, do git apply suggested_change.patch
suggested_change.patch

Contents (if you don't want to download a file):

diff --git a/packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js b/packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js
index 9677fc36..6c1dc2a5 100644
--- a/packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js
+++ b/packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js
@@ -7,7 +7,6 @@ import StyledWrapper from './StyledWrapper';
 import { uuid } from 'utils/common';
 import { useFormik } from 'formik';
 import * as Yup from 'yup';
-import { IconEye, IconEyeOff } from '@tabler/icons';
 import { variableNameRegex } from 'utils/common/regex';
 import { saveEnvironment } from 'providers/ReduxStore/slices/collections/actions';
 import cloneDeep from 'lodash/cloneDeep';
@@ -58,15 +57,6 @@ const EnvironmentVariables = ({ environment, collection, setIsModified, original
     setIsModified(formik.dirty);
   }, [formik.dirty]);
 
-  const toggleVisibleSecret = (variable) => {
-    visibleSecrets[variable.uid] = !visibleSecrets[variable.uid];
-    setVisibleSecrets({ ...visibleSecrets });
-  };
-
-  const isMaskedSecret = (variable) => {
-    return variable.secret && visibleSecrets[variable.uid] !== true;
-  };
-
   const ErrorMessage = ({ name }) => {
     const meta = formik.getFieldMeta(name);
     if (!meta.error) {
@@ -148,19 +138,10 @@ const EnvironmentVariables = ({ environment, collection, setIsModified, original
                       collection={collection}
                       name={`${index}.value`}
                       value={variable.value}
-                      maskInput={isMaskedSecret(variable)}
+                      isSecret={variable.secret}
                       onChange={(newValue) => formik.setFieldValue(`${index}.value`, newValue, true)}
                     />
                   </div>
-                  {variable.secret ? (
-                    <button type="button" className="btn btn-sm !pr-0" onClick={() => toggleVisibleSecret(variable)}>
-                      {isMaskedSecret(variable) ? (
-                        <IconEyeOff size={18} strokeWidth={2} />
-                      ) : (
-                        <IconEye size={18} strokeWidth={2} />
-                      )}
-                    </button>
-                  ) : null}
                 </td>
                 <td>
                   <input
diff --git a/packages/bruno-app/src/components/SingleLineEditor/index.js b/packages/bruno-app/src/components/SingleLineEditor/index.js
index f295d712..04cc0e37 100644
--- a/packages/bruno-app/src/components/SingleLineEditor/index.js
+++ b/packages/bruno-app/src/components/SingleLineEditor/index.js
@@ -3,6 +3,7 @@ import isEqual from 'lodash/isEqual';
 import { getAllVariables } from 'utils/collections';
 import { defineCodeMirrorBrunoVariablesMode, MaskedEditor } from 'utils/common/codemirror';
 import StyledWrapper from './StyledWrapper';
+import { IconEye, IconEyeOff } from '@tabler/icons';
 
 let CodeMirror;
 const SERVER_RENDERED = typeof navigator === 'undefined' || global['PREVENT_CODEMIRROR_RENDER'] === true;
@@ -20,7 +21,11 @@ class SingleLineEditor extends Component {
     this.cachedValue = props.value || '';
     this.editorRef = React.createRef();
     this.variables = {};
-  }
+
+    this.state = {
+      maskInput: props.isSecret || false, // Always mask the input by default (if it's a secret)
+    };
+  }
   componentDidMount() {
     // Initialize CodeMirror as a single line editor
     /** @type {import("codemirror").Editor} */
@@ -91,11 +96,13 @@ class SingleLineEditor extends Component {
     this.editor.setValue(String(this.props.value) || '');
     this.editor.on('change', this._onEdit);
     this.addOverlay();
-    if (this.props.maskInput) this._enableMaskedEditor(this.props.maskInput);
+    this._enableMaskedEditor(this.props.isSecret);
+    this.setState({ maskInput: this.props.isSecret });
   }
 
   /** Enable or disable masking the rendered content of the editor */
   _enableMaskedEditor = (enabled) => {
+    console.log('Enabling masked editor: ' + enabled)
     if (enabled == true) {
       if (!this.maskedEditor) this.maskedEditor = new MaskedEditor(this.editor, '*');
       this.maskedEditor.enable();
@@ -131,11 +138,13 @@ class SingleLineEditor extends Component {
       this.cachedValue = String(this.props.value);
       this.editor.setValue(String(this.props.value) || '');
     }
-    if (!isEqual(this.props.maskInput, prevProps.maskInput)) {
-      this._enableMaskedEditor(this.props.maskInput);
-    }
-    if (this.props.maskInput) {
+    if (!isEqual(this.props.isSecret, prevProps.isSecret)) {
+      console.log('Secret flag changed to ' + this.props.isSecret);
+      // If the secret flag has changed, update the editor to reflect the change
+      this._enableMaskedEditor(this.props.isSecret);
       this.maskedEditor?.update();
+      // also set the maskInput flag to the new value
+      this.setState({ maskInput: this.props.isSecret });
     }
     this.ignoreChangeEvent = false;
   }
@@ -152,8 +161,39 @@ class SingleLineEditor extends Component {
     this.editor.setOption('mode', 'brunovariables');
   };
 
+  toggleVisibleSecret = () => {
+    const isVisible = !this.state.maskInput;
+    this.setState({ maskInput: isVisible });
+    this._enableMaskedEditor(isVisible);
+    console.log('Secret flag changed to ' + isVisible);
+  }
+
+  /**
+   * @brief Eye icon to show/hide the secret value
+   * @returns ReactComponent The eye icon
+   */
+  secretEye = (isSecret) => {
+    return (
+      isSecret === true ? (
+        <button type="button" className="btn btn-sm !pr-0" onClick={() => this.toggleVisibleSecret()}>
+          {this.state.maskInput === true ? (
+            <IconEyeOff size={18} strokeWidth={2} />
+          ) : (
+            <IconEye size={18} strokeWidth={2} />
+          )}
+        </button>
+      ) : null
+    );
+  }
+
+
   render() {
-    return <StyledWrapper ref={this.editorRef} className="single-line-editor"></StyledWrapper>;
+    return (
+      <div style={{ display: 'flex', justifyContent: 'space-between', alignItems: 'center' }}>
+        <StyledWrapper ref={this.editorRef} className="single-line-editor" />
+        {this.secretEye(this.props.isSecret)}
+      </div>
+    );
   }
 }
 export default SingleLineEditor;

Edits: bug fix related to changing between secret and non-secret, and clarification on a comment

[lizziemac]

Cool thanks for your valuable input! I've applied your patch, looks a lot cleaner now :) I just removed two console.log statements as they were logging redundant information.

I'm happy to help! :D And thanks for getting those logs

If you have a no-reply mail I'll add you as co-author, which is required according to the documentation

Absolutely! My email is 18120837+lizziemac@users.noreply.github.com

Also, if you rebase off of main, your tests should pass now :-)

[lizziemac]

Approving because you did most of this, but I expect another one will be needed since I also contributed. Great work, again!

[pietrygamat]

+1 I can see it also fixed this annoying bobbing:
before and after:

What I find missing is any indication on the eye icon when it is focused (when using keyboard navigation) - it can be selected, and activated with space, but its not highlighted in any way.

[krummbar]

What I find missing is any indication on the eye icon when it is focused (when using keyboard navigation) - it can be selected, and activated with space, but its not highlighted in any way.

Good catch, didn't think about keyboard navigation. It should work now.

Screencast.from.2024-05-08.06-48-59.webm

---

While I'm at it... I think the columns enabled and secret look a little bit odd.

• enabled is centered, but slightly shifted left
• secret is aligned left

IMHO it looks better if both columns are simply centered. If that is fine I can add this change to the PR.

[krummbar]

The content for both columns is now centered. I've updated the PR description to match the changes that have been applied in the meantime.