Skip to content

Commit 953efc0

Browse files
Yuki-YuXinYuki-YuXin
authored
Clean up native auth msal logging (#2071)
### Goal: - Review our log statements and reduce to only those that are relevant and useful. Especially the logMethodCall should be much reduced. Not every method call is relevant to know about when debugging. - Double check that the existing logs use string interpolation correctly to avoid leaking sensitive information in the logs. ### Changes summary: - Apply the rule "only logger unknown error" to NativeAuthPublicClientApplication and states. Exception: Sign in has password CodeRequired since it's unusual for the case. - Add arguments under method LogSession.logMethodCall tp distinguish callback and coroutine. ### Company PR: AzureAD/microsoft-authentication-library-common-for-android#2396
1 parent 19be782 commit 953efc0

6 files changed

Lines changed: 42 additions & 24 deletions

File tree

‎msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class NativeAuthPublicClientApplication(
120120
LogSession.logMethodCall(
121121
tag = TAG,
122122
correlationId = null,
123-
methodName = "${TAG}.getCurrentAccountInternal"
123+
methodName = "${TAG}.getCurrentAccountInternal(config: NativeAuthPublicClientApplicationConfiguration)"
124124
)
125125

126126
val params = CommandParametersAdapter.createCommandParameters(
@@ -164,7 +164,7 @@ class NativeAuthPublicClientApplication(
164164
LogSession.logMethodCall(
165165
tag = TAG,
166166
correlationId = null,
167-
methodName = "${TAG}.getAccountFromICacheRecordsList"
167+
methodName = "${TAG}.getAccountFromICacheRecordsList(cacheRecords: List<ICacheRecord?>?)"
168168
)
169169
if (cacheRecords.isNullOrEmpty()) {
170170
return null
@@ -230,6 +230,11 @@ class NativeAuthPublicClientApplication(
230230
* @return [com.microsoft.identity.nativeauth.statemachine.states.AccountState] if there is a signed in account, null otherwise.
231231
*/
232232
override fun getCurrentAccount(callback: GetCurrentAccountCallback) {
233+
LogSession.logMethodCall(
234+
tag = TAG,
235+
correlationId = null,
236+
methodName = "${TAG}.getCurrentAccount(callback: GetCurrentAccountCallback)"
237+
)
233238
pcaScope.launch {
234239
try {
235240
val result = getCurrentAccount()
@@ -246,6 +251,11 @@ class NativeAuthPublicClientApplication(
246251
* @return [com.microsoft.identity.nativeauth.statemachine.states.AccountState] if there is a signed in account, null otherwise.
247252
*/
248253
override suspend fun getCurrentAccount(): GetAccountResult {
254+
LogSession.logMethodCall(
255+
tag = TAG,
256+
correlationId = null,
257+
methodName = "${TAG}.getCurrentAccount"
258+
)
249259
return withContext(Dispatchers.IO) {
250260
try {
251261
val account = getCurrentAccountInternal(nativeAuthConfig)
@@ -291,7 +301,7 @@ class NativeAuthPublicClientApplication(
291301
LogSession.logMethodCall(
292302
tag = TAG,
293303
correlationId = null,
294-
methodName = "${TAG}.signIn"
304+
methodName = "${TAG}.signIn(username: String, password: CharArray?, scopes: List<String>?, callback: SignInCallback)"
295305
)
296306
pcaScope.launch {
297307
try {
@@ -321,7 +331,7 @@ class NativeAuthPublicClientApplication(
321331
LogSession.logMethodCall(
322332
tag = TAG,
323333
correlationId = null,
324-
methodName = "${TAG}.signIn"
334+
methodName = "${TAG}.signIn(username: String, password: CharArray?, scopes: List<String>?)"
325335
)
326336
return withContext(Dispatchers.IO) {
327337
try {
@@ -373,6 +383,7 @@ class NativeAuthPublicClientApplication(
373383
} else {
374384
Logger.warnWithObject(
375385
TAG,
386+
result.correlationId,
376387
"Sign in received unexpected result: ",
377388
result
378389
)
@@ -525,7 +536,7 @@ class NativeAuthPublicClientApplication(
525536
LogSession.logMethodCall(
526537
tag = TAG,
527538
correlationId = null,
528-
methodName = "${TAG}.signUp"
539+
methodName = "${TAG}.signUp(username: String, password: CharArray?, attributes: UserAttributes?, callback: SignUpCallback)"
529540
)
530541
pcaScope.launch {
531542
try {
@@ -554,7 +565,7 @@ class NativeAuthPublicClientApplication(
554565
LogSession.logMethodCall(
555566
tag = TAG,
556567
correlationId = null,
557-
methodName = "${TAG}.signUp"
568+
methodName = "${TAG}.signUp(username: String, password: CharArray?, attributes: UserAttributes?)"
558569
)
559570
var hasPassword = password?.isNotEmpty() == true
560571

@@ -637,6 +648,7 @@ class NativeAuthPublicClientApplication(
637648
if (hasPassword) {
638649
Logger.warnWithObject(
639650
TAG,
651+
result.correlationId,
640652
"Sign up using password received unexpected result: ",
641653
result
642654
)
@@ -677,6 +689,7 @@ class NativeAuthPublicClientApplication(
677689
} else {
678690
Logger.warnWithObject(
679691
TAG,
692+
result.correlationId,
680693
"Sign up received unexpected result: ",
681694
result
682695
)
@@ -759,7 +772,7 @@ class NativeAuthPublicClientApplication(
759772
LogSession.logMethodCall(
760773
tag = TAG,
761774
correlationId = null,
762-
methodName = "${TAG}.resetPassword"
775+
methodName = "${TAG}.resetPassword(username: String, callback: ResetPasswordCallback)"
763776
)
764777
pcaScope.launch {
765778
try {

‎msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationConfiguration.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,8 @@ import com.microsoft.identity.client.PublicClientApplicationConfiguration
2828
import com.microsoft.identity.client.configuration.AccountMode
2929
import com.microsoft.identity.client.exception.MsalClientException
3030
import com.microsoft.identity.common.java.authorities.CIAMAuthority
31-
import com.microsoft.identity.common.java.nativeauth.authorities.NativeAuthCIAMAuthority
32-
import com.microsoft.identity.common.java.logging.LogSession
3331
import com.microsoft.identity.common.java.logging.Logger
32+
import com.microsoft.identity.common.java.nativeauth.authorities.NativeAuthCIAMAuthority
3433
import com.microsoft.identity.common.java.nativeauth.providers.NativeAuthConstants
3534
import lombok.Getter
3635
import lombok.experimental.Accessors

‎msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class AccountState private constructor(
9090
LogSession.logMethodCall(
9191
tag = TAG,
9292
correlationId = null,
93-
methodName = "$TAG.signOut"
93+
methodName = "${TAG}.signOut(callback: SignOutCallback)"
9494
)
9595
NativeAuthPublicClientApplication.pcaScope.launch {
9696
try {
@@ -112,7 +112,7 @@ class AccountState private constructor(
112112
LogSession.logMethodCall(
113113
tag = TAG,
114114
correlationId = null,
115-
methodName = "$TAG.signOut.withContext"
115+
methodName = "${TAG}.signOut()"
116116
)
117117

118118
val account: IAccount =
@@ -220,7 +220,7 @@ class AccountState private constructor(
220220
LogSession.logMethodCall(
221221
tag = TAG,
222222
correlationId = null,
223-
methodName = "$TAG.getAccessToken"
223+
methodName = "${TAG}.getAccessToken(forceRefresh: Boolean = ${forceRefresh}, callback: GetAccessTokenCallback)"
224224
)
225225
NativeAuthPublicClientApplication.pcaScope.launch {
226226
try {
@@ -278,7 +278,7 @@ class AccountState private constructor(
278278
LogSession.logMethodCall(
279279
tag = TAG,
280280
correlationId = null,
281-
methodName = "$TAG.getAccessToken"
281+
methodName = "${TAG}.getAccessToken(forceRefresh: Boolean = ${forceRefresh}, scopes: List<String>, callback: GetAccessTokenCallback)"
282282
)
283283
NativeAuthPublicClientApplication.pcaScope.launch {
284284
try {
@@ -295,7 +295,7 @@ class AccountState private constructor(
295295
LogSession.logMethodCall(
296296
tag = TAG,
297297
correlationId = null,
298-
methodName = "$TAG.getAccessToken(forceRefresh: Boolean)"
298+
methodName = "${TAG}.getAccessTokenInternal(forceRefresh: Boolean = ${forceRefresh}, scopes: List<String>)"
299299
)
300300

301301
return withContext(Dispatchers.IO) {

‎msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/ResetPasswordStates.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class ResetPasswordCodeRequiredState internal constructor(
9595
LogSession.logMethodCall(
9696
tag = TAG,
9797
correlationId = correlationId,
98-
methodName = "${TAG}.submitCode"
98+
methodName = "${TAG}.submitCode(code: String, callback: SubmitCodeCallback)"
9999
)
100100
NativeAuthPublicClientApplication.pcaScope.launch {
101101
try {
@@ -357,7 +357,7 @@ class ResetPasswordPasswordRequiredState internal constructor(
357357
LogSession.logMethodCall(
358358
tag = TAG,
359359
correlationId = correlationId,
360-
methodName = "${TAG}.submitPassword"
360+
methodName = "${TAG}.submitPassword(password: CharArray, callback: SubmitPasswordCallback)"
361361
)
362362
NativeAuthPublicClientApplication.pcaScope.launch {
363363
try {

‎msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/SignInStates.kt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class SignInCodeRequiredState internal constructor(
102102
LogSession.logMethodCall(
103103
tag = TAG,
104104
correlationId = correlationId,
105-
methodName = "${TAG}.submitCode"
105+
methodName = "${TAG}.submitCode(code: String, callback: SubmitCodeCallback)"
106106
)
107107
NativeAuthPublicClientApplication.pcaScope.launch {
108108
try {
@@ -223,7 +223,7 @@ class SignInCodeRequiredState internal constructor(
223223
LogSession.logMethodCall(
224224
tag = TAG,
225225
correlationId = correlationId,
226-
methodName = "${TAG}.resendCode"
226+
methodName = "${TAG}.resendCode(callback: ResendCodeCallback)"
227227
)
228228
NativeAuthPublicClientApplication.pcaScope.launch {
229229
try {
@@ -369,7 +369,7 @@ class SignInPasswordRequiredState(
369369
LogSession.logMethodCall(
370370
tag = TAG,
371371
correlationId = correlationId,
372-
methodName = "${TAG}.submitPassword"
372+
methodName = "${TAG}.submitPassword(password: CharArray, callback: SubmitPasswordCallback)"
373373
)
374374
NativeAuthPublicClientApplication.pcaScope.launch {
375375
try {
@@ -531,7 +531,7 @@ class SignInContinuationState(
531531
LogSession.logMethodCall(
532532
tag = TAG,
533533
correlationId = correlationId,
534-
methodName = "${TAG}.signIn"
534+
methodName = "${TAG}.signIn(scopes: List<String>, callback: SignInContinuationCallback)"
535535
)
536536

537537
NativeAuthPublicClientApplication.pcaScope.launch {
@@ -552,6 +552,12 @@ class SignInContinuationState(
552552
* @return The results of the sign-in-after-sign-up action.
553553
*/
554554
suspend fun signIn(scopes: List<String>? = null): SignInResult {
555+
LogSession.logMethodCall(
556+
tag = TAG,
557+
correlationId = correlationId,
558+
methodName = "${TAG}.signIn(scopes: List<String>)"
559+
)
560+
555561
return withContext(Dispatchers.IO) {
556562
try {
557563
LogSession.logMethodCall(

‎msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/SignUpStates.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class SignUpCodeRequiredState internal constructor(
101101
LogSession.logMethodCall(
102102
tag = TAG,
103103
correlationId = correlationId,
104-
methodName = "${TAG}.submitCode"
104+
methodName = "${TAG}.submitCode(code: String, callback: SubmitCodeCallback)"
105105
)
106106

107107
NativeAuthPublicClientApplication.pcaScope.launch {
@@ -258,7 +258,7 @@ class SignUpCodeRequiredState internal constructor(
258258
LogSession.logMethodCall(
259259
tag = TAG,
260260
correlationId = correlationId,
261-
methodName = "${TAG}.resendCode"
261+
methodName = "${TAG}.resendCode(callback: SignUpWithResendCodeCallback)"
262262
)
263263
NativeAuthPublicClientApplication.pcaScope.launch {
264264
try {
@@ -403,7 +403,7 @@ class SignUpPasswordRequiredState internal constructor(
403403
LogSession.logMethodCall(
404404
tag = TAG,
405405
correlationId = correlationId,
406-
methodName = "${TAG}.submitPassword"
406+
methodName = "${TAG}.submitPassword(password: CharArray, callback: SignUpSubmitPasswordCallback)"
407407
)
408408
NativeAuthPublicClientApplication.pcaScope.launch {
409409
try {
@@ -613,7 +613,7 @@ class SignUpAttributesRequiredState internal constructor(
613613
LogSession.logMethodCall(
614614
tag = TAG,
615615
correlationId = correlationId,
616-
methodName = "${TAG}.submitAttributes"
616+
methodName = "${TAG}.submitAttributes(attributes: UserAttributes, callback: SignUpSubmitUserAttributesCallback)"
617617
)
618618
NativeAuthPublicClientApplication.pcaScope.launch {
619619
try {

0 commit comments

Comments
 (0)