Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure accounts are always derived for privileged apps #370

Merged
merged 1 commit into from
Nov 23, 2024
Merged

Ensure accounts are always derived for privileged apps #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions ...tor/src/main/java/com/solanamobile/seedvaultimpl/contentprovider/WalletContentProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,20 +549,24 @@ class WalletContentProvider : ContentProvider() {
SeedRepository.ChangeNotification.Type.CREATE ->
listOf(
WalletContractV1.UNAUTHORIZED_SEEDS_CONTENT_URI,
ContentUris.withAppendedId(
WalletContractV1.AUTHORIZED_SEEDS_CONTENT_URI,
change.id!!.toLong()
)
change.id?.let {
ContentUris.withAppendedId(
WalletContractV1.AUTHORIZED_SEEDS_CONTENT_URI,
change.id.toLong()
)
} ?: WalletContractV1.AUTHORIZED_SEEDS_CONTENT_URI
)
SeedRepository.ChangeNotification.Type.UPDATE ->
throw AssertionError("Authorizations are not expected to be updated")
SeedRepository.ChangeNotification.Type.DELETE ->
listOf(
WalletContractV1.UNAUTHORIZED_SEEDS_CONTENT_URI,
ContentUris.withAppendedId(
WalletContractV1.AUTHORIZED_SEEDS_CONTENT_URI,
change.id!!.toLong()
),
change.id?.let {
ContentUris.withAppendedId(
WalletContractV1.AUTHORIZED_SEEDS_CONTENT_URI,
change.id.toLong()
)
} ?: WalletContractV1.AUTHORIZED_SEEDS_CONTENT_URI,
WalletContractV1.ACCOUNTS_CONTENT_URI
)
}
Expand All @@ -572,10 +576,12 @@ class WalletContentProvider : ContentProvider() {
SeedRepository.ChangeNotification.Type.CREATE,
SeedRepository.ChangeNotification.Type.UPDATE ->
listOf(
ContentUris.withAppendedId(
WalletContractV1.ACCOUNTS_CONTENT_URI,
change.id!!.toLong()
)
change.id?.let {
ContentUris.withAppendedId(
WalletContractV1.ACCOUNTS_CONTENT_URI,
change.id.toLong()
)
} ?: WalletContractV1.ACCOUNTS_CONTENT_URI
)
SeedRepository.ChangeNotification.Type.DELETE ->
throw AssertionError("Accounts are not expected to be deleted")
Expand Down
142 changes: 49 additions & 93 deletions SeedVaultSimulator/src/main/java/com/solanamobile/seedvaultimpl/data/SeedRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,23 +162,18 @@ class SeedRepository @Inject constructor(
check(!seeds.value.containsKey(nextSeedId)) { "Seed repository already contains an entry for seed $nextSeedId" }
newSeedRecordBuilder.seedId = nextSeedId
id = nextSeedId
val changeNotification = ChangeNotification(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of the changes in this file are strictly necessary, but it cuts down on unnecessary change notifications when nothing actually changed, which improves performance of apps that make use of content notifications.

ChangeNotification.Category.SEED, ChangeNotification.Type.CREATE, id
)

updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
it.toBuilder().addSeeds(newSeedRecordBuilder).apply {
nextId = ++nextSeedId
}.build()
}
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.SEED,
ChangeNotification.Type.CREATE,
id
)
)
}

Log.d(TAG, "EXIT createSeed: $details -> $id")
Expand All @@ -196,8 +191,11 @@ class SeedRepository @Inject constructor(
val newSeedEntryBuilder = createSeedEntryBuilderFromSeed(details)

val updateCompleteJob: Job
val changeNotification = ChangeNotification(
ChangeNotification.Category.SEED, ChangeNotification.Type.UPDATE, id
)
mutex.withLock {
updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { seed -> seed.seedId == id }
check(i != -1) { "Seed repository does not contain an entry for seed $id" }
val newSeedRecordBuilder =
Expand All @@ -207,14 +205,6 @@ class SeedRepository @Inject constructor(
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.SEED,
ChangeNotification.Type.UPDATE,
id
)
)
}

Log.d(TAG, "EXIT updateSeed: $details")
Expand All @@ -229,23 +219,18 @@ class SeedRepository @Inject constructor(
// in the event of cancellation of the originating context.
withContext(repositoryOwnerScope.coroutineContext) {
val updateCompleteJob: Job
val changeNotification = ChangeNotification(
ChangeNotification.Category.SEED, ChangeNotification.Type.DELETE, id
)
mutex.withLock {
updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { seed -> seed.seedId == id }
require(i != -1) { "Seed repository does not contain an entry for seed $id" }
it.toBuilder().removeSeeds(i).build()
}
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.SEED,
ChangeNotification.Type.DELETE,
id
)
)
}

