Skip to content

Commit dd31e4f

Browse files
SaintPatrckSaintPatrck
committed
[PM-11886] Update handling of unprivileged apps and improve error messaging
1 parent 445bd90 commit dd31e4f

File tree

19 files changed

+215
-117
lines changed

19 files changed

+215
-117
lines changed

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt

Lines changed: 86 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.bitwarden.fido.Origin
66
import com.bitwarden.fido.UnverifiedAssetLink
77
import com.bitwarden.sdk.Fido2CredentialStore
88
import com.bitwarden.vault.CipherView
9+
import com.x8bit.bitwarden.R
910
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CreateCredentialRequest
1011
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionRequest
1112
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult
@@ -22,9 +23,8 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.model.AuthenticateFido2Cred
2223
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.RegisterFido2CredentialRequest
2324
import com.x8bit.bitwarden.data.vault.datasource.sdk.util.toAndroidAttestationResponse
2425
import com.x8bit.bitwarden.data.vault.datasource.sdk.util.toAndroidFido2PublicKeyCredential
25-
import com.x8bit.bitwarden.ui.platform.base.util.toHostOrPathOrNull
26+
import com.x8bit.bitwarden.ui.platform.base.util.asText
2627
import kotlinx.serialization.SerializationException
27-
import kotlinx.serialization.encodeToString
2828
import kotlinx.serialization.json.Json
2929

