search

pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
307 stars 448 forks source link

Add FieldPassword Vue component #10310

Open ewhanson opened 3 months ago

ewhanson commented 3 months ago

There are currently use cases where plugins/settings contain sensitive keys such as API keys or client secrets for third party tools. While this will not encrypt them, it will at least obscure them visually from the front end, preventing leaking keys from things such as screen recordings. Making it its own component will also open up further customization options in the future without complicating the basic FieldText component.

This will include a text field with a toggle to hide/show the contents of the field.

ewhanson commented 3 months ago

@Devika008, we talked about this a few weeks ago and I had been considering using a 'show/hide' set of icons for the button. I see we have a "View" icon that presumably has a corresponding "hidden" icon.

Screenshot of PKP SVG icons

Two questions:

  1. Do you think for this use it's okay to use icon buttons (with appropriate alt texts)?
  2. What are the sources/source library for the icons and what is the process for adding another (like a "hidden" icon)?

Thanks!

ewhanson commented 3 months ago

@Devika008, the current prototype looks like this:

https://github.com/user-attachments/assets/5e877ad9-317f-40e5-8a8c-3bbcd746a1a0

Devika008 commented 3 months ago

Hi @ewhanson

Thank you so much for the screenshot.

Usually the icon is a better option than using the whole button. We can add "alt text" to both the icons.

You can find the icons you need here: https://www.figma.com/design/nBonmguwT4YQMqgHTHewNd/PKP-Design-Library?node-id=334-36&t=cTMcEUcYpe8aMfPp-1

ewhanson commented 3 months ago

Perfect. Thanks, @Devika008!

asmecher commented 3 months ago

Just a heads-up that only adding FieldPassword where a password/API key is currently visible is not enough -- they should never be sent from server to client in the first place.

ewhanson commented 3 months ago

Thanks for the reminder, @asmecher. I'll do some thinking on this.

ewhanson commented 3 months ago

Just a heads-up that only adding FieldPassword where a password/API key is currently visible is not enough -- they should never be sent from server to client in the first place.

Curious to pick your brain on this, @asmecher. Thinking of this for things like the ORCID client secret, Datasite/Crossref password that are currently in plain input="text" fields. One approach would be to treat this the same way many services provide tokens, i.e. let you input/display it once when its added/created, then never show it in the client again, giving the user to either generate a new one in the case of something like an API token created by OJS or update the string in cases like the ORCID client secret. This would probably require some additional functionality server-side to flag data that shouldn't be sent to the client, but that should be fine.

Another long-term side path would be to optionally encrypt these values before saving them to the database. I haven't worked directly with these workflows, but that would further protected the keys by neither exposing them in the front-end, but only decrypting them when they're actively being used.

asmecher commented 3 months ago

One approach would be to treat this the same way many services provide tokens, i.e. let you input/display it once when its added/created...

Yes, that's exactly the idea. I don't mind if we store API keys in plaintext on the server side, but it shouldn't be sent back to the client.

bozana commented 3 months ago

Maybe also just another comment: the services require passwords as plain text (Crossref, mEDRA, DataCite), so encrypting and saving them so that they can be decrypted would not make much sense (regarding the security) i.e. back then we decided that it is better to have them saved as plain text for admin to exactly know...