mirror of
https://github.com/bitwarden/android.git
synced 2026-08-01 18:53:39 -05:00
PM-40278: bug: Freeze public key and fingerprint during auth request and validate against updates (#7166)
This commit is contained in:
David Perez
+39
-20
@@ -27,7 +27,9 @@ import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.isActive
|
||||
import java.time.Clock
|
||||
import java.time.Instant
|
||||
import javax.inject.Singleton
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
|
||||
private const val PASSWORDLESS_NOTIFICATION_TIMEOUT_MILLIS: Long = 15L * 60L * 1_000L
|
||||
private const val PASSWORDLESS_NOTIFICATION_RETRY_INTERVAL_MILLIS: Long = 4L * 1_000L
|
||||
@@ -154,32 +156,49 @@ class AuthRequestManagerImpl(
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("LongMethod")
|
||||
private fun getAuthRequest(
|
||||
initialRequest: suspend () -> AuthRequestUpdatesResult,
|
||||
): Flow<AuthRequestUpdatesResult> = flow {
|
||||
val result = initialRequest()
|
||||
emit(result)
|
||||
if (result is AuthRequestUpdatesResult.Error) return@flow
|
||||
val initialResult = initialRequest()
|
||||
emit(initialResult)
|
||||
if (initialResult !is AuthRequestUpdatesResult.Update) return@flow
|
||||
val initialAuthRequest = initialResult.authRequest
|
||||
var isComplete = false
|
||||
while (currentCoroutineContext().isActive && !isComplete) {
|
||||
delay(PASSWORDLESS_APPROVER_INTERVAL_MILLIS)
|
||||
val updateResult = result as AuthRequestUpdatesResult.Update
|
||||
delay(PASSWORDLESS_APPROVER_INTERVAL_MILLIS.milliseconds)
|
||||
authRequestsService
|
||||
.getAuthRequest(result.authRequest.id)
|
||||
.map { request ->
|
||||
AuthRequest(
|
||||
id = request.id,
|
||||
publicKey = request.publicKey,
|
||||
platform = request.platform,
|
||||
ipAddress = request.ipAddress,
|
||||
key = request.key,
|
||||
masterPasswordHash = request.masterPasswordHash,
|
||||
creationDate = request.creationDate,
|
||||
responseDate = request.responseDate,
|
||||
requestApproved = request.requestApproved ?: false,
|
||||
originUrl = request.originUrl,
|
||||
fingerprint = updateResult.authRequest.fingerprint,
|
||||
)
|
||||
.getAuthRequest(requestId = initialResult.authRequest.id)
|
||||
.flatMap { request ->
|
||||
getFingerprintPhrase(publicKey = request.publicKey).map { fingerprint ->
|
||||
val isRequestApproved: Boolean
|
||||
val responseDate: Instant?
|
||||
if (fingerprint == initialAuthRequest.fingerprint) {
|
||||
// Fingerprint is valid, so we process the valid response.
|
||||
isRequestApproved = request.requestApproved ?: false
|
||||
responseDate = request.responseDate
|
||||
} else {
|
||||
// We cannot gaurentee the authenticity of the response, so we will
|
||||
// hard-decline the request.
|
||||
isRequestApproved = false
|
||||
responseDate = clock.instant()
|
||||
}
|
||||
AuthRequest(
|
||||
id = request.id,
|
||||
platform = request.platform,
|
||||
ipAddress = request.ipAddress,
|
||||
key = request.key,
|
||||
masterPasswordHash = request.masterPasswordHash,
|
||||
creationDate = request.creationDate,
|
||||
originUrl = request.originUrl,
|
||||
responseDate = responseDate,
|
||||
requestApproved = isRequestApproved,
|
||||
// The PublicKey and Fingerprint should be frozen in place to
|
||||
// ensure no funny-business happens between multiple requests.
|
||||
publicKey = initialAuthRequest.publicKey,
|
||||
fingerprint = initialAuthRequest.fingerprint,
|
||||
)
|
||||
}
|
||||
}
|
||||
.fold(
|
||||
onFailure = { emit(AuthRequestUpdatesResult.Error(error = it)) },
|
||||
|
||||
+155
-21
@@ -35,6 +35,7 @@ import org.junit.jupiter.api.Test
|
||||
import java.time.Clock
|
||||
import java.time.Instant
|
||||
import java.time.ZoneOffset
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
|
||||
@Suppress("LargeClass")
|
||||
class AuthRequestManagerTest {
|
||||
@@ -509,9 +510,11 @@ class AuthRequestManagerTest {
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authRequestsService.getAuthRequests()
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@@ -546,9 +549,11 @@ class AuthRequestManagerTest {
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authRequestsService.getAuthRequests()
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@@ -589,9 +594,11 @@ class AuthRequestManagerTest {
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authRequestsService.getAuthRequests()
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@@ -631,9 +638,46 @@ class AuthRequestManagerTest {
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authRequestsService.getAuthRequests()
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `getAuthRequestByFingerprintFlow should emit update then declined and cancel when polled fingerprint does not match initial fingerprint`() =
|
||||
runTest {
|
||||
val responseJsonOne = AuthRequestsResponseJson(
|
||||
authRequests = listOf(AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE),
|
||||
)
|
||||
val expectedOne = AuthRequestUpdatesResult.Update(authRequest = AUTH_REQUEST)
|
||||
val expectedTwo = AuthRequestUpdatesResult.Declined
|
||||
coEvery {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
} returns FINGER_PRINT.asSuccess() andThen "mismatchedFingerprint".asSuccess()
|
||||
coEvery { authRequestsService.getAuthRequests() } returns responseJsonOne.asSuccess()
|
||||
coEvery {
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
} returns AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE.asSuccess()
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE
|
||||
|
||||
repository
|
||||
.getAuthRequestByFingerprintFlow(FINGER_PRINT)
|
||||
.test {
|
||||
assertEquals(expectedOne, awaitItem())
|
||||
assertEquals(expectedTwo, awaitItem())
|
||||
awaitComplete()
|
||||
}
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authRequestsService.getAuthRequests()
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@@ -716,11 +760,9 @@ class AuthRequestManagerTest {
|
||||
awaitComplete()
|
||||
}
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authRequestsService.getAuthRequest(REQUEST_ID)
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -752,11 +794,9 @@ class AuthRequestManagerTest {
|
||||
awaitComplete()
|
||||
}
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authRequestsService.getAuthRequest(REQUEST_ID)
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -792,11 +832,9 @@ class AuthRequestManagerTest {
|
||||
awaitComplete()
|
||||
}
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authRequestsService.getAuthRequest(REQUEST_ID)
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -825,6 +863,101 @@ class AuthRequestManagerTest {
|
||||
} returns authRequestResponseOne andThen authRequestResponseTwo
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE
|
||||
|
||||
repository
|
||||
.getAuthRequestByIdFlow(REQUEST_ID)
|
||||
.test {
|
||||
assertEquals(expectedOne, awaitItem())
|
||||
assertEquals(expectedTwo, awaitItem())
|
||||
cancelAndConsumeRemainingEvents()
|
||||
}
|
||||
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `getAuthRequestByIdFlow should emit update then declined and cancel when polled fingerprint does not match initial fingerprint`() =
|
||||
runTest {
|
||||
val expectedOne = AuthRequestUpdatesResult.Update(authRequest = AUTH_REQUEST)
|
||||
val expectedTwo = AuthRequestUpdatesResult.Declined
|
||||
coEvery {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
} returns FINGER_PRINT.asSuccess() andThen "mismatchedFingerprint".asSuccess()
|
||||
coEvery {
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
} returns AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE.asSuccess()
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE
|
||||
|
||||
repository
|
||||
.getAuthRequestByIdFlow(REQUEST_ID)
|
||||
.test {
|
||||
assertEquals(expectedOne, awaitItem())
|
||||
assertEquals(expectedTwo, awaitItem())
|
||||
awaitComplete()
|
||||
}
|
||||
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `getAuthRequestByIdFlow should emit update then error and not cancel when getUserFingerprint fails during polling`() =
|
||||
runTest {
|
||||
val error = Throwable("Fail")
|
||||
val expectedOne = AuthRequestUpdatesResult.Update(authRequest = AUTH_REQUEST)
|
||||
val expectedTwo = AuthRequestUpdatesResult.Error(error = error)
|
||||
coEvery {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
} returns FINGER_PRINT.asSuccess() andThen error.asFailure()
|
||||
coEvery {
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
} returns AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE.asSuccess()
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE
|
||||
|
||||
repository
|
||||
.getAuthRequestByIdFlow(REQUEST_ID)
|
||||
.test {
|
||||
assertEquals(expectedOne, awaitItem())
|
||||
assertEquals(expectedTwo, awaitItem())
|
||||
cancelAndConsumeRemainingEvents()
|
||||
}
|
||||
|
||||
coVerify(exactly = 2) {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `getAuthRequestByIdFlow should emit update with initial publicKey when polled request returns a new publicKey with matching fingerprint`() =
|
||||
runTest {
|
||||
val newPublicKey = "newPublicKey"
|
||||
val authRequestResponseOne = AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE.asSuccess()
|
||||
val authRequestResponseTwo = AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE
|
||||
.copy(publicKey = newPublicKey, requestApproved = false)
|
||||
.asSuccess()
|
||||
val expectedOne = AuthRequestUpdatesResult.Update(authRequest = AUTH_REQUEST)
|
||||
val expectedTwo = AuthRequestUpdatesResult.Update(
|
||||
authRequest = AUTH_REQUEST.copy(requestApproved = false),
|
||||
)
|
||||
coEvery {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
} returns FINGER_PRINT.asSuccess()
|
||||
coEvery {
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = newPublicKey)
|
||||
} returns FINGER_PRINT.asSuccess()
|
||||
coEvery {
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
} returns authRequestResponseOne andThen authRequestResponseTwo
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE
|
||||
|
||||
repository
|
||||
.getAuthRequestByIdFlow(REQUEST_ID)
|
||||
.test {
|
||||
@@ -834,10 +967,11 @@ class AuthRequestManagerTest {
|
||||
}
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
authSdkSource.getUserFingerprint(EMAIL, PUBLIC_KEY)
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
|
||||
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = newPublicKey)
|
||||
}
|
||||
coVerify(exactly = 2) {
|
||||
authRequestsService.getAuthRequest(REQUEST_ID)
|
||||
authRequestsService.getAuthRequest(requestId = REQUEST_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -866,11 +1000,11 @@ class AuthRequestManagerTest {
|
||||
.getAuthRequestsWithUpdates()
|
||||
.test {
|
||||
assertEquals(expectedOne, awaitItem())
|
||||
advanceTimeBy(threeMinutes)
|
||||
advanceTimeBy(threeMinutes.milliseconds)
|
||||
expectNoEvents()
|
||||
advanceTimeBy(threeMinutes)
|
||||
advanceTimeBy(threeMinutes.milliseconds)
|
||||
assertEquals(expectedTwo, awaitItem())
|
||||
advanceTimeBy(threeMinutes)
|
||||
advanceTimeBy(threeMinutes.milliseconds)
|
||||
cancelAndIgnoreRemainingEvents()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user