Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mask EUII logs #1206

Merged
merged 7 commits into from
Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## TBD
* Mask EUII in logs (#1206)

## [1.1.14] - 2020-01-19
* Removed identity core classes from public api (#1158).
* Fixed possible deadlock caused by thread explosion (#1175)
Expand Down
2 changes: 1 addition & 1 deletion MSAL/IdentityCore
Submodule IdentityCore updated 27 files
+3 −3 IdentityCore/src/cache/MSIDKeychainTokenCache.m
+9 −9 IdentityCore/src/cache/accessor/MSIDAccountCredentialCache.m
+4 −4 IdentityCore/src/cache/accessor/MSIDDefaultTokenCacheAccessor.m
+7 −7 IdentityCore/src/cache/accessor/MSIDLegacyTokenCacheAccessor.m
+1 −1 IdentityCore/src/cache/metadata/MSIDAccountMetadataCacheAccessor.m
+1 −1 IdentityCore/src/cache/token/MSIDAccountCacheItem.m
+1 −0 IdentityCore/src/logger/MSIDLogger+Internal.h
+17 −3 IdentityCore/src/logger/MSIDLogger.h
+10 −9 IdentityCore/src/logger/MSIDLogger.m
+2 −2 IdentityCore/src/logger/MSIDLoggerConnecting.h
+5 −0 IdentityCore/src/logger/MSIDMaskedHashableLogParameter.m
+4 −1 IdentityCore/src/logger/MSIDMaskedLogParameter.h
+38 −8 IdentityCore/src/logger/MSIDMaskedLogParameter.m
+13 −2 IdentityCore/src/logger/MSIDMaskedUsernameLogParameter.m
+1 −1 IdentityCore/src/oauth2/account/MSIDAccount.m
+1 −1 IdentityCore/src/oauth2/token/MSIDLegacyRefreshToken.m
+1 −1 IdentityCore/src/requests/MSIDOIDCSignoutRequest.m
+1 −1 IdentityCore/src/requests/broker/MSIDBrokerTokenRequest.m
+1 −1 IdentityCore/src/requests/sdk/adal/MSIDLegacyTokenResponseValidator.m
+19 −0 IdentityCore/src/util/NSDictionary+MSIDLogging.m
+1 −1 IdentityCore/src/webview/embeddedWebview/challangeHandlers/MSIDPKeyAuthHandler.m
+1 −1 IdentityCore/src/webview/operations/MSIDWebOpenBrowserResponseOperation.m
+83 −18 IdentityCore/tests/MSIDLoggerTests.m
+9 −9 IdentityCore/tests/MSIDMaskedLogParameterTests.m
+23 −7 IdentityCore/tests/MSIDMaskedUsernameLogParameterTests.m
+1 −1 IdentityCore/tests/util/MSIDTestLogger.m
+1 −1 changelog.txt
2 changes: 1 addition & 1 deletion MSAL/src/MSALAccountEnumerationParameters.m
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ - (instancetype)initWithTenantProfileIdentifier:(nonnull NSString *)tenantProfil

- (NSString *)description
{
return [NSString stringWithFormat:@"Account identifier %@, username %@, tenant profile identifier %@, return only signed in accounts %d", self.identifier, self.username, self.tenantProfileIdentifier, self.returnOnlySignedInAccounts];
return [NSString stringWithFormat:@"Account identifier %@, username %@, tenant profile identifier %@, return only signed in accounts %d", self.identifier, MSID_PII_LOG_EMAIL(self.username), self.tenantProfileIdentifier, self.returnOnlySignedInAccounts];
}

@end
4 changes: 2 additions & 2 deletions MSAL/src/MSALPublicClientApplication.m
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ - (MSALAccount *)accountForIdentifier:(NSString *)identifier
- (NSArray<MSALAccount *> *)accountsForParameters:(MSALAccountEnumerationParameters *)parameters
error:(NSError **)error
{
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Querying MSAL accounts with parameters (identifier=%@, tenantProfileId=%@, username=%@, return only signed in accounts %d)", MSID_PII_LOG_MASKABLE(parameters.identifier), MSID_PII_LOG_MASKABLE(parameters.tenantProfileIdentifier), MSID_PII_LOG_EMAIL(parameters.username), parameters.returnOnlySignedInAccounts);
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Querying MSAL accounts with parameters (identifier=%@, tenantProfileId=%@, username=%@, return only signed in accounts %d)", MSID_PII_LOG_TRACKABLE(parameters.identifier), MSID_PII_LOG_MASKABLE(parameters.tenantProfileIdentifier), MSID_PII_LOG_EMAIL(parameters.username), parameters.returnOnlySignedInAccounts);

MSALAccountsProvider *request = [[MSALAccountsProvider alloc] initWithTokenCache:self.tokenCache
accountMetadataCache:self.accountMetadataCache
Expand Down Expand Up @@ -1206,7 +1206,7 @@ - (void)acquireTokenWithParameters:(MSALInteractiveTokenParameters *)parameters
" claimsRequest:%@]",
parameters.scopes,
parameters.extraScopesToConsent,
MSID_PII_LOG_MASKABLE(parameters.account.homeAccountId),
MSID_PII_LOG_TRACKABLE(parameters.account.homeAccountId),
MSID_PII_LOG_EMAIL(parameters.loginHint),
MSALStringForPromptType(parameters.promptType),
parameters.extraQueryParameters,
Expand Down
20 changes: 18 additions & 2 deletions MSAL/src/configuration/MSALLoggerConfig.m
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,28 @@ - (MSALLogLevel)logLevel

- (void)setPiiEnabled:(BOOL)piiEnabled
{
[MSIDLogger sharedLogger].piiLoggingEnabled = piiEnabled;
[MSIDLogger sharedLogger].logMaskingLevel = piiEnabled ? MSIDLogMaskingSettingsMaskSecretsOnly : MSIDLogMaskingSettingsMaskAllPII;
}

- (BOOL)piiEnabled
{
return [MSIDLogger sharedLogger].piiLoggingEnabled;
switch ([MSIDLogger sharedLogger].logMaskingLevel) {
case MSIDLogMaskingSettingsMaskAllPII:
return NO;

default:
return YES;
}
}

- (MSALLogMaskingLevel)logMaskingLevel
{
return (MSALLogMaskingLevel)[MSIDLogger sharedLogger].logMaskingLevel;
}

- (void)setLogMaskingLevel:(MSALLogMaskingLevel)logMaskingLevel
{
[MSIDLogger sharedLogger].logMaskingLevel = (MSIDLogMaskingLevel)logMaskingLevel;
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ - (instancetype)initWithJSONDictionary:(NSDictionary *)jsonDictionary error:(NSE
_username = [jsonDictionary msidStringObjectForKey:@"username"];
_accountClaims = claims;

MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Created external ADAL account with identifier %@, object Id %@, tenant Id %@, name %@, username %@, claims %@", MSID_PII_LOG_TRACKABLE(_identifier), MSID_PII_LOG_MASKABLE(_objectId), _tenantId, MSID_PII_LOG_MASKABLE(displayName), MSID_PII_LOG_EMAIL(_username), MSID_PII_LOG_MASKABLE(_accountClaims));
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Created external ADAL account with identifier %@, object Id %@, tenant Id %@, name %@, username %@, claims %@", MSID_PII_LOG_TRACKABLE(_identifier), MSID_PII_LOG_TRACKABLE(_objectId), _tenantId, MSID_EUII_ONLY_LOG_MASKABLE(displayName), MSID_PII_LOG_EMAIL(_username), MSID_EUII_ONLY_LOG_MASKABLE(_accountClaims));
}

return self;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ - (instancetype)initWithJSONDictionary:(NSDictionary *)jsonDictionary error:(NSE
}

_signinStatusDictionary = [jsonDictionary msidObjectForKey:@"signInStatus" ofClass:[NSDictionary class]];
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Created sign in status dictionary %@", MSID_PII_LOG_MASKABLE(_signinStatusDictionary));
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Created sign in status dictionary %@", MSID_EUII_ONLY_LOG_MASKABLE(_signinStatusDictionary));
}

return self;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ - (instancetype)initWithSharedKeychainAccessGroup:(NSString *)sharedGroup

- (BOOL)updateAccount:(id<MSALAccount>)account idTokenClaims:(NSDictionary *)idTokenClaims error:(__unused NSError **)error
{
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Updating account %@", MSID_PII_LOG_MASKABLE(account));
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Updating account %@", MSID_EUII_ONLY_LOG_MASKABLE(account));

[self updateAccountAsync:account
idTokenClaims:idTokenClaims
Expand Down Expand Up @@ -248,7 +248,7 @@ - (BOOL)removeAccount:(id<MSALAccount>)account
tenantProfiles:(nullable NSArray<MSALTenantProfile *> *)tenantProfiles
error:(NSError * _Nullable * _Nullable)error
{
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Removing account %@", MSID_PII_LOG_MASKABLE(account));
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Removing account %@", MSID_EUII_ONLY_LOG_MASKABLE(account));

__block BOOL result = YES;
__block NSError *removeError;
Expand Down Expand Up @@ -416,7 +416,7 @@ - (BOOL)updateAccountImpl:(id<MSALAccount>)account
return NO;
}

MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Updating accounts %@", MSID_PII_LOG_MASKABLE(accounts));
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Updating accounts %@", MSID_EUII_ONLY_LOG_MASKABLE(accounts));

NSError *saveError = nil;
BOOL saveResult = [self saveUpdatedAccount:account
Expand Down Expand Up @@ -450,7 +450,7 @@ - (BOOL)saveUpdatedAccount:(id<MSALAccount>)account
NSString *versionIdentifier = [self accountVersionIdentifier:version];
NSMutableDictionary *resultDictionary = jsonObject ? [[jsonObject jsonDictionary] mutableCopy] : [NSMutableDictionary new];

MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Updating accounts %@", MSID_PII_LOG_MASKABLE(accounts));
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Updating accounts %@", MSID_EUII_ONLY_LOG_MASKABLE(accounts));

for (MSALLegacySharedAccount *sharedAccount in accounts)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ - (instancetype)initWithJSONDictionary:(NSDictionary *)jsonDictionary error:(NSE
_accountClaims = @{@"tid": MSID_DEFAULT_MSA_TENANTID,
@"oid": uid};

MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Created external MSA account with identifier %@, object Id %@, tenant Id %@, username %@, claims %@", MSID_PII_LOG_TRACKABLE(_identifier), MSID_PII_LOG_MASKABLE(cid), MSID_DEFAULT_MSA_TENANTID, MSID_PII_LOG_EMAIL(_username), MSID_PII_LOG_MASKABLE(_accountClaims));
MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Created external MSA account with identifier %@, object Id %@, tenant Id %@, username %@, claims %@", MSID_PII_LOG_TRACKABLE(_identifier), MSID_PII_LOG_TRACKABLE(cid), MSID_DEFAULT_MSA_TENANTID, MSID_PII_LOG_EMAIL(_username), MSID_EUII_ONLY_LOG_MASKABLE(_accountClaims));
}

return self;
Expand Down
23 changes: 22 additions & 1 deletion MSAL/src/public/configuration/global/MSALLoggerConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@
#import <Foundation/Foundation.h>
#import "MSALDefinitions.h"

/*! Levels of log masking */
typedef NS_ENUM(NSInteger, MSALLogMaskingLevel)
{
/** MSAL will not return any messages with any user or organizational information. This includes EUII and EUPI. This is the default level. */
MSALLogMaskingSettingsMaskAllPII,

/** MSAL logs will still include OII (organization identifiable information), and EUPI (end user pseudonymous identifiers), but MSAL will try to exclude and/or mask any EUII (end user identifiable information) like UPN, username, email from its logs. */

MSALLogMaskingSettingsMaskEUIIOnly,

/** MSAL logs will still include OII (organization identifiable information), EUPI (end user pseudonymous identifiers), and EUII (end user identifiable information) like UPN, username, email from its logs. MSAL will still hide all secrets like tokens from its logs */
MSALLogMaskingSettingsMaskSecretsOnly
};

NS_ASSUME_NONNULL_BEGIN

/**
Expand All @@ -45,7 +59,14 @@ NS_ASSUME_NONNULL_BEGIN
/**
MSAL provides logging callbacks that assist in diagnostics. There is a boolean value in the logging callback that indicates whether the message contains user information. If piiEnabled is set to NO, the callback will not be triggered for log messages that contain any user information. By default the library will not return any messages with user information in them.
*/
@property (atomic) BOOL piiEnabled;
@property (atomic) BOOL piiEnabled DEPRECATED_MSG_ATTRIBUTE("Use logMaskingLevel instead");

/**
MSAL provides logging callbacks that assist in diagnostics. By default the library will not return any messages with any user or organizational information. However, this might make diagnosing issues difficult.
logMaskingLevel property can be used to adjust level of MSAL masking.
Default value is MSALLogMaskingSettingsMaskAllPII.
*/
@property (atomic) MSALLogMaskingLevel logMaskingLevel;
oldalton marked this conversation as resolved.
Show resolved Hide resolved

#pragma mark - Setting up the logging callback

Expand Down
2 changes: 1 addition & 1 deletion MSAL/test/app/ios/MSALTestAppLogViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ - (id)init

[self setEdgesForExtendedLayout:UIRectEdgeNone];

MSALGlobalConfig.loggerConfig.piiEnabled = YES;
MSALGlobalConfig.loggerConfig.logMaskingLevel = MSALLogMaskingSettingsMaskEUIIOnly;
[MSALGlobalConfig.loggerConfig setLogCallback:^(MSALLogLevel level, NSString * _Nullable message, __unused BOOL containsPII)
{
(void)level;
Expand Down