Log.d(TAG, "EXIT deleteSeed: $id")
Expand All @@ -260,21 +245,16 @@ class SeedRepository @Inject constructor(
// in the event of cancellation of the originating context.
withContext(repositoryOwnerScope.coroutineContext) {
val updateCompleteJob: Job
val changeNotification = ChangeNotification(
ChangeNotification.Category.SEED, ChangeNotification.Type.DELETE, null
)
mutex.withLock {
updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
it.toBuilder().clearSeeds().build()
}
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.SEED,
ChangeNotification.Type.DELETE,
null
)
)
}

Log.d(TAG, "EXIT deleteAllSeeds")
Expand All @@ -301,8 +281,13 @@ class SeedRepository @Inject constructor(
mutex.withLock {
var assignedAuthToken = nextAuthToken
newAuthorizationEntryBuilder.authToken = nextAuthToken
val changeNotification = ChangeNotification(
ChangeNotification.Category.AUTHORIZATION,
ChangeNotification.Type.CREATE,
assignedAuthToken
)

updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { sr -> sr.seedId == id }
require(i != -1) { "Seed repository does not contain an entry for seed $id" }
val existingAuthRecord =
Expand All @@ -323,14 +308,6 @@ class SeedRepository @Inject constructor(
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.AUTHORIZATION,
ChangeNotification.Type.CREATE,
authToken
)
)
}

Log.d(TAG, "EXIT authorizeSeedForUid: $id/$uid -> $authToken")
Expand All @@ -346,9 +323,11 @@ class SeedRepository @Inject constructor(
// in the event of cancellation of the originating context.
withContext(repositoryOwnerScope.coroutineContext) {
val updateAllSeedsJob: Job
val approvedAuthTokens = mutableListOf<Long>()
val changeNotification = ChangeNotification(
ChangeNotification.Category.AUTHORIZATION, ChangeNotification.Type.CREATE, null
)
mutex.withLock {
updateAllSeedsJob = updateSeedCollectionDataStore { seedCollection ->
updateAllSeedsJob = updateSeedCollectionDataStore(changeNotification) { seedCollection ->
val newSeedCollection = seedCollection.toBuilder()
seedCollection.seedsList.forEachIndexed { index, seedRecord ->
val existingAuthRecord =
Expand All @@ -360,7 +339,6 @@ class SeedRepository @Inject constructor(
this.purpose = purpose.ordinal
this.authToken = nextAuthToken++
}
approvedAuthTokens.add(newAuthorizationEntryBuilder.authToken)
val newSeedRecordBuilder = seedRecord.toBuilder()
.addAuthorizations(newAuthorizationEntryBuilder)
newSeedRecordBuilder.build()
Expand All @@ -375,15 +353,6 @@ class SeedRepository @Inject constructor(
}
}
updateAllSeedsJob.join()
approvedAuthTokens.forEach {
_changes.emit(
ChangeNotification(
ChangeNotification.Category.AUTHORIZATION,
ChangeNotification.Type.CREATE,
it
)
)
}
}
Log.d(TAG, "EXIT authorizeAllSeedsForUid")
}
Expand All @@ -397,8 +366,11 @@ class SeedRepository @Inject constructor(
// in the event of cancellation of the originating context.
withContext(repositoryOwnerScope.coroutineContext) {
val updateCompleteJob: Job
val changeNotification = ChangeNotification(
ChangeNotification.Category.AUTHORIZATION, ChangeNotification.Type.DELETE, authToken
)
mutex.withLock {
updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { sr -> sr.seedId == id }
require(i != -1) { "Seed repository does not contain an entry for seed $id" }
val j =
Expand All @@ -410,14 +382,6 @@ class SeedRepository @Inject constructor(
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.AUTHORIZATION,
ChangeNotification.Type.DELETE,
authToken
)
)
}

Log.d(TAG, "EXIT deauthorizeSeed: $id/$authToken")
Expand Down Expand Up @@ -450,8 +414,13 @@ class SeedRepository @Inject constructor(
mutex.withLock {
var assignedAccountId = nextAccountId
newKnownAccountEntryBuilder.accountId = nextAccountId
val changeNotification = ChangeNotification(
ChangeNotification.Category.ACCOUNT,
ChangeNotification.Type.CREATE,
assignedAccountId
)

updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { sr -> sr.seedId == id }
require(i != -1) { "Seed repository does not contain an entry for seed $id" }
val bip32Uri = account.bip32DerivationPathUri.toString()
Expand All @@ -475,14 +444,6 @@ class SeedRepository @Inject constructor(
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.ACCOUNT,
ChangeNotification.Type.CREATE,
accountId
)
)
}

Log.d(TAG, "EXIT addKnownAccountForSeed")
Expand All @@ -499,8 +460,11 @@ class SeedRepository @Inject constructor(
// in the event of cancellation of the originating context.
withContext(repositoryOwnerScope.coroutineContext) {
val updateCompleteJob: Job
val changeNotification = ChangeNotification(
ChangeNotification.Category.ACCOUNT, ChangeNotification.Type.DELETE, null
)
mutex.withLock {
updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { sr -> sr.seedId == id }
require(i != -1) { "Seed repository does not contain an entry for seed $id" }
val newSeedRecordBuilder =
Expand All @@ -510,13 +474,6 @@ class SeedRepository @Inject constructor(
}

updateCompleteJob.join()
_changes.emit(
ChangeNotification(
ChangeNotification.Category.ACCOUNT,
ChangeNotification.Type.DELETE,
null
)
)
}

Log.d(TAG, "EXIT removeAllKnownAccountForSeed")
Expand Down Expand Up @@ -544,8 +501,11 @@ class SeedRepository @Inject constructor(
}

val updateCompleteJob: Job
val changeNotification = ChangeNotification(
ChangeNotification.Category.ACCOUNT, ChangeNotification.Type.UPDATE, account.id
)
mutex.withLock {
updateCompleteJob = updateSeedCollectionDataStore {
updateCompleteJob = updateSeedCollectionDataStore(changeNotification) {
val i = it.seedsList.indexOfFirst { sr -> sr.seedId == id }
require(i != -1) { "Seed repository does not contain an entry for seed $id" }
val j =
Expand All @@ -558,14 +518,6 @@ class SeedRepository @Inject constructor(
}

updateCompleteJob.join()

_changes.emit(
ChangeNotification(
ChangeNotification.Category.ACCOUNT,
ChangeNotification.Type.UPDATE,
account.id
)
)
}

Log.d(TAG, "EXIT updateKnownAccountForSeed")
Expand All @@ -583,12 +535,16 @@ class SeedRepository @Inject constructor(
}

// NOTE: should be called with mutex held
private suspend fun updateSeedCollectionDataStore(transform: suspend (t: SeedCollection) -> SeedCollection): Job {
private suspend fun updateSeedCollectionDataStore(
changeNotification: ChangeNotification,
transform: suspend (t: SeedCollection) -> SeedCollection
): Job {
// Launch undispatched, so that the first element of seedCollection (which is a replay of
// the last emitted value) is collected immediately.
val updateCompleteJob = repositoryOwnerScope.launch(start = CoroutineStart.UNDISPATCHED) {
withTimeout(CHANGE_PROPAGATION_TIMEOUT_MS) {
seedCollection.take(2).collect()
_changes.emit(changeNotification)
}
}

Expand Down
15 changes: 9 additions & 6 deletions ...mulator/src/main/java/com/solanamobile/seedvaultimpl/ui/seeddetail/SeedDetailViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,17 @@ class SeedDetailViewModel @Inject constructor(
when (val mode = mode) { // immutable snapshot of mode
is NewSeedMode -> {
val seedId = seedRepository.createSeed(seedDetails)
mode.authorize?.let { authorize ->
val authToken = seedRepository.authorizeSeedForUid(seedId, authorize.uid, authorize.purpose)

// Ensure that the seed vault contains appropriate known accounts for this authorization purpose
val seed = seedRepository.seeds.value[seedId]!!
PrepopulateKnownAccountsUseCase(seedRepository).populateKnownAccounts(seed, authorize.purpose)
// Pre-populate known accounts for all purposes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the fix for the root cause of known accounts not being derived.

val seed = seedRepository.seeds.value[seedId]!!
PrepopulateKnownAccountsUseCase(seedRepository).apply {
for (purpose in Authorization.Purpose.entries) {
populateKnownAccounts(seed, purpose)
}
}

authToken
mode.authorize?.let { authorize ->
seedRepository.authorizeSeedForUid(seedId, authorize.uid, authorize.purpose)
} ?: -1L
}
is EditSeedMode -> {
Expand Down
Loading