AzureAD / microsoft-authentication-library-for-objc

Microsoft Authentication Library (MSAL) for iOS and macOS
http://aka.ms/aadv2
MIT License
259 stars 141 forks source link

Unify log levels in Native Auth #2184

Closed diegojerezba closed 2 months ago

diegojerezba commented 3 months ago

Proposed changes

This PR unifies the logging level across all the NativeAuth classes using the MSAL Obj logging criteria. As some of these decisions can be a bit subjective, opinionated comments are more than welcome.

Main changes:

Type of change

Risk

nilo-ms commented 3 months ago

@diegojerezba can you please clarify when different of log levels are used? And can you align with MSAL Android too? Thanks

spetrescu84 commented 2 months ago

I'm looking at the 3 .verbose messages in MSALNativeAuthSignUpController.swift: MSALLogger.log(level: .verbose, context: context, format: "credential_required received in signup/continue request") MSALLogger.log(level: .verbose, context: context, format: "attributes_required received in signup/continue request: \(attributes)") MSALLogger.log(level: .verbose, context: context, format: "attributes_required received in signup/continue request: \(attributes)") Should they also be .info? The ones in the other files - MSALNativeAuthCredentialsController.swift and MSALNativeAuthTokenResponseValidating.swift look ok. Overall we have a way fewer verbose messages than the ones in the .m files

diegojerezba commented 2 months ago

MSALNativeAuthCredentialsController

I'm looking at the 3 .verbose messages in MSALNativeAuthSignUpController.swift: MSALLogger.log(level: .verbose, context: context, format: "credential_required received in signup/continue request") MSALLogger.log(level: .verbose, context: context, format: "attributes_required received in signup/continue request: \(attributes)") MSALLogger.log(level: .verbose, context: context, format: "attributes_required received in signup/continue request: \(attributes)") Should they also be .info? The ones in the other files - MSALNativeAuthCredentialsController.swift and MSALNativeAuthTokenResponseValidating.swift look ok. Overall we have a way fewer verbose messages than the ones in the .m files

I've changed the logs in MSALNativeAuthSignUpController.swift into .info. Thanks @spetrescu84