core
core copied to clipboard
opnsense
Sensible to hide OTP seed in System > Access > Users > User Edit > OTP seed
Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
- [X] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
- [X] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue
Is your feature request related to a problem? Please describe.
For the purpose of managing users, I think it is better to hide the OTP seed by default because it is sensitive information and is easily captured by screen recording/screen sharing.
I have to regenerate the OTP seed and re-add it to my OTP Authenticator App every time I
Describe the solution you like
I think it would be sensible to hide the OTP seed either with:
- similar "Click to unhide" button that is used with the OTP QR code, or
- make it hidden with asterisks "************" and displayed when a "Show Seed" checkbox next to the input field is toggled on. And make it blank "" when a OTP seed is not generated yet.
Describe alternatives you considered
There are no alternatives.
Additional context
In System > Access > Users > User Edit > OTP seed
this seems pretty reasonable to me. I can put something together an open a PR if there are no objections
I don't mind too much either way, but which security issue are we fixing? An admin allowed to edit all users on the system not being allowed to see the OTP seed he can click on to see anyway?
Cheers, Franco
Is that the technical term? What about network internals on an admin screen? Passwords on Slack? ;)
Well be my guest to raise a PR but take note that #7904 will rewrite the page for 25.1 anyway and additions may disappear for lack of core team time while converting the pages. The self-service Lobby: Password use case probably also needs fixing.
That being said in MVC we would have the "advanced" toggle which may be all you need here so it's hidden by default. It may just be better to wait this out.
Cheers, Franco
Not sure I understand the "Lobby: Password" use case. The sensitive information is already hidden by the "password" input type.
Im not exactly sold on the advanced toggle. If its usage is the same as in Services: Unbound DNS: General or Services: Intrusion Detection: Administration then wouldn't that just hide the whole OTP field? As in it wouldn't even appear on the page in the first place? Its nice that the feature is just there and you don't have to find it. It does hide the OTP seed though.
Its nice that the feature is just there and you don't have to find it.
Well, you're arguing from your preference, not a requirement perspective.
And just check src/www/system_usermanager_passwordmg.php to see what I mean...
On another note, I noticed extensive use of jquery's $().click() method. Which has been deprecated. Are there any plans to update? Or is it not a concern?
Ok I confused the seed and the QR code. Password page seems fine.
Unless we decide to update jquery there is no need for noise.
This issue has been automatically timed-out (after 180 days of inactivity).
For more information about the policies for this repository, please read https://github.com/opnsense/core/blob/master/CONTRIBUTING.md for further details.
If someone wants to step up and work on this issue, just let us know, so we can reopen the issue and assign an owner to it.
