Skip to content

Commit b448677

Browse files
antspriggsclaude
andcommitted
Address pre-release security audit findings (all Critical + High)
Fixes all 4 Critical and 5 High findings, plus 5 of 6 Medium and all 3 Low findings from the pre-release security audit. Critical: - [#1] State validation is now unconditional; missing state throws StateMismatch, closing the CSRF / authorization code injection vector - [#2] Public IDmeAuth constructor now requires Context and defaults to EncryptedCredentialStore; CredentialStore demoted to internal - [#3] JWKSClient cache fields are @volatile and all access is serialised through a Mutex, eliminating the race condition - [#4] policies() sends credentials via HTTP Basic Auth header instead of GET query parameter, keeping the client secret out of server logs High: - [#5] Demo network_security_config.xml removes user-cert trust and sets cleartextTrafficPermitted=false - [#6] iss and aud JWT claims are now mandatory; tokens that omit either throw JWTClaimInvalid instead of silently passing - [#7] JWTValidator validates nbf with 30-second clock skew tolerance and applies the same skew window to exp - [#8] IDmeAuthManager replaces the single CompletableDeferred with a ConcurrentHashMap keyed by state; IDmeAuth passes state as sessionId so callbacks cannot be routed to the wrong flow - [#9] extractJSON is now suspend and calls JWTValidator before decoding, ensuring userinfo JWT signatures are verified before claims are exposed Medium: - [#10] Log.isEnabled flag (default false) gates all SDK log output to prevent credential leakage in release builds - [#11] Redirect URI validation rejects http/https/javascript/file/data schemes in IDmeAuth, AuthorizationRequest, and GroupsRequest - [#12] clearSync() cancels the refresh deferred before nulling state, reducing the window for concurrent-write races - [#13] expiresIn is coerced to [0, 86400] seconds before multiplication, preventing integer-overflow-induced negative expiry timestamps - [#14] AuthViewModel extends AndroidViewModel (provides Context to IDmeAuth); clientSecret is only forwarded in OAUTH mode Low: - [#15] secure-pipeline-ast.yml pinned to immutable commit SHA instead of mutable @master ref - [#17] Demo release build enables minification - [#18] Base64URL.decode() throws IDmeAuthError.InvalidJWT on failure instead of returning null; JWTDecoder call sites cleaned up accordingly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e36d75c commit b448677

16 files changed

Lines changed: 135 additions & 79 deletions

File tree

.github/workflows/secure-pipeline-ast.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,5 @@ on:
1414

1515
jobs:
1616
execute:
17-
uses: IDme/workflow-library/.github/workflows/secure-pipeline-ast.yml@master
17+
uses: IDme/workflow-library/.github/workflows/secure-pipeline-ast.yml@7a259bb101fd4f20d7cd0137c1f99e8d60af0859
1818
secrets: inherit

demo/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ configure<BaseAppModuleExtension> {
1919

2020
buildTypes {
2121
release {
22-
isMinifyEnabled = false
22+
isMinifyEnabled = true
2323
proguardFiles(
2424
getDefaultProguardFile("proguard-android-optimize.txt"),
2525
"proguard-rules.pro"

demo/src/main/kotlin/com/idme/auth/demo/AuthViewModel.kt

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
package com.idme.auth.demo
22

33
import android.app.Activity
4+
import android.app.Application
45
import androidx.compose.runtime.getValue
56
import androidx.compose.runtime.mutableStateOf
67
import androidx.compose.runtime.setValue
7-
import androidx.lifecycle.ViewModel
8+
import androidx.lifecycle.AndroidViewModel
89
import androidx.lifecycle.viewModelScope
910
import com.idme.auth.IDmeAuth
1011
import com.idme.auth.configuration.IDmeAuthMode
@@ -17,7 +18,7 @@ import com.idme.auth.models.Credentials
1718
import com.idme.auth.models.Policy
1819
import kotlinx.coroutines.launch
1920

20-
class AuthViewModel : ViewModel() {
21+
class AuthViewModel(application: Application) : AndroidViewModel(application) {
2122

2223
// MARK: - Configuration Inputs
2324

@@ -196,16 +197,20 @@ class AuthViewModel : ViewModel() {
196197
finalScopes.add(0, IDmeScope.OPENID)
197198
}
198199

200+
// Client secret is only applicable for server-side OAuth flows; never embed in PKCE or OIDC.
201+
// TODO: Load credentials from local.properties via BuildConfig rather than hardcoding.
202+
val secret = if (authMode == IDmeAuthMode.OAUTH) clientSecret else null
203+
199204
val config = IDmeConfiguration(
200205
clientId = clientId,
201206
redirectURI = redirectURI,
202207
scopes = finalScopes,
203208
environment = environment,
204209
authMode = authMode,
205210
verificationType = verificationType,
206-
clientSecret = clientSecret
211+
clientSecret = secret
207212
)
208213

209-
return IDmeAuth(config)
214+
return IDmeAuth(config, getApplication())
210215
}
211216
}
Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<network-security-config>
3-
<base-config>
3+
<base-config cleartextTrafficPermitted="false">
44
<trust-anchors>
55
<certificates src="system" />
6-
<certificates src="user" />
76
</trust-anchors>
87
</base-config>
98
</network-security-config>

sdk/src/main/kotlin/com/idme/auth/IDmeAuth.kt

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.idme.auth
22

33
import android.app.Activity
4+
import android.content.Context
45
import android.net.Uri
56
import com.idme.auth.auth.AuthorizationRequest
67
import com.idme.auth.auth.GroupsRequest
@@ -13,6 +14,7 @@ import com.idme.auth.configuration.IDmeVerificationType
1314
import com.idme.auth.errors.IDmeAuthError
1415
import com.idme.auth.jwt.JWKSClient
1516
import com.idme.auth.jwt.JWKSFetching
17+
import com.idme.auth.jwt.JWTDecoder
1618
import com.idme.auth.jwt.JWTValidator
1719
import com.idme.auth.models.AttributeResponse
1820
import com.idme.auth.models.Credentials
@@ -21,7 +23,7 @@ import com.idme.auth.models.UserInfo
2123
import com.idme.auth.networking.APIEndpoint
2224
import com.idme.auth.networking.DefaultHTTPClient
2325
import com.idme.auth.networking.HTTPClient
24-
import com.idme.auth.storage.CredentialStore
26+
import com.idme.auth.storage.EncryptedCredentialStore
2527
import com.idme.auth.token.TokenManager
2628
import com.idme.auth.token.TokenRefresher
2729
import com.idme.auth.utilities.Log
@@ -31,7 +33,6 @@ import kotlinx.serialization.json.JsonPrimitive
3133
import kotlinx.serialization.json.jsonArray
3234
import kotlinx.serialization.json.jsonObject
3335
import kotlinx.serialization.json.jsonPrimitive
34-
import com.idme.auth.jwt.JWTDecoder
3536

3637
/**
3738
* Main entry point for the IDmeAuthSDK.
@@ -45,7 +46,8 @@ import com.idme.auth.jwt.JWTDecoder
4546
* redirectURI = "yourapp://idme/callback",
4647
* scopes = listOf(IDmeScope.MILITARY),
4748
* verificationType = IDmeVerificationType.SINGLE
48-
* )
49+
* ),
50+
* context = applicationContext
4951
* )
5052
*
5153
* val credentials = idme.login(activity)
@@ -59,11 +61,11 @@ class IDmeAuth(
5961
) {
6062
private var lastNonce: String? = null
6163

62-
/** Creates a new IDmeAuth instance with the given configuration. */
63-
constructor(configuration: IDmeConfiguration) : this(
64+
/** Creates a new IDmeAuth instance with the given configuration. Credentials are stored in EncryptedSharedPreferences. */
65+
constructor(configuration: IDmeConfiguration, context: Context) : this(
6466
configuration = configuration,
6567
tokenManager = TokenManager(
66-
credentialStore = CredentialStore(),
68+
credentialStore = EncryptedCredentialStore(context),
6769
refresher = TokenRefresher(configuration, DefaultHTTPClient())
6870
),
6971
httpClient = DefaultHTTPClient(),
@@ -109,7 +111,7 @@ class IDmeAuth(
109111

110112
Log.info("Starting auth session: ${configuration.verificationType.value} mode")
111113

112-
val callbackURL = IDmeAuthManager.launchAuth(activity, authURL)
114+
val callbackURL = IDmeAuthManager.launchAuth(activity, authURL, state)
113115

114116
val code = extractAuthorizationCode(callbackURL, state)
115117

@@ -147,21 +149,20 @@ class IDmeAuth(
147149
/**
148150
* Fetches the available verification policies for the organization.
149151
*
150-
* Uses the client credentials (client_id and client_secret) to authenticate.
152+
* Uses the client credentials (client_id and client_secret) to authenticate via HTTP Basic Auth.
151153
* The policy `handle` can be used as the OAuth `scope` parameter.
152154
*
153155
* @return A list of available policies.
154156
*/
155157
suspend fun policies(): List<Policy> {
156-
val baseUrl = APIEndpoint.policies(configuration.environment)
157-
val url = "$baseUrl?client_id=${
158-
java.net.URLEncoder.encode(configuration.clientId, "UTF-8")
159-
}&client_secret=${
160-
java.net.URLEncoder.encode(configuration.clientSecret ?: "", "UTF-8")
161-
}"
158+
val url = APIEndpoint.policies(configuration.environment)
159+
val credentials = "${configuration.clientId}:${configuration.clientSecret ?: ""}"
160+
val encoded = java.util.Base64.getEncoder()
161+
.encodeToString(credentials.toByteArray(Charsets.UTF_8))
162+
val headers = mapOf("Authorization" to "Basic $encoded")
162163

163164
val response = try {
164-
httpClient.get(url, mapOf())
165+
httpClient.get(url, headers)
165166
} catch (e: IDmeAuthError) {
166167
throw e
167168
} catch (e: Exception) {
@@ -303,10 +304,16 @@ class IDmeAuth(
303304

304305
// MARK: - Private
305306

306-
/** Extracts JSON data from a response that may be plain JSON or a JWT. */
307-
private fun extractJSON(body: String): String {
307+
/**
308+
* Extracts JSON data from a response that may be plain JSON or a JWT.
309+
* If the body is a JWT, the signature is verified before claims are extracted.
310+
*/
311+
private suspend fun extractJSON(body: String): String {
308312
val trimmed = body.trim().trim('"')
309313
if (trimmed.startsWith("eyJ")) {
314+
val issuer = "${configuration.environment.apiBaseURL}oidc"
315+
val validator = JWTValidator(jwksFetcher, issuer, configuration.clientId)
316+
validator.validate(trimmed, null)
310317
val decoded = JWTDecoder.decode(trimmed)
311318
return Json.encodeToString(JsonObject.serializer(), JsonObject(decoded.payload.mapValues { (_, v) ->
312319
JsonPrimitive(v.toString())
@@ -337,7 +344,8 @@ class IDmeAuth(
337344
throw IDmeAuthError.GroupsNotAvailableInSandbox
338345
}
339346

340-
if (Uri.parse(configuration.redirectURI).scheme == null) {
347+
val scheme = Uri.parse(configuration.redirectURI).scheme
348+
if (scheme == null || scheme.lowercase() in DISALLOWED_REDIRECT_SCHEMES) {
341349
throw IDmeAuthError.InvalidRedirectURI
342350
}
343351
}
@@ -354,14 +362,19 @@ class IDmeAuth(
354362
throw IDmeAuthError.TokenExchangeFailed(0, description)
355363
}
356364

357-
// Validate state
365+
// Validate state — mandatory; a missing state is treated as a CSRF/injection attempt
358366
val returnedState = uri.getQueryParameter("state")
359-
if (returnedState != null && returnedState != expectedState) {
367+
?: throw IDmeAuthError.StateMismatch
368+
if (returnedState != expectedState) {
360369
throw IDmeAuthError.StateMismatch
361370
}
362371

363372
// Extract code
364373
return uri.getQueryParameter("code")
365374
?: throw IDmeAuthError.MissingAuthorizationCode
366375
}
376+
377+
companion object {
378+
private val DISALLOWED_REDIRECT_SCHEMES = setOf("http", "https", "javascript", "file", "data")
379+
}
367380
}

sdk/src/main/kotlin/com/idme/auth/auth/AuthorizationRequest.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ class AuthorizationRequest(configuration: IDmeConfiguration) {
1616
val pkce: PKCEGenerator?
1717

1818
init {
19-
if (android.net.Uri.parse(configuration.redirectURI).scheme == null) {
19+
val uriScheme = android.net.Uri.parse(configuration.redirectURI).scheme
20+
if (uriScheme == null || uriScheme.lowercase() in DISALLOWED_SCHEMES) {
2021
throw IDmeAuthError.InvalidRedirectURI
2122
}
2223

@@ -57,4 +58,8 @@ class AuthorizationRequest(configuration: IDmeConfiguration) {
5758
}
5859
url = "$baseUrl?$queryString"
5960
}
61+
62+
companion object {
63+
private val DISALLOWED_SCHEMES = setOf("http", "https", "javascript", "file", "data")
64+
}
6065
}

sdk/src/main/kotlin/com/idme/auth/auth/GroupsRequest.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ class GroupsRequest(configuration: IDmeConfiguration) {
2121
throw IDmeAuthError.GroupsNotAvailableInSandbox
2222
}
2323

24-
if (android.net.Uri.parse(configuration.redirectURI).scheme == null) {
24+
val uriScheme = android.net.Uri.parse(configuration.redirectURI).scheme
25+
if (uriScheme == null || uriScheme.lowercase() in DISALLOWED_SCHEMES) {
2526
throw IDmeAuthError.InvalidRedirectURI
2627
}
2728

@@ -62,4 +63,8 @@ class GroupsRequest(configuration: IDmeConfiguration) {
6263
}
6364
url = "$baseUrl?$queryString"
6465
}
66+
67+
companion object {
68+
private val DISALLOWED_SCHEMES = setOf("http", "https", "javascript", "file", "data")
69+
}
6570
}

sdk/src/main/kotlin/com/idme/auth/auth/IDmeAuthManager.kt

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,35 @@ import android.app.Activity
44
import android.net.Uri
55
import androidx.browser.customtabs.CustomTabsIntent
66
import com.idme.auth.errors.IDmeAuthError
7+
import java.util.concurrent.ConcurrentHashMap
78
import kotlinx.coroutines.CompletableDeferred
89

910
/**
1011
* Singleton that manages the bridge between the Chrome Custom Tab auth flow
1112
* and the coroutine-based SDK API.
1213
*
1314
* Flow:
14-
* 1. [launchAuth] stores a [CompletableDeferred] and opens a Custom Tab.
15+
* 1. [launchAuth] stores a [CompletableDeferred] keyed by the session's `state` value and opens a Custom Tab.
1516
* 2. The browser redirects to the app's scheme, which is caught by [IDmeRedirectActivity].
16-
* 3. [handleRedirect] completes the deferred with the callback URL.
17+
* 3. [handleRedirect] extracts the `state` from the callback URL and routes the result to the
18+
* matching deferred, preventing cross-flow code injection.
1719
* 4. [launchAuth] resumes and returns the callback URL to the caller.
1820
*/
1921
internal object IDmeAuthManager {
20-
private var pendingAuth: CompletableDeferred<String>? = null
22+
private val pending = ConcurrentHashMap<String, CompletableDeferred<String>>()
2123

2224
/**
2325
* Launches the authentication flow in a Chrome Custom Tab and suspends
2426
* until the redirect is received.
2527
*
2628
* @param activity The Activity to launch from.
2729
* @param authUrl The authorization URL to open.
30+
* @param sessionId The `state` value for this flow; used to route the callback to the correct coroutine.
2831
* @return The full callback URL string including query parameters.
2932
*/
30-
suspend fun launchAuth(activity: Activity, authUrl: String): String {
31-
// Cancel any existing pending auth
32-
pendingAuth?.cancel()
33-
33+
suspend fun launchAuth(activity: Activity, authUrl: String, sessionId: String): String {
3434
val deferred = CompletableDeferred<String>()
35-
pendingAuth = deferred
35+
pending[sessionId] = deferred
3636

3737
val customTabsIntent = CustomTabsIntent.Builder()
3838
.setShowTitle(true)
@@ -45,17 +45,28 @@ internal object IDmeAuthManager {
4545
} catch (e: kotlinx.coroutines.CancellationException) {
4646
throw IDmeAuthError.UserCancelled
4747
} finally {
48-
pendingAuth = null
48+
pending.remove(sessionId)
4949
}
5050
}
5151

52-
/** Called by [IDmeRedirectActivity] when the redirect URI is received. */
52+
/** Called by [IDmeRedirectActivity] when the redirect URI is received. Routes to the matching session. */
5353
internal fun handleRedirect(callbackUrl: String) {
54-
pendingAuth?.complete(callbackUrl)
54+
val state = try {
55+
Uri.parse(callbackUrl).getQueryParameter("state")
56+
} catch (_: Exception) {
57+
null
58+
}
59+
if (state != null) {
60+
pending[state]?.complete(callbackUrl)
61+
} else {
62+
// No state in callback (error or non-compliant response) — deliver to any waiting flow
63+
pending.values.firstOrNull()?.complete(callbackUrl)
64+
}
5565
}
5666

57-
/** Called by [IDmeRedirectActivity] when no URI data is present. */
67+
/** Called by [IDmeRedirectActivity] when no URI data is present. Cancels all pending flows. */
5868
internal fun handleCancel() {
59-
pendingAuth?.completeExceptionally(IDmeAuthError.UserCancelled)
69+
pending.values.forEach { it.completeExceptionally(IDmeAuthError.UserCancelled) }
70+
pending.clear()
6071
}
6172
}

sdk/src/main/kotlin/com/idme/auth/jwt/JWKSClient.kt

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import com.idme.auth.models.JWKS
66
import com.idme.auth.networking.APIEndpoint
77
import com.idme.auth.networking.DefaultHTTPClient
88
import com.idme.auth.networking.HTTPClient
9+
import kotlinx.coroutines.sync.Mutex
10+
import kotlinx.coroutines.sync.withLock
911
import kotlinx.serialization.json.Json
1012

1113
/** Interface for fetching JWKS, enabling mock injection. */
@@ -20,16 +22,17 @@ class JWKSClient(
2022
private val cacheTTL: Long = 3600_000L // 1 hour in milliseconds
2123
) : JWKSFetching {
2224

23-
private var cached: JWKS? = null
24-
private var cacheTime: Long = 0
25+
@Volatile private var cached: JWKS? = null
26+
@Volatile private var cacheTime: Long = 0
27+
private val cacheMutex = Mutex()
2528

2629
private val json = Json { ignoreUnknownKeys = true }
2730

28-
override suspend fun fetchJWKS(): JWKS {
31+
override suspend fun fetchJWKS(): JWKS = cacheMutex.withLock {
2932
val now = System.currentTimeMillis()
3033
val cachedValue = cached
3134
if (cachedValue != null && (now - cacheTime) < cacheTTL) {
32-
return cachedValue
35+
return@withLock cachedValue
3336
}
3437

3538
val url = APIEndpoint.jwks(environment)
@@ -55,6 +58,6 @@ class JWKSClient(
5558
cached = jwks
5659
cacheTime = now
5760

58-
return jwks
61+
jwks
5962
}
6063
}

sdk/src/main/kotlin/com/idme/auth/jwt/JWTDecoder.kt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ object JWTDecoder {
5252

5353
// Decode header
5454
val headerData = Base64URL.decode(headerPart)
55-
?: throw IDmeAuthError.InvalidJWT("Failed to decode JWT header")
5655
val headerJSON = try {
5756
Json.parseToJsonElement(String(headerData, Charsets.UTF_8)) as JsonObject
5857
} catch (e: Exception) {
@@ -66,7 +65,6 @@ object JWTDecoder {
6665

6766
// Decode payload
6867
val payloadData = Base64URL.decode(payloadPart)
69-
?: throw IDmeAuthError.InvalidJWT("Failed to decode JWT payload")
7068
val payloadJSON = try {
7169
Json.parseToJsonElement(String(payloadData, Charsets.UTF_8)) as JsonObject
7270
} catch (e: Exception) {
@@ -80,7 +78,6 @@ object JWTDecoder {
8078

8179
// Decode signature
8280
val signatureData = Base64URL.decode(signaturePart)
83-
?: throw IDmeAuthError.InvalidJWT("Failed to decode JWT signature")
8481

8582
val signedPortion = "$headerPart.$payloadPart"
8683

0 commit comments

Comments
 (0)