Skip to content

Conversation

@rewanthtammana
Copy link

@rewanthtammana rewanthtammana commented Jul 22, 2019

Issue -

The user can store secrets in drone UI but the secrets are "displayed in plain text" while the user types the values into the input field.

Fix -

This PR sets the input to "password type" to make sure the sensitive information isn't visible to others through shoulder surfing.

cc: @HrushikeshK

@bradrydzewski
Copy link
Contributor

bradrydzewski commented Jul 22, 2019

This is similar to pull request #190. The password field does not allow for multi-line inputs, which is important since a large percentage of secrets are multi-line strings (private keys, json documents, etc). We have therefore decided not to use a password field.

There are pure css workarounds that could be implemented such as -webkit-text-security: disc. This is a relatively new and non-standard css feature, but is something I would be willing to accept since it works with textarea and supports multi-line values.

@rewanthtammana
Copy link
Author

rewanthtammana commented Jul 22, 2019

Valid point! But I don't think this CSS feature will work in all browsers. If true, this wouldn't be the best solution.

I think we can use JS/Jquery to hide every character entered into the input field into dot/star character. Would that be a good way to resolve this issue?

@tboerger
Copy link

Maybe build a widget where the content is hidden by default via js including an overflowing icon to toggle the visibility?

@bradrydzewski
Copy link
Contributor

bradrydzewski commented Jul 22, 2019

In my experience hidden textboxes that intercept keystrokes, mouse clicks, text selection, etc are highly error prone and would become a maintenance burden. I do not want to introduce such complexity into the codebase. I can already see the issues piling up :)

As a short term solution, I would be willing to accept a pull request for -webkit-text-security: disc which data suggests would support 94.4% of users or more. The remaining 4.6% of users can fallback to the command line if they feel uncomfortable with the unmasked textarea.

@bradrydzewski
Copy link
Contributor

As a longer term solution, perhaps we can look into https://github.com/noppa/text-security which is a special font that renders all characters as discs. Since it is a font it would be cross-browser compatible.

@rewanthtammana
Copy link
Author

rewanthtammana commented Jul 23, 2019

Hi @bradrydzewski @tboerger ,

The text-security solution ran successfully in Edge, Firefox and Chrome. We think the text-security module is the best solution to prevent shoulder surfing attacks and integrated it with drone-ui. Please review the PR and share your views.

You can test "text-security" project on my codepen, https://codepen.io/rewanthcool/pen/ZgQJXG

@tboerger
Copy link

IMHO it should rely on https://www.npmjs.com/package/text-security instead of bundling this vendor files.

@rewanthtammana
Copy link
Author

rewanthtammana commented Jul 26, 2019

I presumed that the font will be stable at all times and cloned "text-security" GitHub repo into the code instead of using npm. After you mentioned about npm bundle, even I felt using npm is the best way to integrate new modules into the code. I made the changes accordingly. Please have a look at the latest code.

@tboerger
Copy link

I just realized now that you are targeting the wrong branch, the react app isn't used anymore, the drone ui is using the vue branch.

@rewanthtammana
Copy link
Author

Thanks for pointing that @tboerger

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.

4 participants