Skip to content

Commit

Permalink
Dedup credentials suggestions and sort them by latest usage (#5675)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/488551667048375/1209453476472067

### Description
We introduce the same logic to dedup credentials and sorting for
Autofill Service.
That includes updating last used timestamp when we autofill outside our
app, to improve UX.

### Steps to test this PR

_Feature 1_
- [x] install this branch
- [x] Select DDG as autofill provider
- [x] Create a bunch of logins (same username and passwords) for
mlb.mlb.com, securea.mlb.com and www.mlb.com
- [x] Create another login for www.mlb.com with a different username
- [x] Visit www.mlb.com in our browser
- [x] You will see 2 logins suggested
- [x] Now go to another browser and visit the same site
- [x] Ensure you only get 2 logins, same order from the autofill
provider
- [x] Select the second one
- [x] Now go back to our browser
- [x] Trigger again the suggestions prompt
- [x] Ensure the order has changed now (since you selected the second
one in the previous step)

### UI changes
| Before  | After |
| ------ | ----- |
!(Upload before screenshot)|(Upload after screenshot)|
  • Loading branch information
cmonfortep authored Feb 24, 2025
1 parent 79b0e9a commit 449975a
Show file tree
Hide file tree
Showing 26 changed files with 575 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@ package com.duckduckgo.autofill.impl.service
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.duckduckgo.anvil.annotations.ContributesViewModel
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.impl.securestorage.SecureStorage
import com.duckduckgo.autofill.impl.securestorage.WebsiteLoginDetailsWithCredentials
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseViewModel.Command.AutofillLogin
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseViewModel.Command.ContinueWithoutAuthentication
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseViewModel.Command.ForceFinish
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseViewModel.Command.RequestAuthentication
import com.duckduckgo.autofill.impl.store.InternalAutofillStore
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.Command
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.ActivityScope
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.Flow
Expand All @@ -41,8 +43,9 @@ import timber.log.Timber
@ContributesViewModel(ActivityScope::class)
class AutofillProviderChooseViewModel @Inject constructor(
private val autofillProviderDeviceAuth: AutofillProviderDeviceAuth,
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
private val dispatchers: DispatcherProvider,
private val secureStorage: SecureStorage,
private val autofillStore: InternalAutofillStore,
) : ViewModel() {

private val command = Channel<Command>(1, BufferOverflow.DROP_OLDEST)
Expand Down Expand Up @@ -72,15 +75,23 @@ class AutofillProviderChooseViewModel @Inject constructor(
fun continueAfterAuthentication(credentialId: Long) {
Timber.i("DDGAutofillService request to autofill login with credentialId: $credentialId")
viewModelScope.launch(dispatchers.io()) {
secureStorage.getWebsiteLoginDetailsWithCredentials(credentialId)?.toLoginCredentials()?.let {
autofillStore.getCredentialsWithId(credentialId)?.let { loginCredential ->
loginCredential.updateLastUsedTimestamp()
Timber.i("DDGAutofillService $credentialId found, autofilling")
command.send(AutofillLogin(it))
command.send(AutofillLogin(loginCredential))
} ?: run {
command.send(ForceFinish)
}
}
}

private fun LoginCredentials.updateLastUsedTimestamp() {
appCoroutineScope.launch(dispatchers.io()) {
val updated = this@updateLastUsedTimestamp.copy(lastUsedMillis = System.currentTimeMillis())
autofillStore.updateCredentials(updated, refreshLastUpdatedTimestamp = false)
}
}

private fun WebsiteLoginDetailsWithCredentials.toLoginCredentials(): LoginCredentials {
return LoginCredentials(
id = details.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ package com.duckduckgo.autofill.impl.service
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.duckduckgo.anvil.annotations.ContributesViewModel
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.impl.store.InternalAutofillStore
import com.duckduckgo.autofill.impl.ui.credential.management.searching.CredentialListFilter
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.ActivityScope
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
Expand All @@ -40,6 +42,7 @@ class AutofillProviderCredentialsListViewModel @Inject constructor(
private val pixel: Pixel,
private val dispatchers: DispatcherProvider,
private val credentialListFilter: CredentialListFilter,
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
) : ViewModel() {

private val _viewState = MutableStateFlow(ViewState())
Expand Down Expand Up @@ -70,6 +73,17 @@ class AutofillProviderCredentialsListViewModel @Inject constructor(
_viewState.value = _viewState.value.copy(credentialSearchQuery = searchText)
}

fun onCredentialSelected(credentials: LoginCredentials) {
credentials.updateLastUsedTimestamp()
}

private fun LoginCredentials.updateLastUsedTimestamp() {
appCoroutineScope.launch(dispatchers.io()) {
val updated = this@updateLastUsedTimestamp.copy(lastUsedMillis = System.currentTimeMillis())
autofillStore.updateCredentials(updated, refreshLastUpdatedTimestamp = false)
}
}

data class ViewState(
val logins: List<LoginCredentials>? = null,
val credentialSearchQuery: String = "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,12 @@ import android.view.autofill.AutofillValue
import androidx.annotation.RequiresApi
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.api.store.AutofillStore
import com.duckduckgo.autofill.impl.service.AutofillFieldType.UNKNOWN
import com.duckduckgo.autofill.impl.service.AutofillFieldType.USERNAME
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseActivity.Companion.FILL_REQUEST_AUTOFILL_CREDENTIAL_ID_EXTRAS
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseActivity.Companion.FILL_REQUEST_AUTOFILL_ID_EXTRAS
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseActivity.Companion.FILL_REQUEST_PACKAGE_ID_EXTRAS
import com.duckduckgo.autofill.impl.service.AutofillProviderChooseActivity.Companion.FILL_REQUEST_URL_EXTRAS
import com.duckduckgo.autofill.impl.service.mapper.AppCredentialProvider
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
Expand All @@ -55,10 +53,9 @@ interface AutofillProviderSuggestions {
@ContributesBinding(AppScope::class)
class RealAutofillProviderSuggestions @Inject constructor(
private val appBuildConfig: AppBuildConfig,
private val autofillStore: AutofillStore,
private val viewProvider: AutofillServiceViewProvider,
private val suggestionsFormatter: AutofillServiceSuggestionCredentialFormatter,
private val appCredentialProvider: AppCredentialProvider,
private val autofillSuggestions: AutofillSuggestions,
) : AutofillProviderSuggestions {

companion object {
Expand Down Expand Up @@ -210,18 +207,16 @@ class RealAutofillProviderSuggestions @Inject constructor(
AutofillValue.forText(credential?.password ?: "password")
}

private suspend fun loginCredentials(node: AutofillRootNode): List<LoginCredentials>? {
val crendentialsForDomain = node.website.takeUnless { it.isNullOrBlank() }?.let {
autofillStore.getCredentials(it)
private suspend fun loginCredentials(node: AutofillRootNode): List<LoginCredentials> {
val credentialsForDomain = node.website.takeUnless { it.isNullOrBlank() }?.let { nonEmptyWebsite ->
autofillSuggestions.getSiteSuggestions(nonEmptyWebsite)
} ?: emptyList()

val crendentialsForPackage = node.packageId.takeUnless { it.isNullOrBlank() }?.let {
appCredentialProvider.getCredentials(it)
val credentialsForPackage = node.packageId.takeUnless { it.isNullOrBlank() }?.let { nonEmptyPackageId ->
autofillSuggestions.getAppSuggestions(nonEmptyPackageId)
} ?: emptyList()

Timber.v("DDGAutofillService credentials for domain: $crendentialsForDomain")
Timber.v("DDGAutofillService credentials for package: $crendentialsForPackage")
return crendentialsForDomain.plus(crendentialsForPackage).distinct()
return credentialsForDomain.plus(credentialsForPackage).distinct()
}

private fun createAutofillSelectionIntent(context: Context, url: String, packageId: String): PendingIntent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ class AutofillSimpleCredentialsListFragment : DuckDuckGoFragment(R.layout.fragme
}

private fun onCredentialsSelected(credentials: LoginCredentials) {
viewModel.onCredentialSelected(credentials)
parentActivity()?.autofillLogin(credentials) ?: run {
activity?.finish()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (c) 2025 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.autofill.impl.service

import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.api.store.AutofillStore
import com.duckduckgo.autofill.impl.deduper.AutofillLoginDeduplicator
import com.duckduckgo.autofill.impl.service.mapper.AppCredentialProvider
import com.duckduckgo.autofill.impl.ui.credential.selecting.AutofillSelectCredentialsGrouper
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import dagger.SingleInstanceIn
import javax.inject.Inject
import kotlinx.coroutines.withContext
import timber.log.Timber

interface AutofillSuggestions {
suspend fun getSiteSuggestions(website: String): List<LoginCredentials>
suspend fun getAppSuggestions(packageId: String): List<LoginCredentials>
}

@SingleInstanceIn(AppScope::class)
@ContributesBinding(AppScope::class)
class AutofillServiceSuggestions @Inject constructor(
private val autofillStore: AutofillStore,
private val loginDeduplicator: AutofillLoginDeduplicator,
private val grouper: AutofillSelectCredentialsGrouper,
private val appCredentialProvider: AppCredentialProvider,
private val dispatcherProvider: DispatcherProvider,
) : AutofillSuggestions {

override suspend fun getSiteSuggestions(website: String): List<LoginCredentials> {
return withContext(dispatcherProvider.io()) {
val credentials = autofillStore.getCredentials(website)
loginDeduplicator.deduplicate(website, credentials).let { dedupLogins ->
grouper.group(website, dedupLogins).let { groups ->
groups.perfectMatches
.plus(groups.partialMatches.values.flatten())
.plus(groups.shareableCredentials.values.flatten())
}
}.also {
Timber.v("DDGAutofillService credentials for domain: $it")
}
}
}

override suspend fun getAppSuggestions(packageId: String): List<LoginCredentials> {
return withContext(dispatcherProvider.io()) {
appCredentialProvider.getCredentials(packageId).also {
Timber.v("DDGAutofillService credentials for package: $it")
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.impl.urlmatcher.AutofillUrlMatcher
import com.duckduckgo.autofill.impl.urlmatcher.AutofillUrlMatcher.ExtractedUrlParts
import com.duckduckgo.common.utils.extractDomain
import com.duckduckgo.di.scopes.FragmentScope
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import java.text.Collator
import javax.inject.Inject
Expand All @@ -30,7 +30,7 @@ interface CredentialListSorter {
fun comparator(): Collator
}

@ContributesBinding(FragmentScope::class)
@ContributesBinding(AppScope::class)
class CredentialListSorterByTitleAndDomain @Inject constructor(
private val autofillUrlMatcher: AutofillUrlMatcher,
) : CredentialListSorter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.impl.ui.credential.management.sorting.CredentialListSorter
import com.duckduckgo.autofill.impl.ui.credential.selecting.AutofillSelectCredentialsGrouper.Groups
import com.duckduckgo.autofill.impl.urlmatcher.AutofillUrlMatcher
import com.duckduckgo.di.scopes.FragmentScope
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import java.util.*
import javax.inject.Inject
Expand All @@ -40,7 +40,7 @@ interface AutofillSelectCredentialsGrouper {
)
}

@ContributesBinding(FragmentScope::class)
@ContributesBinding(AppScope::class)
class RealAutofillSelectCredentialsGrouper @Inject constructor(
private val autofillUrlMatcher: AutofillUrlMatcher,
private val sorter: CredentialListSorter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
package com.duckduckgo.autofill.impl.ui.credential.selecting

import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.di.scopes.FragmentScope
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import javax.inject.Inject
import javax.inject.Named

interface TimestampBasedLoginSorter : Comparator<LoginCredentials>

@ContributesBinding(FragmentScope::class)
@ContributesBinding(AppScope::class)
@Named("LastUsedCredentialSorter")
class LastUsedCredentialSorter @Inject constructor() : TimestampBasedLoginSorter {

Expand Down Expand Up @@ -54,7 +54,7 @@ class LastUsedCredentialSorter @Inject constructor() : TimestampBasedLoginSorter
}
}

@ContributesBinding(FragmentScope::class)
@ContributesBinding(AppScope::class)
@Named("LastUpdatedCredentialSorter")
class LastUpdatedCredentialSorter @Inject constructor() : TimestampBasedLoginSorter {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* Copyright (c) 2025 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.autofill

import com.duckduckgo.autofill.api.AutofillFeature
import com.duckduckgo.autofill.api.domain.app.LoginCredentials
import com.duckduckgo.autofill.impl.FakePasswordStoreEventPlugin
import com.duckduckgo.autofill.impl.SecureStoreBackedAutofillStore
import com.duckduckgo.autofill.impl.deduper.AutofillLoginDeduplicator
import com.duckduckgo.autofill.impl.encoding.TestUrlUnicodeNormalizer
import com.duckduckgo.autofill.impl.securestorage.SecureStorage
import com.duckduckgo.autofill.impl.ui.credential.selecting.AutofillSelectCredentialsGrouper
import com.duckduckgo.autofill.impl.ui.credential.selecting.AutofillSelectCredentialsGrouper.Groups
import com.duckduckgo.autofill.impl.urlmatcher.AutofillDomainNameUrlMatcher
import com.duckduckgo.autofill.impl.urlmatcher.AutofillUrlMatcher
import com.duckduckgo.autofill.store.AutofillPrefsStore
import com.duckduckgo.autofill.store.LastUpdatedTimeProvider
import com.duckduckgo.autofill.sync.CredentialsSyncMetadata
import com.duckduckgo.autofill.sync.SyncCredentialsListener
import com.duckduckgo.autofill.sync.inMemoryAutofillDatabase
import com.duckduckgo.common.utils.DispatcherProvider
import kotlinx.coroutines.CoroutineScope

fun fakeAutofillStore(
secureStorage: SecureStorage = fakeStorage(),
lastUpdatedTimeProvider: LastUpdatedTimeProvider = lastUpdatedTimeProvider(),
autofillPrefsStore: AutofillPrefsStore,
autofillUrlMatcher: AutofillUrlMatcher = autofillDomainNameUrlMatcher(),
autofillFeature: AutofillFeature,
dispatcherProvider: DispatcherProvider,
coroutineScope: CoroutineScope,
) = SecureStoreBackedAutofillStore(
secureStorage = secureStorage,
lastUpdatedTimeProvider = lastUpdatedTimeProvider,
autofillPrefsStore = autofillPrefsStore,
dispatcherProvider = dispatcherProvider,
autofillUrlMatcher = autofillUrlMatcher,
passwordStoreEventListenersPlugins = FakePasswordStoreEventPlugin(),
syncCredentialsListener = SyncCredentialsListener(
CredentialsSyncMetadata(inMemoryAutofillDatabase().credentialsSyncDao()),
dispatcherProvider,
coroutineScope,
),
autofillFeature = autofillFeature,
)

fun fakeStorage(
canAccessSecureStorage: Boolean = true,
urlMatcher: AutofillUrlMatcher = autofillDomainNameUrlMatcher(),
) = FakeSecureStore(
canAccessSecureStorage = canAccessSecureStorage,
urlMatcher = urlMatcher,
)

fun lastUpdatedTimeProvider() = object : LastUpdatedTimeProvider {
override fun getInMillis(): Long = 10000L
}

fun autofillDomainNameUrlMatcher() = AutofillDomainNameUrlMatcher(TestUrlUnicodeNormalizer())

fun noopDeduplicator() = object : AutofillLoginDeduplicator {
override fun deduplicate(
originalUrl: String,
logins: List<LoginCredentials>,
): List<LoginCredentials> = logins
}

fun noopGroupBuilder() = object : AutofillSelectCredentialsGrouper {
override fun group(
originalUrl: String,
unsortedCredentials: List<LoginCredentials>,
): Groups {
return Groups(unsortedCredentials, emptyMap(), emptyMap())
}
}
Loading

0 comments on commit 449975a

Please sign in to comment.