3030
/**
@@ -48,41 +48,45 @@ class Fido2CredentialManagerImpl(
4848
fido2CreateCredentialRequest: Fido2CreateCredentialRequest,
4949
selectedCipherView: CipherView,
5050
): Fido2RegisterCredentialResult {
51-
val clientData = if (fido2CreateCredentialRequest.callingAppInfo.isOriginPopulated()) {
52-
fido2CreateCredentialRequest
53-
.callingAppInfo
51+
val callingAppInfo = fido2CreateCredentialRequest.callingAppInfo
52+
val clientData = if (fido2CreateCredentialRequest.origin.isNullOrEmpty()) {
53+
ClientData.DefaultWithExtraData(androidPackageName = callingAppInfo.packageName)
54+
} else {
55+
callingAppInfo
5456
.getAppSigningSignatureFingerprint()
5557
?.let { ClientData.DefaultWithCustomHash(hash = it) }
56-
?: return Fido2RegisterCredentialResult.Error
57-
} else {
58-
ClientData.DefaultWithExtraData(
59-
androidPackageName = fido2CreateCredentialRequest
60-
.callingAppInfo
61-
.packageName,
58+
?: return Fido2RegisterCredentialResult.Error(
59+
R.string.passkey_operation_failed_because_app_is_signed_incorrectly.asText(),
60+
)
61+
}
62+
val sdkOrigin = if (fido2CreateCredentialRequest.origin.isNullOrEmpty()) {
63+
val host = getOriginUrlFromAttestationOptionsOrNull(
64+
requestJson = fido2CreateCredentialRequest.requestJson,
65+
)
66+
?: return Fido2RegisterCredentialResult.Error(
67+
R.string.passkey_operation_failed_because_host_url_is_not_present_in_request
68+
.asText(),
69+
)
70+
Origin.Android(
71+
UnverifiedAssetLink(
72+
packageName = callingAppInfo.packageName,
73+
sha256CertFingerprint = callingAppInfo.getSignatureFingerprintAsHexString()
74+
?: return Fido2RegisterCredentialResult.Error(
75+
R.string.passkey_operation_failed_because_app_signature_is_invalid
76+
.asText(),
77+
),
78+
host = host,
79+
assetLinkUrl = host,
80+
),
6281
)
82+
} else {
83+
Origin.Web(fido2CreateCredentialRequest.origin)
6384
}
64-
val assetLinkUrl = fido2CreateCredentialRequest
65-
.origin
66-
?: getOriginUrlFromAttestationOptionsOrNull(fido2CreateCredentialRequest.requestJson)
67-
?: return Fido2RegisterCredentialResult.Error
68-
69-
val origin = Origin.Android(
70-
UnverifiedAssetLink(
71-
packageName = fido2CreateCredentialRequest.packageName,
72-
sha256CertFingerprint = fido2CreateCredentialRequest
73-
.callingAppInfo
74-
.getSignatureFingerprintAsHexString()
75-
?: return Fido2RegisterCredentialResult.Error,
76-
host = assetLinkUrl.toHostOrPathOrNull()
77-
?: return Fido2RegisterCredentialResult.Error,
78-
assetLinkUrl = assetLinkUrl,
79-
),
80-
)
8185
return vaultSdkSource
8286
.registerFido2Credential(
8387
request = RegisterFido2CredentialRequest(
8488
userId = userId,
85-
origin = origin,
89+
origin = sdkOrigin,
8690
requestJson = """{"publicKey": ${fido2CreateCredentialRequest.requestJson}}""",
8791
clientData = clientData,
8892
selectedCipherView = selectedCipherView,
@@ -96,7 +100,11 @@ class Fido2CredentialManagerImpl(
96100
.mapCatching { json.encodeToString(it) }
97101
.fold(
98102
onSuccess = { Fido2RegisterCredentialResult.Success(it) },
99-
onFailure = { Fido2RegisterCredentialResult.Error },
103+
onFailure = {
104+
Fido2RegisterCredentialResult.Error(
105+
R.string.passkey_registration_failed_due_to_an_internal_error.asText(),
106+
)
107+
},
100108
)
101109
}
102110

@@ -114,9 +122,9 @@ class Fido2CredentialManagerImpl(
114122
): PasskeyAttestationOptions? =
115123
try {
116124
json.decodeFromString<PasskeyAttestationOptions>(requestJson)
117-
} catch (e: SerializationException) {
125+
} catch (_: SerializationException) {
118126
null
119-
} catch (e: IllegalArgumentException) {
127+
} catch (_: IllegalArgumentException) {
120128
null
121129
}
122130

@@ -125,12 +133,13 @@ class Fido2CredentialManagerImpl(
125133
): PasskeyAssertionOptions? =
126134
try {
127135
json.decodeFromString<PasskeyAssertionOptions>(requestJson)
128-
} catch (e: SerializationException) {
136+
} catch (_: SerializationException) {
129137
null
130-
} catch (e: IllegalArgumentException) {
138+
} catch (_: IllegalArgumentException) {
131139
null
132140
}
133141

142+
@Suppress("LongMethod")
134143
override suspend fun authenticateFido2Credential(
135144
userId: String,
136145
request: Fido2CredentialAssertionRequest,
@@ -140,39 +149,52 @@ class Fido2CredentialManagerImpl(
140149
val clientData = request.clientDataHash
141150
?.let { ClientData.DefaultWithCustomHash(hash = it) }
142151
?: ClientData.DefaultWithExtraData(androidPackageName = callingAppInfo.getAppOrigin())
143-
val origin = callingAppInfo.origin
144-
?: getOriginUrlFromAssertionOptionsOrNull(request.requestJson)
145-
?: return Fido2CredentialAssertionResult.Error
146152
val relyingPartyId = json
147153
.decodeFromStringOrNull<PasskeyAssertionOptions>(request.requestJson)
148154
?.relyingPartyId
149-
?: return Fido2CredentialAssertionResult.Error
155+
?: return Fido2CredentialAssertionResult.Error(
156+
R.string.passkey_operation_failed_because_relying_party_cannot_be_identified
157+
.asText(),
158+
)
150159

151160
val validateOriginResult = validateOrigin(
152161
callingAppInfo = callingAppInfo,
153162
relyingPartyId = relyingPartyId,
154163
)
155164

165+
val sdkOrigin = if (!request.origin.isNullOrEmpty()) {
166+
Origin.Web(request.origin)
167+
} else {
168+
val hostUrl = getOriginUrlFromAssertionOptionsOrNull(request.requestJson)
169+
?: return Fido2CredentialAssertionResult.Error(
170+
R.string.passkey_operation_failed_because_host_url_is_not_present_in_request
171+
.asText(),
172+
)
173+
Origin.Android(
174+
UnverifiedAssetLink(
175+
packageName = callingAppInfo.packageName,
176+
sha256CertFingerprint = callingAppInfo.getSignatureFingerprintAsHexString()
177+
?: return Fido2CredentialAssertionResult.Error(
178+
R.string.passkey_operation_failed_because_app_signature_is_invalid
179+
.asText(),
180+
),
181+
host = hostUrl,
182+
assetLinkUrl = hostUrl,
183+
),
184+
)
185+
}
186+
156187
return when (validateOriginResult) {
157188
is Fido2ValidateOriginResult.Error -> {
158-
Fido2CredentialAssertionResult.Error
189+
Fido2CredentialAssertionResult.Error(validateOriginResult.messageResId.asText())
159190
}
160191

161192
is Fido2ValidateOriginResult.Success -> {
162193
vaultSdkSource
163194
.authenticateFido2Credential(
164195
request = AuthenticateFido2CredentialRequest(
165196
userId = userId,
166-
origin = Origin.Android(
167-
UnverifiedAssetLink(
168-
callingAppInfo.packageName,
169-
callingAppInfo.getSignatureFingerprintAsHexString()
170-
?: return Fido2CredentialAssertionResult.Error,
171-
origin.toHostOrPathOrNull()
172-
?: return Fido2CredentialAssertionResult.Error,
173-
origin,
174-
),
175-
),
197+
origin = sdkOrigin,
176198
requestJson = """{"publicKey": ${request.requestJson}}""",
177199
clientData = clientData,
178200
selectedCipherView = selectedCipherView,
@@ -184,7 +206,12 @@ class Fido2CredentialManagerImpl(
184206
.mapCatching { json.encodeToString(it) }
185207
.fold(
186208
onSuccess = { Fido2CredentialAssertionResult.Success(it) },
187-
onFailure = { Fido2CredentialAssertionResult.Error },
209+
onFailure = {
210+
Fido2CredentialAssertionResult.Error(
211+
R.string.passkey_authentication_failed_due_to_an_internal_error
212+
.asText(),
213+
)
214+
},
188215
)
189216
}
190217
}
@@ -196,13 +223,20 @@ class Fido2CredentialManagerImpl(
196223
private fun getOriginUrlFromAssertionOptionsOrNull(requestJson: String) =
197224
getPasskeyAssertionOptionsOrNull(requestJson)
198225
?.relyingPartyId
199-
?.let { "$HTTPS$it" }
226+
?.prefixWithHttpsIfNecessary()
200227

201228
private fun getOriginUrlFromAttestationOptionsOrNull(requestJson: String) =
202229
getPasskeyAttestationOptionsOrNull(requestJson)
203230
?.relyingParty
204231
?.id
205-
?.let { "$HTTPS$it" }
232+
?.prefixWithHttpsIfNecessary()
233+
234+
private fun String.prefixWithHttpsIfNecessary(): String =
235+
if (!this.startsWith(HTTPS)) {
236+
"$HTTPS$this"
237+
} else {
238+
this
239+
}
206240
}
207241

208242
private const val MAX_AUTHENTICATION_ATTEMPTS = 5

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionResult.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.x8bit.bitwarden.data.autofill.fido2.model
22

3+
import com.x8bit.bitwarden.ui.platform.base.util.Text
4+
35
/**
46
* Represents possible outcomes of a FIDO 2 credential assertion request.
57
*/
@@ -13,5 +15,5 @@ sealed class Fido2CredentialAssertionResult {
1315
/**
1416
* Indicates there was an error and the assertion was not successful.
1517
*/
16-
data object Error : Fido2CredentialAssertionResult()
18+
data class Error(val message: Text) : Fido2CredentialAssertionResult()
1719
}

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2RegisterCredentialResult.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.x8bit.bitwarden.data.autofill.fido2.model
22

