Skip to content

feat: toggle visibility of secret envVars #650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 22, 2024

Conversation

dw-0
Copy link
Contributor

@dw-0 dw-0 commented Oct 17, 2023

Description

Today at work i noticed, that environment variables which are configured as "secret" will be shown in plain text in the variable overview:
image
username and password are configured as secret variables.
This could lead to leaked credentials or any other secrets (there is a point why you mark them as secret right?) during MS Teams meetings while sharing you screen to other people and either want to show them something specific in that variables overview or you simply leak it by accident during other situations.

For that reason i implemented this feature. Now, by default, all secret variables are disguised by default and it requires the user to explicitly make those values visible by clicking on a new button:

23-10-17_23-23-30_electron

In case we even want to disguise the actual length of a variables value, we could refrain from using the maskValue function defined in line 14 and used in line 29 and simply replace the whole envVar.value by a fixed length ***********-string for example.

Where i'm not 100% sure at the moment is the placement and visuals of the actual toggle button. I'm open to suggestions on how to improve the placement and overall visualization in the UI.

Contribution Checklist:

  • The pull request does not introduce any breaking changes
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

closes #330
closes #1646

@pove
Copy link

pove commented Oct 18, 2023

Love it. Can this be also applied to the environment variable editor as well? I mean once you click on the "secret" checkbox while adding/editing variables.

There is an issue for that: #330

@helloanoop helloanoop added this to the v1 milestone Oct 18, 2023
@dw-0
Copy link
Contributor Author

dw-0 commented Oct 18, 2023

@pove
Ah okay, i didn't see that issue. But that probably makes sense, yes. So as soon as you tick the secret checkbox, mask the actual values with "*****" right?

@dw-0
Copy link
Contributor Author

dw-0 commented Oct 19, 2023

I pushed an update so secrets now get hidden in the env-var settings once selected as secret.
23-10-19_17-38-40_electron

I see some checks are failing, will sort that out asap.

dw-0 added 2 commits October 19, 2023 17:55
Signed-off-by: Dominik Willner <[email protected]>
# Conflicts:
#	packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js
@dw-0
Copy link
Contributor Author

dw-0 commented Oct 27, 2023

I just merged main into this PR and resolved the conflict, so it's hopefully ready for the v1 merge :)

@dw-0
Copy link
Contributor Author

dw-0 commented Nov 22, 2023

@helloanoop Hey :) Do you think this PR could make it into one of the next releases? In case you want something changed, please let me know!

@dw-0
Copy link
Contributor Author

dw-0 commented Jan 12, 2024

@helloanoop Any chance getting this merged anytime soon?

@jzorn
Copy link
Contributor

jzorn commented Feb 23, 2024

Great feature, I'd love to see it too!

@MPratley
Copy link

While in the area (At the risk of burdening this PR even more):
Secrets are also currently exposed if you hover over a variable definition

Screenshot 2024-03-14 at 13 20 51 Screenshot 2024-03-14 at 13 21 08

Would probably be good to ***** blank them there too!

@dw-0
Copy link
Contributor Author

dw-0 commented Mar 14, 2024

Good point. Unfortunately, except for one reaction to the main post, this feature doesn't seem to be getting any attention from a maintainer. Perhaps there is no real interest. This PR was assigned to a milestone (v1), but the concept of milestones does not seem to be consistently implemented, otherwise this feature would have found its way into the codebase long ago.

For this reason, I am currently less interested in continuing to work on the feature without knowing whether it will be merged in the near future. The PR has existed for almost 5 months now.

@sanjai0py
Copy link
Member

Hey @dw-0, thanks for the PR - the feature looks great! Can you please resolve the conflicts?

@dw-0 dw-0 force-pushed the hide-secrets branch 3 times, most recently from 6eef055 to e595ae2 Compare March 20, 2024 15:59
@dw-0
Copy link
Contributor Author

dw-0 commented Mar 20, 2024

I resolved the conflicts, i don't know why all tests are failing but it seems this PR is not the only one affected.

I did not manage to understand what needs to be changed to realize the proposal of @MPratley

@helloanoop helloanoop merged commit 82c600a into usebruno:main Mar 22, 2024
@helloanoop
Copy link
Contributor

Merged!

Thank you for working on this @dw-0 !
My apologies for the delay in the merge.

This will be shipped in the next outgoing release on 23 Mar 2024

Its-treason added a commit to Its-treason/bruno that referenced this pull request Mar 25, 2024
Due to changes in usebruno#650
collection variables would be passed as a object but was exptected
to be an array. Collection variables are now converted to an array.
@dw-0 dw-0 deleted the hide-secrets branch March 25, 2024 19:44
Its-treason added a commit to Its-treason/bruno that referenced this pull request Mar 27, 2024
Due to changes in usebruno#650
collection variables would be passed as a object but was exptected
to be an array. Collection variables are now converted to an array.
end3rbyte pushed a commit to end3rbyte/bruno that referenced this pull request Apr 5, 2024
Due to changes in usebruno#650
collection variables would be passed as a object but was exptected
to be an array. Collection variables are now converted to an array.
helloanoop pushed a commit that referenced this pull request Apr 9, 2024
Due to changes in #650
collection variables would be passed as a object but was exptected
to be an array. Collection variables are now converted to an array.
slowjoe007 pushed a commit to slowjoe007/bruno that referenced this pull request Apr 10, 2024
Due to changes in usebruno#650
collection variables would be passed as a object but was exptected
to be an array. Collection variables are now converted to an array.
lizziemac pushed a commit to lizziemac/bruno that referenced this pull request May 4, 2024
Due to changes in usebruno#650
collection variables would be passed as a object but was exptected
to be an array. Collection variables are now converted to an array.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide secrets in the UI [Feature Request] Show password type editor for secret environment values
6 participants