Skip to content
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

fix: firefox suggesting unlock passcode on nostr private keys #2497

Closed

Conversation

fczuardi
Copy link
Contributor

@fczuardi fczuardi commented Jun 20, 2023

Describe the changes you have made in this PR

Hints Firefox password manager that the Nostr private keys are not password for the same (no user) that is the unlock password.

Link this PR to an issue [optional]

Fixes #2489

Type of change

  • fix: Bug fix (non-breaking change which fixes an issue)

How has this been tested?

On Firefox (Linux) I have set up 3 accounts with 3 different nostr private keys, later I opened about:logins and searched for "moz". Firefox didn't replace my unlock passcode, the password for the (no username) user.

Checklist

  • My code follows the style guidelines of this project and performed a self-review of my own code
  • New and existing tests pass locally with my changes
  • I checked if I need to make corresponding changes to the documentation (and made those changes if needed)

@github-actions
Copy link

🚀 Thanks for the pull request!

Here are the current build files for testing:

Download and unzip the file for your browser. Refer to the readme for detailed install instructions.

Don't forget: keep earning sats!

@reneaaron
Copy link
Contributor

But you still want to be able to save your nostr key in the password manager, is that right?

@fczuardi
Copy link
Contributor Author

But you still want to be able to save your nostr key in the password manager, is that right?

Yes, the password manager will still be able to save the nostr private key of an account as a password, however now it will use the account name as the username, instead of the (no username) user.

In other words: the Firefox password manager will still save your nostr private key as a password if you choose to, and it will not overwrite the existing unlock passcode as it did before.

@bumi bumi requested a review from reneaaron July 3, 2023 21:04
@bumi
Copy link
Collaborator

bumi commented Jul 3, 2023

interesting! great find @fczuardi wow.

it sounds like a bit of a strange FF behavior.
@reneaaron any objections to merge this?
@fczuardi maybe we can add a link to the issue in the comment.

@fczuardi
Copy link
Contributor Author

fczuardi commented Jul 4, 2023

interesting! great find @fczuardi wow.

it sounds like a bit of a strange FF behavior. @reneaaron any objections to merge this? @fczuardi maybe we can add a link to the issue in the comment.

reference to the issue added

Co-authored-by: René Aaron <100827540+reneaaron@users.noreply.github.com>
@pavanjoshi914
Copy link
Contributor

hey @fczuardi thanks for making up the PR. we had a lot of changes in Alby screens the past few days. so I decided to make a new pr. we corrected the autocomplete problem on all the password fields that firefox is causing. closing this in favour of #2595.

@fczuardi
Copy link
Contributor Author

fczuardi commented Jul 30, 2023 via email

@pavanjoshi914
Copy link
Contributor

thanks for helping out there! please do test and let me know! on my side. issue is resolved for firefox password manager as well. let me know if issue persist on your side

@pavanjoshi914
Copy link
Contributor

where did you tried new-password field? on form level or on text field?

@fczuardi
Copy link
Contributor Author

fczuardi commented Aug 1, 2023

Hi @pavanjoshi914 I just confirmed on the master branch (that have your patch) and Firefox+Linux the issue is still reproducible. Entering a Nostr private key on the Nostr Settings and accepting the Firefox prompt to save the password, it will overwrite the saved unlocking passcode (the password with "no username" in the password manager), see screenshots below:

01-Firefox-prompt-Screenshot_20230731_214948
02-Alby-with-unlocking-password-filled-Screenshot_20230731_215430
03-Alby-with-unlocking-password-visible-Screenshot_20230731_215509

@pavanjoshi914
Copy link
Contributor

we use new-password on all the fields except unlock field. even if you save nostr private key in pasword manager. it will not autocomplete the field. in the nostr text field. as it will always default to new-password. the current discussion was to just disable autocomplete for all other fields or don't allow any saving feature on other fields. and only accept auto-complete for unlock fields.

if we use the hidden field on nostr private key. it will also disable autocomplete feature. then why to save private key in password manager?

regarding unlock fields, we don't put any restrictions there. probably we can disable autocomplete there as well. cc @rolznz @reneaaron @bumi

@fczuardi
Copy link
Contributor Author

fczuardi commented Aug 2, 2023

The password manager wont autocomplete the nostr private key field, that's correct. But if someone accepts the firefox prompt asking to save the password (by accident or by habit) on the settings page it will overwrite the unlocking password with nostr private key.

Issue #2489 was indeed about the other way around (unlocking password getting autocompleted on nostr field) and that is not happening anymore. However, after the fix we now have a new problem: nostr private key getting autocompleted on the unlocking passcode field.

Should we open a new issue then? instead of reopening #2489 ?

@pavanjoshi914 do you see my point? What my screenshots shows is that a value that is not the unlocking passcode value being completed on the unlocking passcode field. (because Firefox have only one "no username" password per domain).

The statement below is not true:

if we use the hidden field on nostr private key. it will also disable autocomplete feature.

It wont.

@pavanjoshi914
Copy link
Contributor

The password manager wont autocomplete the nostr private key field, that's correct. But if someone accepts the firefox prompt asking to save the password (by accident or by habit) on the settings page it will overwrite the unlocking password with nostr private key.

regarding unlock fields, we don't put any restrictions there. probably we can disable autocomplete there as well

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.

[BUG] Unlock password gets used as nostr private key
4 participants