3+
import com.x8bit.bitwarden.ui.platform.base.util.Text
4+
35
/**
46
* Models the data returned from creating a FIDO 2 credential.
57
*/
@@ -9,13 +11,13 @@ sealed class Fido2RegisterCredentialResult {
911
* Indicates the credential has been successfully registered.
1012
*/
1113
data class Success(
12-
val registrationResponse: String,
14+
val responseJson: String,
1315
) : Fido2RegisterCredentialResult()
1416

1517
/**
1618
* Indicates there was an error and the credential was not registered.
1719
*/
18-
data object Error : Fido2RegisterCredentialResult()
20+
data class Error(val message: Text) : Fido2RegisterCredentialResult()
1921

2022
/**
2123
* Indicates the user cancelled the request.

‎app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ fun VaultUnlockScreen(
118118
)
119119
}
120120

121-
VaultUnlockEvent.Fido2CredentialAssertionError -> {
121+
is VaultUnlockEvent.Fido2CredentialAssertionError -> {
122122
fido2CompletionManager.completeFido2Assertion(
123-
result = Fido2CredentialAssertionResult.Error,
123+
result = Fido2CredentialAssertionResult.Error(event.message),
124124
)
125125
}
126126

‎app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,12 @@ class VaultUnlockViewModel @Inject constructor(
181181
}
182182

183183
state.fido2CredentialAssertionRequest != null -> {
184-
sendEvent(VaultUnlockEvent.Fido2CredentialAssertionError)
184+
sendEvent(
185+
VaultUnlockEvent.Fido2CredentialAssertionError(
186+
R.string.passkey_operation_failed_because_user_could_not_be_verified
187+
.asText(),
188+
),
189+
)
185190
}
186191

187192
else -> Unit
@@ -522,7 +527,7 @@ sealed class VaultUnlockEvent {
522527
/**
523528
* Completes the FIDO2 credential assertion request with an error response.
524529
*/
525-
data object Fido2CredentialAssertionError : VaultUnlockEvent()
530+
data class Fido2CredentialAssertionError(val message: Text) : VaultUnlockEvent()
526531
}
527532

