Skip to content

Conversation

@Nailik
Copy link
Contributor

@Nailik Nailik commented Jul 24, 2025

🎟️ Tracking

#4100

📔 Objective

Currently there is only an implementation to provide or save Fido2 credentials via the CredentialManager.
The objective is to add the possibility to save and create Passwords.

Credential Manager usage: https://developer.android.com/identity/sign-in/credential-manager
Credential Provider to provide Credential Manager: https://developer.android.com/identity/sign-in/credential-provider
Example Project: https://github.com/android/identity-samples/tree/main/CredentialManager

Save was already done in #4110 , therefore this PR is for creating or updating password entries.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@Nailik
Copy link
Contributor Author

Nailik commented Jul 24, 2025

@SaintPatrck fyi i already worked on creating password entries via credential manager but there are still some doings. This is only to prevent others from working on this simultaneously.

@SaintPatrck
Copy link
Contributor

Sounds good. Can you convert this to a draft until you're ready for an initial review?

@bitwarden-bot
Copy link

Thank you for your contribution! We've added this to our internal tracking system for review.
ID: PM-24148
Link: https://bitwarden.atlassian.net/browse/PM-24148

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

@bitwarden-bot bitwarden-bot changed the title [Draft] add credential manager provider for create passwords [PM-24148] [Draft] add credential manager provider for create passwords Jul 25, 2025
@SaintPatrck SaintPatrck marked this pull request as draft July 25, 2025 14:13
@Nailik Nailik force-pushed the feature/add-password-credential-manager-provider-create branch from 607b7e7 to 1a61ce7 Compare September 30, 2025 01:33
@Nailik Nailik force-pushed the feature/add-password-credential-manager-provider-create branch from 1b2b97e to 78bb2b7 Compare September 30, 2025 23:57
@Nailik Nailik marked this pull request as ready for review September 30, 2025 23:57
@Nailik
Copy link
Contributor Author

Nailik commented Oct 1, 2025

@SaintPatrck i fixed the last issue where overwrite password was shown even if it was not saved yet
I don't particularly like how i fixed it.
The issue is that when creating a new passkey/password the data is saved in the content which is then converted in a cipherview. But to check if overwrite should be shown, it is checked if there is already a passkey/password. For passkey there is none yet because it needs to be created but for passwords there is then a password present. For this special case i "misused" deleted date and set it to Instant.now() in order to fake that the data is not stored.

Anyway it's ready for review now, maybe you'll find a more elegant solution to this problem.

@Nailik Nailik changed the title [PM-24148] [Draft] add credential manager provider for create passwords [PM-24148] add credential manager provider for create passwords Oct 6, 2025
}

createCredentialRequest != null -> {
//TODO only force verification for passkeys??
Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct. Passkeys require user verification as part of the FIDO2 spec. The same is not true for passwords, so we do no need to perform user verification for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think i would have it there for password aswell because it's currently also used for GetPassword

@Jiejd
Copy link

Jiejd commented Oct 17, 2025

#5911 这个问题是否个和 #4110 有关

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants