Skip to content

Commit 6071abb

Browse files
taridepacotaridepaco
authored
chore: [ANDROSDK-2315] Implement multi account and offline for OpenId connections (#2628)
2 parents fde9b58 + f2f6050 commit 6071abb

16 files changed

Lines changed: 621 additions & 13 deletions

File tree

‎core/api/core.api‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19914,11 +19914,20 @@ public final class org/hisp/dhis/android/core/user/openid/OpenIDConnectConfig {
1991419914
}
1991519915

1991619916
public abstract interface class org/hisp/dhis/android/core/user/openid/OpenIDConnectHandler {
19917+
public fun blockingChangePin (Ljava/lang/String;Ljava/lang/String;)Lorg/hisp/dhis/android/core/arch/helpers/Result;
1991719918
public abstract fun blockingHandleLogInResponse (Ljava/lang/String;Landroid/content/Intent;I)Lorg/hisp/dhis/android/core/user/User;
1991819919
public abstract fun blockingLogIn (Lorg/hisp/dhis/android/core/user/openid/OpenIDConnectConfig;)Lorg/hisp/dhis/android/core/user/openid/IntentWithRequestCode;
19920+
public fun blockingSetPin (Ljava/lang/String;)Lorg/hisp/dhis/android/core/arch/helpers/Result;
1991919921
public abstract fun handleLogInResponse (Ljava/lang/String;Landroid/content/Intent;I)Lio/reactivex/Single;
1992019922
public abstract fun logIn (Lorg/hisp/dhis/android/core/user/openid/OpenIDConnectConfig;)Lio/reactivex/Single;
1992119923
public abstract fun logOutObservable ()Lio/reactivex/Observable;
19924+
public abstract fun suspendChangePin (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
19925+
public abstract fun suspendSetPin (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
19926+
}
19927+
19928+
public final class org/hisp/dhis/android/core/user/openid/OpenIDConnectHandler$DefaultImpls {
19929+
public static fun blockingChangePin (Lorg/hisp/dhis/android/core/user/openid/OpenIDConnectHandler;Ljava/lang/String;Ljava/lang/String;)Lorg/hisp/dhis/android/core/arch/helpers/Result;
19930+
public static fun blockingSetPin (Lorg/hisp/dhis/android/core/user/openid/OpenIDConnectHandler;Ljava/lang/String;)Lorg/hisp/dhis/android/core/arch/helpers/Result;
1992219931
}
1992319932

1992419933
public final class org/hisp/dhis/android/core/util/StringUtils {

‎core/src/androidTest/java/org/hisp/dhis/android/core/user/openid/OpenIDConnectStateSecureStoreShould.kt‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright (c) 2004-2026, University of Oslo
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are met:
7+
* Redistributions of source code must retain the above copyright notice, this
8+
* list of conditions and the following disclaimer.
9+
*
10+
* Redistributions in binary form must reproduce the above copyright notice,
11+
* this list of conditions and the following disclaimer in the documentation
12+
* and/or other materials provided with the distribution.
13+
* Neither the name of the HISP project nor the names of its contributors may
14+
* be used to endorse or promote products derived from this software without
15+
* specific prior written permission.
16+
*
17+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
18+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
19+
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
20+
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
21+
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
22+
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
23+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
24+
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
26+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
*/
28+
package org.hisp.dhis.android.core.user.openid
29+
30+
import com.google.common.truth.Truth.assertThat
31+
import net.openid.appauth.AuthState
32+
import org.hisp.dhis.android.core.arch.storage.internal.ChunkedSecureStore
33+
import org.hisp.dhis.android.core.arch.storage.internal.InMemorySecureStore
34+
import org.junit.Before
35+
import org.junit.Test
36+
37+
/**
38+
* Instrumented test: [AuthState] serialization relies on org.json (and Uri) which are only
39+
* available on a device/emulator, and AuthState has no value-based equals, so states are
40+
* compared through their serialized JSON.
41+
*/
42+
class OpenIDConnectStateSecureStoreShould {
43+
44+
private lateinit var store: OpenIDConnectStateSecureStore
45+
46+
// refreshToken is one of the simple top-level keys preserved by AuthState (de)serialization.
47+
private val stateA = AuthState.jsonDeserialize("""{"refreshToken":"refresh-A"}""")
48+
private val stateB = AuthState.jsonDeserialize("""{"refreshToken":"refresh-B"}""")
49+
50+
@Before
51+
fun setUp() {
52+
store = OpenIDConnectStateSecureStore(ChunkedSecureStore(InMemorySecureStore()))
53+
}
54+
55+
@Test
56+
fun should_return_null_for_unknown_account() {
57+
assertThat(store.get("https://play.dhis2.org", "admin")).isNull()
58+
}
59+
60+
@Test
61+
fun should_round_trip_state() {
62+
store.set("https://play.dhis2.org", "admin", stateA)
63+
64+
assertThat(store.get("https://play.dhis2.org", "admin")?.jsonSerializeString())
65+
.isEqualTo(stateA.jsonSerializeString())
66+
}
67+
68+
@Test
69+
fun should_isolate_entries_per_account() {
70+
store.set("https://play.dhis2.org", "admin", stateA)
71+
store.set("https://play.dhis2.org", "user", stateB)
72+
store.set("https://other.dhis2.org", "admin", stateB)
73+
74+
assertThat(store.get("https://play.dhis2.org", "admin")?.jsonSerializeString())
75+
.isEqualTo(stateA.jsonSerializeString())
76+
assertThat(store.get("https://play.dhis2.org", "user")?.jsonSerializeString())
77+
.isEqualTo(stateB.jsonSerializeString())
78+
assertThat(store.get("https://other.dhis2.org", "admin")?.jsonSerializeString())
79+
.isEqualTo(stateB.jsonSerializeString())
80+
}
81+
82+
@Test
83+
fun should_treat_normalized_server_url_variants_as_same_account() {
84+
store.set("https://play.dhis2.org", "admin", stateA)
85+
86+
assertThat(store.get("HTTPS://PLAY.DHIS2.ORG/", "admin")?.jsonSerializeString())
87+
.isEqualTo(stateA.jsonSerializeString())
88+
assertThat(store.get("https://play.dhis2.org/api", "admin")?.jsonSerializeString())
89+
.isEqualTo(stateA.jsonSerializeString())
90+
}
91+
92+
@Test
93+
fun should_remove_only_the_target_account() {
94+
store.set("https://play.dhis2.org", "admin", stateA)
95+
store.set("https://play.dhis2.org", "user", stateB)
96+
97+
store.remove("https://play.dhis2.org", "admin")
98+
99+
assertThat(store.get("https://play.dhis2.org", "admin")).isNull()
100+
assertThat(store.get("https://play.dhis2.org", "user")?.jsonSerializeString())
101+
.isEqualTo(stateB.jsonSerializeString())
102+
}
103+
104+
@Test
105+
fun should_overwrite_existing_state_for_same_account() {
106+
store.set("https://play.dhis2.org", "admin", stateA)
107+
store.set("https://play.dhis2.org", "admin", stateB)
108+
109+
assertThat(store.get("https://play.dhis2.org", "admin")?.jsonSerializeString())
110+
.isEqualTo(stateB.jsonSerializeString())
111+
}
112+
}

‎core/src/main/java/org/hisp/dhis/android/core/arch/api/authentication/internal/OpenIDConnectAuthenticator.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import io.ktor.client.request.header
3434
import org.hisp.dhis.android.core.arch.storage.internal.Credentials
3535
import org.hisp.dhis.android.core.arch.storage.internal.CredentialsSecureStore
3636
import org.hisp.dhis.android.core.user.openid.OpenIDConnectLogoutHandler
37+
import org.hisp.dhis.android.core.user.openid.OpenIDConnectStateSecureStore
3738
import org.hisp.dhis.android.core.user.openid.OpenIDConnectTokenRefresher
3839
import org.koin.core.annotation.Singleton
3940

@@ -45,6 +46,7 @@ internal class OpenIDConnectAuthenticator(
4546
private val tokenRefresher: OpenIDConnectTokenRefresher,
4647
private val userIdHelper: UserIdAuthenticatorHelper,
4748
private val logoutHandler: OpenIDConnectLogoutHandler,
49+
private val openIDConnectStateSecureStore: OpenIDConnectStateSecureStore,
4850
) {
4951

5052
suspend fun handleTokenCall(
@@ -68,6 +70,8 @@ internal class OpenIDConnectAuthenticator(
6870
return if (state.needsTokenRefresh) {
6971
val token = tokenRefresher.blockingGetFreshToken(state)
7072
credentialsSecureStore.set(credentials) // Auth state internally updated
73+
// Keep the per-account persisted state fresh so relogin uses the latest refresh token.
74+
openIDConnectStateSecureStore.set(credentials.serverUrl, credentials.username, state)
7175
token
7276
} else {
7377
state.idToken!!

‎core/src/main/java/org/hisp/dhis/android/core/user/internal/AccountManagerImpl.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import org.hisp.dhis.android.core.maintenance.D2ErrorComponent
4747
import org.hisp.dhis.android.core.user.AccountDeletionReason
4848
import org.hisp.dhis.android.core.user.AccountManager
4949
import org.hisp.dhis.android.core.user.oauth2.internal.OAuth2StateSecureStore
50+
import org.hisp.dhis.android.core.user.openid.OpenIDConnectStateSecureStore
5051
import org.koin.core.annotation.Singleton
5152

5253
@Singleton
@@ -60,6 +61,7 @@ internal class AccountManagerImpl(
6061
private val context: Context,
6162
private val databaseConfigurationHelper: DatabaseConfigurationHelper,
6263
private val oauth2StateSecureStore: OAuth2StateSecureStore,
64+
private val openIDConnectStateSecureStore: OpenIDConnectStateSecureStore,
6365
) : AccountManager {
6466
private val accountDeletionSubject = PublishSubject.create<AccountDeletionReason>()
6567

@@ -147,6 +149,7 @@ internal class AccountManagerImpl(
147149
FileResourceDirectoryHelper.deleteFileResourceDirectories(context, loggedAccount)
148150
databaseManager.deleteDatabase(loggedAccount.databaseName(), loggedAccount.encrypted())
149151
oauth2StateSecureStore.remove(credentials.serverUrl, credentials.username)
152+
openIDConnectStateSecureStore.remove(credentials.serverUrl, credentials.username)
150153
}
151154
}
152155

‎core/src/main/java/org/hisp/dhis/android/core/user/internal/LogInCall.kt‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ import org.hisp.dhis.android.core.user.AuthenticatedUser
4242
import org.hisp.dhis.android.core.user.User
4343
import org.hisp.dhis.android.core.user.oauth2.OAuth2State
4444
import org.hisp.dhis.android.core.user.oauth2.internal.OAuth2StateSecureStore
45+
import org.hisp.dhis.android.core.user.openid.OpenIDConnectStateSecureStore
46+
import org.hisp.dhis.android.core.user.openid.OpenIDConnectTokenRefresher
4547
import org.koin.core.annotation.Singleton
4648

4749
@Singleton
@@ -60,6 +62,8 @@ internal class LogInCall(
6062
private val accountManager: AccountManagerImpl,
6163
private val apiCallErrorCatcher: UserAuthenticateCallErrorCatcher,
6264
private val oauth2StateSecureStore: OAuth2StateSecureStore,
65+
private val openIDConnectStateSecureStore: OpenIDConnectStateSecureStore,
66+
private val openIDConnectTokenRefresher: Lazy<OpenIDConnectTokenRefresher>,
6367
) {
6468
@Throws(D2Error::class)
6569
suspend fun logIn(username: String?, password: String?, serverUrl: String?): User {
@@ -71,11 +75,16 @@ internal class LogInCall(
7175
ServerURLWrapper.setServerUrl(parsedServerUrl.toString())
7276

7377
val oauth2State = oauth2StateSecureStore.get(trimmedServerUrl!!, username!!)
74-
return if (oauth2State != null) {
75-
logInWithOAuth2State(trimmedServerUrl, username, password, oauth2State)
76-
} else {
77-
exceptions.throwExceptionIfPasswordNull(password)
78-
logInWithPassword(trimmedServerUrl, username, password)
78+
val openIdState = openIDConnectStateSecureStore.get(trimmedServerUrl, username)
79+
return when {
80+
oauth2State != null ->
81+
logInWithOAuth2State(trimmedServerUrl, username, password, oauth2State)
82+
openIdState != null ->
83+
logInWithOpenIdState(trimmedServerUrl, username, password, openIdState)
84+
else -> {
85+
exceptions.throwExceptionIfPasswordNull(password)
86+
logInWithPassword(trimmedServerUrl, username, password)
87+
}
7988
}
8089
}
8190

@@ -100,6 +109,31 @@ internal class LogInCall(
100109
}
101110
}
102111

112+
private suspend fun logInWithOpenIdState(
113+
serverUrl: String,
114+
username: String,
115+
pin: String?,
116+
state: AuthState,
117+
): User {
118+
val credentials = Credentials(username, serverUrl, pin, state, null)
119+
// Try to refresh the idToken first; if it fails (offline or invalid refresh token) fall
120+
// back to the stored idToken and let the network call decide between online and offline.
121+
val token = openIDConnectTokenRefresher.value.blockingGetFreshTokenOrNull(state) ?: state.idToken!!
122+
return try {
123+
val user = coroutineAPICallExecutor.wrap(errorCatcher = apiCallErrorCatcher) {
124+
networkHandler.authenticate("Bearer $token")
125+
}.getOrThrow()
126+
openIDConnectStateSecureStore.set(serverUrl, username, state)
127+
loginOnline(user, credentials)
128+
} catch (d2Error: D2Error) {
129+
if (d2Error.isOffline) {
130+
tryLoginOffline(credentials, d2Error)
131+
} else {
132+
throw handleOnlineException(d2Error, credentials)
133+
}
134+
}
135+
}
136+
103137
private suspend fun logInWithPassword(
104138
serverUrl: String,
105139
username: String,
@@ -159,7 +193,7 @@ internal class LogInCall(
159193

160194
return coroutineAPICallExecutor.wrapTransactionallyRoom {
161195
try {
162-
if (credentials.oauth2State != null) {
196+
if (credentials.oauth2State != null || credentials.openIDConnectState != null) {
163197
verifyPinAgainstStoredHash(credentials)
164198
}
165199
val authenticatedUser = AuthenticatedUser.builder()

‎core/src/main/java/org/hisp/dhis/android/core/user/internal/LogInExceptions.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@ internal class LogInExceptions internal constructor(
104104
.build()
105105
}
106106

107-
fun pinRequiresOAuth2AccountError(): D2Error {
107+
fun pinRequiresTokenBasedAccountError(): D2Error {
108108
return D2Error.builder()
109109
.errorCode(D2ErrorCode.INVALID_CONFIGURATION)
110-
.errorDescription("PIN can only be set for OAuth2 accounts")
110+
.errorDescription("PIN can only be set for OAuth2 or OpenID Connect accounts")
111111
.errorComponent(D2ErrorComponent.SDK)
112112
.build()
113113
}

‎core/src/main/java/org/hisp/dhis/android/core/user/oauth2/internal/OAuth2HandlerImpl.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ internal class OAuth2HandlerImpl(
201201
val credentials = credentialsSecureStore.get()
202202
return when {
203203
credentials == null -> Result.Failure(logInExceptions.noActiveSessionError())
204-
credentials.oauth2State == null -> Result.Failure(logInExceptions.pinRequiresOAuth2AccountError())
204+
credentials.oauth2State == null -> Result.Failure(logInExceptions.pinRequiresTokenBasedAccountError())
205205
else -> {
206206
val updated = credentials.copy(password = pin)
207207
credentialsSecureStore.set(updated)

‎core/src/main/java/org/hisp/dhis/android/core/user/openid/OpenIDConnectHandler.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ package org.hisp.dhis.android.core.user.openid
3131
import android.content.Intent
3232
import io.reactivex.Observable
3333
import io.reactivex.Single
34+
import kotlinx.coroutines.runBlocking
35+
import org.hisp.dhis.android.core.arch.helpers.Result
36+
import org.hisp.dhis.android.core.maintenance.D2Error
3437
import org.hisp.dhis.android.core.user.User
3538

3639
interface OpenIDConnectHandler {
@@ -39,4 +42,13 @@ interface OpenIDConnectHandler {
3942
fun handleLogInResponse(serverUrl: String, intent: Intent?, requestCode: Int): Single<User>
4043
fun blockingHandleLogInResponse(serverUrl: String, intent: Intent?, requestCode: Int): User
4144
fun logOutObservable(): Observable<Unit>
45+
46+
suspend fun suspendSetPin(pin: String): Result<Unit, D2Error>
47+
48+
fun blockingSetPin(pin: String): Result<Unit, D2Error> = runBlocking { suspendSetPin(pin) }
49+
50+
suspend fun suspendChangePin(currentPin: String, newPin: String): Result<Unit, D2Error>
51+
52+
fun blockingChangePin(currentPin: String, newPin: String): Result<Unit, D2Error> =
53+
runBlocking { suspendChangePin(currentPin, newPin) }
4254
}

‎core/src/main/java/org/hisp/dhis/android/core/user/openid/OpenIDConnectHandlerImpl.kt‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,27 @@ import io.reactivex.Single
3535
import io.reactivex.schedulers.Schedulers
3636
import kotlinx.coroutines.runBlocking
3737
import net.openid.appauth.*
38+
import org.hisp.dhis.android.core.arch.helpers.Result
39+
import org.hisp.dhis.android.core.arch.storage.internal.CredentialsSecureStore
40+
import org.hisp.dhis.android.core.maintenance.D2Error
3841
import org.hisp.dhis.android.core.user.User
42+
import org.hisp.dhis.android.core.user.internal.AuthenticatedUserStore
3943
import org.hisp.dhis.android.core.user.internal.LogInCall
44+
import org.hisp.dhis.android.core.user.internal.LogInExceptions
4045
import org.koin.core.annotation.Singleton
4146

4247
private const val RC_AUTH = 2021
4348

49+
@Suppress("LongParameterList")
4450
@Singleton
4551
internal class OpenIDConnectHandlerImpl(
4652
private val context: Context,
4753
private val logInCall: LogInCall,
4854
private val logoutHandler: OpenIDConnectLogoutHandler,
55+
private val openIDConnectStateSecureStore: OpenIDConnectStateSecureStore,
56+
private val credentialsSecureStore: CredentialsSecureStore,
57+
private val authenticatedUserStore: AuthenticatedUserStore,
58+
private val logInExceptions: LogInExceptions,
4959
) : OpenIDConnectHandler {
5060

5161
override fun logIn(config: OpenIDConnectConfig): Single<IntentWithRequestCode> {
@@ -74,9 +84,11 @@ internal class OpenIDConnectHandlerImpl(
7484
val response = AuthorizationResponse.fromIntent(intent)!!
7585
downloadToken(response.createTokenExchangeRequest())
7686
.observeOn(Schedulers.io())
77-
.map {
87+
.map { authState ->
7888
runBlocking {
79-
logInCall.blockingLogInOpenIDConnect(serverUrl, it)
89+
val user = logInCall.blockingLogInOpenIDConnect(serverUrl, authState)
90+
openIDConnectStateSecureStore.set(serverUrl, user.username()!!, authState)
91+
user
8092
}
8193
}
8294
}
@@ -97,6 +109,37 @@ internal class OpenIDConnectHandlerImpl(
97109
return logoutHandler.logOutObservable()
98110
}
99111

112+
override suspend fun suspendSetPin(pin: String): Result<Unit, D2Error> {
113+
val credentials = credentialsSecureStore.get()
114+
return when {
115+
credentials == null -> Result.Failure(logInExceptions.noActiveSessionError())
116+
credentials.openIDConnectState == null ->
117+
Result.Failure(logInExceptions.pinRequiresTokenBasedAccountError())
118+
else -> {
119+
val updated = credentials.copy(password = pin)
120+
credentialsSecureStore.set(updated)
121+
val existing = authenticatedUserStore.selectFirst()
122+
if (existing == null) {
123+
Result.Failure(logInExceptions.noAuthenticatedUserPersistedError())
124+
} else {
125+
authenticatedUserStore.updateOrInsertWhere(
126+
existing.toBuilder().hash(updated.getHash()).build(),
127+
)
128+
Result.Success(Unit)
129+
}
130+
}
131+
}
132+
}
133+
134+
override suspend fun suspendChangePin(currentPin: String, newPin: String): Result<Unit, D2Error> {
135+
val credentials = credentialsSecureStore.get()
136+
return when {
137+
credentials == null -> Result.Failure(logInExceptions.noActiveSessionError())
138+
credentials.password != currentPin -> Result.Failure(logInExceptions.incorrectPinError())
139+
else -> suspendSetPin(newPin)
140+
}
141+
}
142+
100143
private fun downloadToken(tokenRequest: TokenRequest): Single<AuthState> {
101144
return Single.create { emitter ->
102145
val authService = AuthorizationService(context)

0 commit comments

Comments
 (0)