Skip to content

Commit fc767ce

Browse files
zerox80zerox80
committed
Fix: Variable shadowing in LoginActivity.kt and add unit tests for public PKCE clients
1 parent e8b72ce commit fc767ce

4 files changed

Lines changed: 151 additions & 6 deletions

File tree

‎opencloudApp/src/main/java/eu/opencloud/android/presentation/authentication/LoginActivity.kt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -682,15 +682,15 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
682682
// Use oidc discovery one, or build an oauth endpoint using serverBaseUrl + Setup string.
683683
val tokenEndPoint: String
684684

685-
var clientId: String? = null
686-
var clientSecret: String? = null
685+
var clientIdForRequest: String? = null
686+
var clientSecretForRequest: String? = null
687687

688688
val serverInfo = authenticationViewModel.serverInfo.value?.peekContent()?.getStoredData()
689689
if (serverInfo is ServerInfo.OIDCServer) {
690690
tokenEndPoint = serverInfo.oidcServerConfiguration.tokenEndpoint
691691
if (serverInfo.oidcServerConfiguration.isTokenEndpointAuthMethodSupportedClientSecretPost()) {
692-
clientId = clientRegistrationInfo?.clientId ?: contextProvider.getString(R.string.oauth2_client_id)
693-
clientSecret = clientRegistrationInfo?.clientSecret ?: contextProvider.getString(R.string.oauth2_client_secret)
692+
clientIdForRequest = clientRegistrationInfo?.clientId ?: contextProvider.getString(R.string.oauth2_client_id)
693+
clientSecretForRequest = clientRegistrationInfo?.clientSecret ?: contextProvider.getString(R.string.oauth2_client_secret)
694694
}
695695
} else {
696696
tokenEndPoint = "$serverBaseUrl${File.separator}${contextProvider.getString(R.string.oauth2_url_endpoint_access)}"
@@ -703,8 +703,8 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
703703
tokenEndpoint = tokenEndPoint,
704704
clientAuth = clientAuth,
705705
scope = scope,
706-
clientId = clientId,
707-
clientSecret = clientSecret,
706+
clientId = clientIdForRequest,
707+
clientSecret = clientSecretForRequest,
708708
authorizationCode = authorizationCode,
709709
redirectUri = OAuthUtils.buildRedirectUri(applicationContext).toString(),
710710
codeVerifier = authenticationViewModel.codeVerifier

‎opencloudData/src/test/java/eu/opencloud/android/data/oauth/RemoteOAuthUtils.kt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ import eu.opencloud.android.testutil.oauth.OC_CLIENT_REGISTRATION
2727
import eu.opencloud.android.testutil.oauth.OC_CLIENT_REGISTRATION_REQUEST
2828
import eu.opencloud.android.testutil.oauth.OC_OIDC_SERVER_CONFIGURATION
2929
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_ACCESS
30+
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT
3031
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_REFRESH
32+
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT
3133
import eu.opencloud.android.testutil.oauth.OC_TOKEN_RESPONSE
3234

3335
val OC_REMOTE_OIDC_DISCOVERY_RESPONSE = OIDCDiscoveryResponse(
@@ -65,6 +67,32 @@ val OC_REMOTE_TOKEN_REQUEST_PARAMS_REFRESH = TokenRequestParams.RefreshToken(
6567
refreshToken = OC_TOKEN_REQUEST_REFRESH.refreshToken
6668
)
6769

70+
/**
71+
* Test fixtures for public PKCE clients (RFC 7636).
72+
* Public clients MUST NOT send Authorization header during token exchange.
73+
*/
74+
val OC_REMOTE_TOKEN_REQUEST_PARAMS_ACCESS_PUBLIC_CLIENT = TokenRequestParams.Authorization(
75+
tokenEndpoint = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.tokenEndpoint,
76+
clientAuth = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.clientAuth, // Empty string
77+
grantType = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.grantType,
78+
scope = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.scope,
79+
clientId = null,
80+
clientSecret = null,
81+
authorizationCode = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.authorizationCode,
82+
redirectUri = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.redirectUri,
83+
codeVerifier = OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.codeVerifier
84+
)
85+
86+
val OC_REMOTE_TOKEN_REQUEST_PARAMS_REFRESH_PUBLIC_CLIENT = TokenRequestParams.RefreshToken(
87+
tokenEndpoint = OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.tokenEndpoint,
88+
clientAuth = OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.clientAuth, // Empty string
89+
grantType = OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.grantType,
90+
scope = OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.scope,
91+
clientId = null,
92+
clientSecret = null,
93+
refreshToken = OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.refreshToken
94+
)
95+
6896
val OC_REMOTE_TOKEN_RESPONSE = TokenResponse(
6997
accessToken = OC_TOKEN_RESPONSE.accessToken,
7098
expiresIn = OC_TOKEN_RESPONSE.expiresIn,

‎opencloudData/src/test/java/eu/opencloud/android/data/oauth/datasources/implementation/OCRemoteOAuthDataSourceTest.kt‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,16 @@ import eu.opencloud.android.testutil.oauth.OC_CLIENT_REGISTRATION
3434
import eu.opencloud.android.testutil.oauth.OC_CLIENT_REGISTRATION_REQUEST
3535
import eu.opencloud.android.testutil.oauth.OC_OIDC_SERVER_CONFIGURATION
3636
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_ACCESS
37+
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT
38+
import eu.opencloud.android.testutil.oauth.OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT
3739
import eu.opencloud.android.testutil.oauth.OC_TOKEN_RESPONSE
3840
import eu.opencloud.android.utils.createRemoteOperationResultMock
3941
import io.mockk.every
4042
import io.mockk.mockk
4143
import io.mockk.verify
4244
import org.junit.Assert.assertEquals
4345
import org.junit.Assert.assertNotNull
46+
import org.junit.Assert.assertTrue
4447
import org.junit.Before
4548
import org.junit.Test
4649

@@ -102,6 +105,98 @@ class OCRemoteOAuthDataSourceTest {
102105
}
103106
}
104107

108+
/**
109+
* Test for public PKCE clients (RFC 7636).
110+
* Public clients MUST NOT send Authorization header during token exchange.
111+
* This test verifies that token requests work correctly with empty clientAuth.
112+
*/
113+
@Test
114+
fun `performTokenRequest with public PKCE client returns a TokenResponse`() {
115+
val tokenResponseResult: RemoteOperationResult<TokenResponse> =
116+
createRemoteOperationResultMock(data = OC_REMOTE_TOKEN_RESPONSE, isSuccess = true)
117+
118+
every {
119+
oidcService.performTokenRequest(ocClientMocked, any())
120+
} returns tokenResponseResult
121+
122+
// Use the public client fixture which has empty clientAuth
123+
assertTrue(
124+
"clientAuth should be empty for public clients",
125+
OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.clientAuth.isEmpty()
126+
)
127+
128+
val tokenResponse = remoteOAuthDataSource.performTokenRequest(OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT)
129+
130+
assertNotNull(tokenResponse)
131+
assertEquals(OC_TOKEN_RESPONSE, tokenResponse)
132+
133+
verify(exactly = 1) {
134+
clientManager.getClientForAnonymousCredentials(OC_SECURE_BASE_URL, any())
135+
oidcService.performTokenRequest(ocClientMocked, any())
136+
}
137+
}
138+
139+
/**
140+
* Test for public PKCE clients (RFC 7636) using refresh token.
141+
* Public clients MUST NOT send Authorization header during token refresh.
142+
* This test verifies that refresh token requests work correctly with empty clientAuth.
143+
*/
144+
@Test
145+
fun `performTokenRequest with public PKCE client refresh token returns a TokenResponse`() {
146+
val tokenResponseResult: RemoteOperationResult<TokenResponse> =
147+
createRemoteOperationResultMock(data = OC_REMOTE_TOKEN_RESPONSE, isSuccess = true)
148+
149+
every {
150+
oidcService.performTokenRequest(ocClientMocked, any())
151+
} returns tokenResponseResult
152+
153+
// Verify the refresh token fixture has empty clientAuth
154+
assertTrue(
155+
"clientAuth should be empty for public clients",
156+
OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.clientAuth.isEmpty()
157+
)
158+
159+
val tokenResponse = remoteOAuthDataSource.performTokenRequest(OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT)
160+
161+
assertNotNull(tokenResponse)
162+
assertEquals(OC_TOKEN_RESPONSE, tokenResponse)
163+
164+
verify(exactly = 1) {
165+
clientManager.getClientForAnonymousCredentials(OC_SECURE_BASE_URL, any())
166+
oidcService.performTokenRequest(ocClientMocked, any())
167+
}
168+
}
169+
170+
/**
171+
* RFC 7636 compliance verification:
172+
* This test ensures that our public client test fixtures correctly have empty clientAuth,
173+
* which means TokenRequestRemoteOperation will NOT add an Authorization header.
174+
* The actual header logic is in TokenRequestRemoteOperation:
175+
* if (tokenRequestParams.clientAuth.isNotEmpty()) {
176+
* postMethod.addRequestHeader(AUTHORIZATION_HEADER, tokenRequestParams.clientAuth)
177+
* }
178+
*/
179+
@Test
180+
fun `public PKCE client fixtures have empty clientAuth preventing Authorization header`() {
181+
// Verify access token fixture
182+
assertTrue(
183+
"Access token public client fixture should have empty clientAuth",
184+
OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT.clientAuth.isEmpty()
185+
)
186+
187+
// Verify refresh token fixture
188+
assertTrue(
189+
"Refresh token public client fixture should have empty clientAuth",
190+
OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT.clientAuth.isEmpty()
191+
)
192+
193+
// Verify confidential client fixtures have non-empty clientAuth (for comparison)
194+
assertTrue(
195+
"Confidential client fixture should have non-empty clientAuth",
196+
OC_TOKEN_REQUEST_ACCESS.clientAuth.isNotEmpty()
197+
)
198+
}
199+
105200
@Test
106201
fun `registerClient returns a ClientRegistrationInfo`() {
107202
val clientRegistrationResponse: RemoteOperationResult<ClientRegistrationResponse> =

‎opencloudTestUtil/src/main/java/eu/opencloud/android/testutil/oauth/TokenRequest.kt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,25 @@ val OC_TOKEN_REQUEST_ACCESS = TokenRequest.AccessToken(
4343
redirectUri = OC_REDIRECT_URI,
4444
codeVerifier = "A high-entropy cryptographic random STRING using the unreserved characters"
4545
)
46+
47+
/**
48+
* Test fixtures for public PKCE clients (RFC 7636).
49+
* Public clients MUST NOT send Authorization header during token exchange.
50+
*/
51+
val OC_TOKEN_REQUEST_REFRESH_PUBLIC_CLIENT = TokenRequest.RefreshToken(
52+
baseUrl = OC_SECURE_BASE_URL,
53+
tokenEndpoint = OC_TOKEN_ENDPOINT,
54+
clientAuth = "", // Empty for public clients per RFC 7636
55+
scope = OC_SCOPE,
56+
refreshToken = OC_REFRESH_TOKEN
57+
)
58+
59+
val OC_TOKEN_REQUEST_ACCESS_PUBLIC_CLIENT = TokenRequest.AccessToken(
60+
baseUrl = OC_SECURE_BASE_URL,
61+
tokenEndpoint = OC_TOKEN_ENDPOINT,
62+
clientAuth = "", // Empty for public clients per RFC 7636
63+
scope = OC_SCOPE,
64+
authorizationCode = "4uth0r1z4t10nC0d3",
65+
redirectUri = OC_REDIRECT_URI,
66+
codeVerifier = "A high-entropy cryptographic random STRING using the unreserved characters"
67+
)

0 commit comments

Comments
 (0)