528533
/**

‎app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerImpl.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class Fido2CompletionManagerImpl(
4949
.setCreateCredentialResponse(
5050
intent = intent,
5151
response = CreatePublicKeyCredentialResponse(
52-
registrationResponseJson = result.registrationResponse,
52+
registrationResponseJson = result.responseJson,
5353
),
5454
)
5555
}
@@ -71,7 +71,7 @@ class Fido2CompletionManagerImpl(
7171
activity.also {
7272
val intent = Intent()
7373
when (result) {
74-
Fido2CredentialAssertionResult.Error -> {
74+
is Fido2CredentialAssertionResult.Error -> {
7575
PendingIntentHandler
7676
.setGetCredentialException(
7777
intent = intent,

‎app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
2525
import com.x8bit.bitwarden.R
2626
import com.x8bit.bitwarden.ui.autofill.fido2.manager.Fido2CompletionManager
2727
import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect
28+
import com.x8bit.bitwarden.ui.platform.base.util.Text
2829
import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar
2930
import com.x8bit.bitwarden.ui.platform.components.appbar.NavigationIcon
3031
import com.x8bit.bitwarden.ui.platform.components.appbar.action.BitwardenOverflowActionItem
@@ -131,7 +132,9 @@ fun VaultAddEditScreen(
131132
}
132133

133134
is VaultAddEditEvent.CompleteFido2Registration -> {
134-
fido2CompletionManager.completeFido2Registration(event.result)
135+
fido2CompletionManager.completeFido2Registration(
136+
result = event.result,
137+
)
135138
}
136139

137140
is VaultAddEditEvent.Fido2UserVerification -> {
@@ -187,7 +190,11 @@ fun VaultAddEditScreen(
187190
{ viewModel.trySendAction(VaultAddEditAction.Common.InitialAutofillDialogDismissed) }
188191
},
189192
onFido2ErrorDismiss = remember(viewModel) {
190-
{ viewModel.trySendAction(VaultAddEditAction.Common.Fido2ErrorDialogDismissed) }
193+
{ errorMessage ->
194+
viewModel.trySendAction(
195+
VaultAddEditAction.Common.Fido2ErrorDialogDismissed(message = errorMessage),
196+
)
197+
}
191198
},
192199
onConfirmOverwriteExistingPasskey = remember(viewModel) {
193200
{
@@ -414,7 +421,7 @@ private fun VaultAddEditItemDialogs(
414421
dialogState: VaultAddEditState.DialogState?,
415422
onDismissRequest: () -> Unit,
416423
onAutofillDismissRequest: () -> Unit,
417-
onFido2ErrorDismiss: () -> Unit,
424+
onFido2ErrorDismiss: (Text) -> Unit,
418425
onConfirmOverwriteExistingPasskey: () -> Unit,
419426
onSubmitMasterPasswordFido2Verification: (password: String) -> Unit,
420427
onRetryFido2PasswordVerification: () -> Unit,
@@ -449,7 +456,7 @@ private fun VaultAddEditItemDialogs(
449456
BitwardenBasicDialog(
450457
title = stringResource(id = R.string.an_error_has_occurred),
451458
message = dialogState.message(),
452-
onDismissRequest = onFido2ErrorDismiss,
459+
onDismissRequest = { onFido2ErrorDismiss(dialogState.message) },
453460
)
454461
}
455462

0 commit comments

Comments
 (0)