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

Offer to store Account Number in password manager #7737

Merged

Conversation

Rawa
Copy link
Contributor

@Rawa Rawa commented Mar 1, 2025

When users create new account prompt their password manager, if available, to store the password.


This change is Reviewable

@Rawa Rawa added the Android Issues related to Android label Mar 1, 2025
@Rawa Rawa self-assigned this Mar 1, 2025
Copy link

linear bot commented Mar 1, 2025

@Rawa Rawa requested review from kl, Pururun and albin-mullvad and removed request for kl March 1, 2025 13:15
@Rawa Rawa changed the title Offer Account Number with credentialmanager Offer to store Account Number in password manager Mar 3, 2025
@Rawa Rawa force-pushed the offer-to-store-the-account-with-credentialmanager-on-droid-1854 branch from 0dd1e93 to bbce20b Compare March 3, 2025 08:11
Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

I found three things while testing:

  1. The account number is saved as a password and not a username. I assume this intentional?
  2. When trying to log in to the app I do not get prompted to use my stored account number. I guess this is because the input field is not a password. IIRC we do not want it to be a password for reasons...
  3. When trying to log in on the homepage I also do not get prompted. I guess this is becase the account is saved under mullvadvpn.mullvad.net is there a way to change this?

Reviewed 3 of 3 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Rawa Rawa force-pushed the offer-to-store-the-account-with-credentialmanager-on-droid-1854 branch from bbce20b to 43663bd Compare March 3, 2025 08:51
Copy link
Contributor Author

@Rawa Rawa left a comment

Choose a reason for hiding this comment

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

  1. Yes, this is intentional, since the account number in closer to a password than a username and a bit of a secret that you don't share. This also means that password managers won't show it w/o user explicitly revealing it like on our account page.
  2. That would part of another feature, with Compose 1.8.0 we can offer autofill from keyboard, then if we want to offer to store on login that would be yet another feature.
  3. Yes, there is nothing linking the password entry to the website at this moment, I'm a bit uncertain about the integration to make it work with the website (if it is supported at all). But there is something called digital asset links (https://developer.android.com/identity/sign-in/credential-manager#add-support-dal) that allows us to add a link to our webiste as well, if this would be forwarded to the website with the store password request I'm not sure of though.

Reviewable status: 8 of 10 files reviewed, all discussions resolved

Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

  1. 👍
  2. Right, let's wait until then again.
  3. I think it is fine with this feature then, just maybe create a new issue if we want to explore that in the future.

Reviewable status: 3 of 10 files reviewed, all discussions resolved

Pururun
Pururun previously approved these changes Mar 3, 2025
Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 7 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Rawa Rawa requested a review from Copilot March 5, 2025 12:23

Choose a reason for hiding this comment

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

PR Overview

This PR adds a feature to prompt the password manager to store a new account number during account creation by updating dependencies and documentation.

  • Added the androidx-credentials dependency in the Gradle versions file.
  • Updated CHANGELOG.md to reflect the new functionality.

Reviewed Changes

File Description
android/gradle/libs.versions.toml Added the androidx-credentials dependency for password manager usage
android/CHANGELOG.md Added changelog entry describing the new account number storage prompt

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

@Rawa Rawa force-pushed the offer-to-store-the-account-with-credentialmanager-on-droid-1854 branch from 43663bd to 0b0d60e Compare March 7, 2025 06:57
@Rawa Rawa force-pushed the offer-to-store-the-account-with-credentialmanager-on-droid-1854 branch 3 times, most recently from 6f04b15 to f65ac5c Compare March 7, 2025 07:37
@Rawa Rawa requested a review from Pururun March 7, 2025 07:37
Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Rawa Rawa force-pushed the offer-to-store-the-account-with-credentialmanager-on-droid-1854 branch from f65ac5c to 1907ced Compare March 7, 2025 08:07
@Rawa Rawa merged commit 2286727 into main Mar 7, 2025
33 checks passed
@Rawa Rawa deleted the offer-to-store-the-account-with-credentialmanager-on-droid-1854 branch March 7, 2025 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants