Skip to content

Adding feature to save account#9

Open
arloncunha wants to merge 2 commits into
bendews:mainfrom
arloncunha:feature/SaveAccount
Open

Adding feature to save account#9
arloncunha wants to merge 2 commits into
bendews:mainfrom
arloncunha:feature/SaveAccount

Conversation

@arloncunha

Copy link
Copy Markdown

@bendews, hope you like it 👍

Implemented save to account feature per #8

TL;DR

it doesn't validate if account tuple (url, loginname, password) already exists which means it can create or update the account seamlessly.

creating account...


image


updating password...


image

  • Testing environment:

Model Name: MacBook Air
Model Identifier: Mac15,13
Model Number: MXD13BZ/A
Chip: Apple M3

  • Version bumped to 1.0.2

@bendews

bendews commented Mar 11, 2025

Copy link
Copy Markdown
Owner

@arloncunha Love it mate! Great work, thanks so much!

@arloncunha arloncunha force-pushed the feature/SaveAccount branch from 94d6095 to dba365d Compare March 12, 2025 01:34
Comment thread src/client.ts
tabId: 0,
frameId: 0,
payload: JSON.stringify({
QID: "CmdNewAccount4URL",

@maxloeffler maxloeffler Mar 20, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey ^^. I have implemented this feature as well on my end before knowing of this PR. I have had a look at the Apple Passwords extension for chrome and investigated the commands used there to set a new password. Instead of the NEW_ACCOUNT_FOR_URL (7) command, it uses the SET_PASSWORD_FOR_LOGIN_NAME_AND_URL (6) command, even when setting passwords for sites that you have not had a password for prior. Also, instead of CmdNewAccount4URL in the payload, it uses CmdSetPassword4LoginName_URL.

While comparing your approach and "mine" I discovered that both work fine in all cases, however in "mine" you do not have to biometrically authenticate, i.e., touch id. You only need to click away the notice just as you would have to in your approach after biometrically authenticating. Therefore, I think that "my" approach is a little simpler.

I agree with everything else on this PR tho ^^.

@arloncunha arloncunha Apr 6, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx for feedback @maxloeffler, it makes sense to me.
waiting for a fix on this issue #10 in order to test and push an update with your suggestions.

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.

3 participants