Skip to content

Commit 8f3d7aa

Browse files
committed
Redact OAuth secrets from logs
1 parent d28208a commit 8f3d7aa

6 files changed

Lines changed: 114 additions & 21 deletions

File tree

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,7 @@ private String refreshToken(
329329
return null;
330330
}
331331

332-
Timber.d("Ready to exchange for new tokens. Account: [ %s ], Refresh token: [ %s ]", account.name,
333-
refreshToken);
332+
Timber.d("Ready to exchange refresh token for new tokens. Account: [ %s ]", account.name);
334333

335334
String baseUrl = accountManager.getUserData(account, AccountUtils.Constants.KEY_OC_BASE_URL);
336335

@@ -440,8 +439,7 @@ private String handleSuccessfulRefreshToken(
440439
}
441440
accountManager.setUserData(account, KEY_OAUTH2_REFRESH_TOKEN, refreshTokenToUseFromNowOn);
442441

443-
Timber.d("Token refreshed successfully. New access token: [ %s ]. New refresh token: [ %s ]",
444-
newAccessToken, refreshTokenToUseFromNowOn);
442+
Timber.d("Token refreshed successfully for account [ %s ]", account.name);
445443

446444
return newAccessToken;
447445
}

‎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
@@ -133,7 +133,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
133133
override fun onCreate(savedInstanceState: Bundle?) {
134134
super.onCreate(savedInstanceState)
135135

136-
Timber.d("onCreate called with intent data: ${intent.data}, isTaskRoot: $isTaskRoot")
136+
Timber.d("onCreate called with intent data present: ${intent.data != null}, isTaskRoot: $isTaskRoot")
137137

138138
if (handleOAuthRedirectOnCreate()) return
139139

@@ -747,7 +747,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
747747
override fun onNewIntent(intent: Intent?) {
748748
super.onNewIntent(intent)
749749
intent?.let {
750-
Timber.d("onNewIntent received with data: ${it.data}")
750+
Timber.d("onNewIntent received with data present: ${it.data != null}")
751751
if (!::binding.isInitialized) {
752752
Timber.w("onNewIntent received before binding initialized, ignoring OAuth response")
753753
return
@@ -762,12 +762,12 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
762762
val state = intent.data?.getQueryParameter("state")
763763

764764
if (state != authenticationViewModel.oidcState) {
765-
Timber.e("OAuth: state mismatch (expected=${authenticationViewModel.oidcState}, got=$state). Finishing.")
765+
Timber.e("OAuth state mismatch. Finishing.")
766766
showMessageInSnackbar(message = getString(R.string.auth_oauth_error))
767767
finish()
768768
} else {
769769
if (authorizationCode != null) {
770-
Timber.d("Authorization code received [$authorizationCode]. Let's exchange it for access token")
770+
Timber.d("Authorization code received. Exchanging it for access token")
771771
exchangeAuthorizationCodeForTokens(authorizationCode)
772772
} else {
773773
val authorizationError = intent.data?.getQueryParameter("error")
@@ -851,12 +851,12 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
851851
when (val uiResult = it.peekContent()) {
852852
is UIResult.Loading -> {}
853853
is UIResult.Success -> {
854-
Timber.d("Tokens received ${uiResult.data}, trying to login, creating account and adding it to account manager")
854+
Timber.d("Tokens received, trying to login, creating account and adding it to account manager")
855855
val tokenResponse = uiResult.data ?: return@observe
856856

857857
// Extract preferred_username from id_token for login_hint on re-login
858858
preferredUsername = extractPreferredUsernameFromIdToken(tokenResponse.idToken)
859-
Timber.d("Preferred username from id_token: $preferredUsername")
859+
Timber.d("Preferred username extracted from id_token: ${preferredUsername != null}")
860860

861861
// When webfinger provides a client_id without dynamic registration,
862862
// store it so AccountAuthenticator can use it for token refresh

‎opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/logging/LogInterceptor.kt‎

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import eu.opencloud.android.lib.common.http.HttpConstants.OC_X_REQUEST_ID
2828
import com.squareup.moshi.Moshi
2929
import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
3030
import okhttp3.Headers
31+
import okhttp3.HttpUrl
3132
import okhttp3.Interceptor
3233
import okhttp3.MediaType
3334
import okhttp3.Request
@@ -74,7 +75,7 @@ class LogInterceptor : Interceptor {
7475
info = RequestInfo(
7576
id = requestId,
7677
method = request.method,
77-
url = request.url.toString(),
78+
url = redactUrl(request.url),
7879
)
7980
)
8081
)
@@ -88,10 +89,16 @@ class LogInterceptor : Interceptor {
8889
if (auxHeaders.contains(AUTHORIZATION_HEADER)) {
8990
val authHeaderList = auxHeaders[AUTHORIZATION_HEADER]!!.split(" ")
9091
val authType = authHeaderList[0]
91-
val authInfo = if (redactAuthHeader) "[redacted]" else authHeaderList[1]
92+
val authInfo = if (redactAuthHeader) REDACTED else authHeaderList.getOrNull(1).orEmpty()
9293
auxHeaders[AUTHORIZATION_HEADER] = "$authType $authInfo"
9394
}
94-
return auxHeaders
95+
return auxHeaders.mapValues { (header, value) ->
96+
if (SENSITIVE_HEADER_NAMES.any { it.equals(header, ignoreCase = true) }) {
97+
REDACTED
98+
} else {
99+
redactSensitiveData(value)
100+
}
101+
}
95102
}
96103

97104
private fun getRequestBodyString(requestBodyParam: RequestBody?): String? {
@@ -110,7 +117,7 @@ class LogInterceptor : Interceptor {
110117
val contentType = requestBody.contentType()
111118
val charset: Charset = contentType?.charset(StandardCharsets.UTF_8) ?: StandardCharsets.UTF_8
112119
if (contentType.isLoggable()) {
113-
return buffer.readString(charset)
120+
return redactSensitiveData(buffer.readString(charset))
114121
} else if (requestBody.contentLength() > 0) {
115122
return "$BINARY_OMITTED ${requestBody.contentLength()} $BYTES"
116123
}
@@ -147,7 +154,7 @@ class LogInterceptor : Interceptor {
147154
status = response.code,
148155
version = response.protocol.toString(),
149156
),
150-
url = request.url.toString(),
157+
url = redactUrl(request.url),
151158
)
152159
)
153160
)
@@ -158,7 +165,7 @@ class LogInterceptor : Interceptor {
158165

159166
private fun getResponseBodyString(contentType: MediaType?, contentLength: Int, responseBody: String): String? =
160167
if (contentType?.isLoggable() == true) {
161-
responseBody
168+
redactSensitiveData(responseBody)
162169
} else if (contentLength > 0) {
163170
"$BINARY_OMITTED $contentLength $BYTES"
164171
} else {
@@ -176,11 +183,63 @@ class LogInterceptor : Interceptor {
176183
return String.format(DURATION_FORMAT, hours, minutes, seconds, auxMillis)
177184
}
178185

186+
private fun redactUrl(url: HttpUrl): String {
187+
var redactedUrl = url.newBuilder()
188+
url.queryParameterNames
189+
.filter(::isSensitiveField)
190+
.forEach { redactedUrl = redactedUrl.setQueryParameter(it, REDACTED) }
191+
return redactedUrl.build().toString()
192+
}
193+
194+
private fun redactSensitiveData(value: String): String =
195+
FORM_FIELD_REGEX.replace(
196+
JSON_FIELD_REGEX.replace(
197+
TO_STRING_FIELD_REGEX.replace(value) {
198+
"${it.groupValues[1]}$REDACTED"
199+
}
200+
) {
201+
"${it.groupValues[1]}$REDACTED${it.groupValues[4]}"
202+
}
203+
) {
204+
"${it.groupValues[1]}${it.groupValues[2]}=$REDACTED"
205+
}
206+
207+
private fun isSensitiveField(field: String): Boolean =
208+
SENSITIVE_FIELD_NAMES.any { it.equals(field, ignoreCase = true) }
209+
179210
companion object {
180211
var httpLogsEnabled: Boolean = false
181212
var redactAuthHeader: Boolean = true
213+
private const val REDACTED = "[redacted]"
182214
private const val LIMIT_BODY_LOG: Long = 1000000
183215
private const val BINARY_OMITTED = "<-- Body end for response -- Binary -- Omitted:"
184216
private const val BYTES = "bytes -->"
217+
private val SENSITIVE_HEADER_NAMES = setOf(
218+
"Cookie",
219+
"Set-Cookie",
220+
"X-Auth-Token",
221+
)
222+
private val SENSITIVE_FIELD_NAMES = setOf(
223+
"access_token",
224+
"accessToken",
225+
"authorization_code",
226+
"authorizationCode",
227+
"client_secret",
228+
"clientSecret",
229+
"code",
230+
"code_verifier",
231+
"codeVerifier",
232+
"id_token",
233+
"idToken",
234+
"password",
235+
"passcode",
236+
"pattern",
237+
"refresh_token",
238+
"refreshToken",
239+
)
240+
private val SENSITIVE_FIELD_PATTERN = SENSITIVE_FIELD_NAMES.joinToString("|") { Regex.escape(it) }
241+
private val JSON_FIELD_REGEX = Regex("""(?i)("($SENSITIVE_FIELD_PATTERN)"\s*:\s*")([^"]*)(")""")
242+
private val FORM_FIELD_REGEX = Regex("""(?i)(^|[?&])($SENSITIVE_FIELD_PATTERN)=([^&#]*)""")
243+
private val TO_STRING_FIELD_REGEX = Regex("""(?i)(\b($SENSITIVE_FIELD_PATTERN)\s*=\s*)([^,)\s&]+)""")
185244
}
186245
}

‎opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/oauth/TokenRequestRemoteOperation.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,20 @@ class TokenRequestRemoteOperation(
6363
val responseBody = postMethod.getResponseBodyAsString()
6464

6565
return if (status == HTTP_OK && responseBody != null) {
66-
Timber.d("Successful response $responseBody")
66+
Timber.d("Successful token response received")
6767

6868
// Parse the response
6969
val moshi: Moshi = Moshi.Builder().build()
7070
val jsonAdapter: JsonAdapter<TokenResponse> = moshi.adapter(TokenResponse::class.java)
7171
val tokenResponse: TokenResponse? = jsonAdapter.fromJson(responseBody)
72-
Timber.d("Get tokens completed and parsed to $tokenResponse")
72+
Timber.d("Get tokens completed and parsed")
7373

7474
RemoteOperationResult<TokenResponse>(RemoteOperationResult.ResultCode.OK).apply {
7575
data = tokenResponse
7676
}
7777

7878
} else {
79-
Timber.e("Failed response while getting tokens from the server status code: $status; response message: $responseBody")
79+
Timber.e("Failed response while getting tokens from the server status code: $status")
8080
RemoteOperationResult<TokenResponse>(postMethod)
8181
}
8282

‎opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/oauth/responses/TokenResponse.kt‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,22 @@ data class TokenResponse(
4444
val idToken: String? = null,
4545
@Json(name = "additional_parameters")
4646
val additionalParameters: Map<String, String>?
47-
)
47+
) {
48+
override fun toString(): String =
49+
"TokenResponse(" +
50+
"accessToken=$REDACTED, " +
51+
"expiresIn=$expiresIn, " +
52+
"refreshToken=${refreshToken.redactedOrNull()}, " +
53+
"tokenType=$tokenType, " +
54+
"userId=$userId, " +
55+
"scope=$scope, " +
56+
"idToken=${idToken.redactedOrNull()}, " +
57+
"additionalParameters=$additionalParameters" +
58+
")"
59+
60+
private fun String?.redactedOrNull(): String = if (this == null) "null" else REDACTED
61+
62+
private companion object {
63+
const val REDACTED = "[redacted]"
64+
}
65+
}

‎opencloudDomain/src/main/java/eu/opencloud/android/domain/authentication/oauth/model/TokenResponse.kt‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,22 @@ data class TokenResponse(
2828
val scope: String?,
2929
val idToken: String? = null,
3030
val additionalParameters: Map<String, String>?
31-
)
31+
) {
32+
override fun toString(): String =
33+
"TokenResponse(" +
34+
"accessToken=$REDACTED, " +
35+
"expiresIn=$expiresIn, " +
36+
"refreshToken=${refreshToken.redactedOrNull()}, " +
37+
"tokenType=$tokenType, " +
38+
"userId=$userId, " +
39+
"scope=$scope, " +
40+
"idToken=${idToken.redactedOrNull()}, " +
41+
"additionalParameters=$additionalParameters" +
42+
")"
43+
44+
private fun String?.redactedOrNull(): String = if (this == null) "null" else REDACTED
45+
46+
private companion object {
47+
const val REDACTED = "[redacted]"
48+
}
49+
}

0 commit comments

Comments
 (0)