[PM-29570] feat: Implement password generation with UI in AutoFill extension#2782
Conversation
…tension Adds prepareInterface(for:) to handle ASGeneratePasswordsRequest with user interaction (iOS 26.2+). A GeneratePasswordExtensionDelegate bridges GeneratorCoordinatorDelegate to the extension context, mapping GeneratorType and the password string to ASGeneratedPassword.Kind (.passphrase, .strong, .alphanumeric). VaultCoordinator handles the new VaultRoute.generatePassword route by pushing the generator via the existing stack navigator rather than presenting modally, avoiding the window-hierarchy crash at auth-completion time.
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR implements the iOS 26.2+ Code Review DetailsNo findings. The hardcoded |
| @@ -0,0 +1,88 @@ | |||
| import AuthenticationServices | |||
| import XCTest | |||
There was a problem hiding this comment.
⛏️ Want to convert this to Swift Testing?
There was a problem hiding this comment.
As far as I know I can't, as we would require @availability check at the suite level (given var subject: GeneratePasswordExtensionDelegate!) and that's not possible for the time being. See swiftlang/swift-testing#608
| /// | ||
| @available(iOSApplicationExtension 26.2, *) | ||
| public func generatePasswordCredential(request: any GeneratePasswordRequestProxy) async throws -> String { | ||
| try await unlockVaultWithNeverlockKey() |
There was a problem hiding this comment.
🤔 Does the vault need to be unlocked to generate a password? Or is this needed for something else?
There was a problem hiding this comment.
I've asked Product because if we need to save the password in the password generator history, then we need to unlock the vault. Otherwise it wouldn't be necessary.
| /// A route to the generate password screen in the autofill extension. | ||
| case generatePassword |
There was a problem hiding this comment.
🤔 If this is something that wouldn't ever be used standalone, and only used with the extension, I wonder if we should name it to be specific to the extension?
There was a problem hiding this comment.
I think it could potentially be used with AppIntents if we were to integrate the UI somehow there. Also I believe that given that it's in the VaultRoute it doesn't matter whether that is for the extension or for whatever else, it just tells what's the route. I can see that I may need to remove the comment of "autofill extension" on the comment though. Would that be fine or would you still like me to rename it?
There was a problem hiding this comment.
I guess there was two things that prompted my initial question. 1) the comment "in the autofill extension", which could be removed. But 2) showGeneratePassword() requires appExtensionDelegate to be set, which kind of limits it to being used in the extension, at least how it's set up today.
See my other comment though, if this route just displays the generator as the root view, instead of showing it on/over the vault list, I feel like it would be better to promote this to AppRoute, since that would better match the view hierarchy of how it's displayed.
| stackNavigator: stackNavigator, | ||
| ).asAnyCoordinator() | ||
| coordinator.start() | ||
| coordinator.navigate(to: .generator(staticType: .password)) |
There was a problem hiding this comment.
🤔 Given that we're essentially bypassing the vault here and just showing the generator, would it make sense to add a generator route to AppRoute and have AppCoordinator handle displaying the generator? Is that more or less complex than using the vault coordinator?
fedemkr
changed the base branch from
main
to
PM-29569/extension-password-generation-no-ui
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-29570
📔 Objective
Implements
prepareInterface(for:)inCredentialProviderViewControllerto handleASGeneratePasswordsRequestwith user interaction (iOS 26.2+).When the OS triggers this entry point, the extension:
generatePasswordCredential(_:userInteraction: true)context.VaultRoute.generatePasswordroute.GeneratePasswordExtensionDelegatebridgesGeneratorCoordinatorDelegatecallbacks to the extension context, mappingGeneratorType+ password string toASGeneratedPassword.Kind(.passphrase,.strong, or.alphanumeric).extensionContext.completeGeneratePasswordRequest(results:)with the user-selected password.Warning
Info.plist values has not been updated as we need to wait for #2743 to be merged to have bitwarden/sdk-internal#1102 included to get the appropriate generation rules. Currently the password generation rules are not the ones that come from the OS. The SDK incorporation and update of the
Info.plistwill be done in a future PR.📸 Screenshots
Password.generation.with.UI.mov