-
Notifications
You must be signed in to change notification settings - Fork 57
feat: Obfuscate potential PII in logs #1163
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
base: development
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import { | |
| isFunction, | ||
| AttributeValue, | ||
| isEmpty, | ||
| obfuscateData, | ||
| } from "./utils"; | ||
| import { SDKIdentityApi } from "./identity.interfaces"; | ||
| import { SDKLoggerApi } from "./sdkRuntimeModels"; | ||
|
|
@@ -202,7 +203,7 @@ export default class RoktManager { | |
| const { attributes } = options; | ||
| const sandboxValue = attributes?.sandbox || null; | ||
| const mappedAttributes = this.mapPlacementAttributes(attributes, this.placementAttributesMapping); | ||
| this.logger?.verbose(`mParticle.Rokt selectPlacements called with attributes:\n${JSON.stringify(attributes, null, 2)}`); | ||
| this.logger?.verbose(`mParticle.Rokt selectPlacements called with attributes:\n${JSON.stringify(obfuscateData(attributes), null, 2)}`); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was added for our support staff to be able to easily see what a customer initially passed to selectPlacements prior to us enriching the attributes with other MP related items. We may need to remove this one temporarily, or provide another logLevel, like |
||
|
|
||
| this.currentUser = this.identityService.getCurrentUser(); | ||
| const currentUserIdentities = this.currentUser?.getUserIdentities()?.userIdentities || {}; | ||
|
|
@@ -241,7 +242,7 @@ export default class RoktManager { | |
| newIdentities[this.mappedEmailShaIdentityType] = newHashedEmail; | ||
| this.logger.warning(`emailsha256 mismatch detected. Current mParticle hashedEmail differs from hashedEmail passed to selectPlacements call. Proceeding to call identify with hashedEmail from selectPlacements call. Please verify your implementation.`); | ||
| } | ||
|
|
||
| if (!isEmpty(newIdentities)) { | ||
| // Call identify with the new user identities | ||
| try { | ||
|
|
@@ -256,7 +257,8 @@ export default class RoktManager { | |
| }); | ||
| }); | ||
| } catch (error) { | ||
| this.logger.error('Failed to identify user with new email: ' + JSON.stringify(error)); | ||
| const errorMessage = error instanceof Error ? error.message : JSON.stringify(error); | ||
| this.logger.error('Failed to identify user with updated identities: ' + errorMessage); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -472,7 +474,7 @@ export default class RoktManager { | |
| return; | ||
| } | ||
|
|
||
| this.logger?.verbose(`RoktManager: Processing queued message: ${message.methodName} with payload: ${JSON.stringify(message.payload)}`); | ||
| this.logger?.verbose(`RoktManager: Processing queued message: ${message.methodName} with payload: ${JSON.stringify(obfuscateData(message.payload))}`); | ||
|
|
||
| // Capture resolve/reject functions before async processing | ||
| const resolve = message.resolve; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,38 +1,22 @@ | ||||||||||||||||||||||||||||||||||||
| import { Logger } from '@mparticle/web-sdk'; | ||||||||||||||||||||||||||||||||||||
| import { isEmpty, isNumber } from './utils'; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export interface IVaultOptions { | ||||||||||||||||||||||||||||||||||||
| logger?: Logger; | ||||||||||||||||||||||||||||||||||||
| offlineStorageEnabled?: boolean; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export abstract class BaseVault<StorableItem> { | ||||||||||||||||||||||||||||||||||||
| public contents: StorableItem; | ||||||||||||||||||||||||||||||||||||
| protected readonly _storageKey: string; | ||||||||||||||||||||||||||||||||||||
| protected logger?: Logger; | ||||||||||||||||||||||||||||||||||||
| protected storageObject: Storage; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||
| * @param {string} storageKey the local storage key string | ||||||||||||||||||||||||||||||||||||
| * @param {Storage} Web API Storage object that is being used | ||||||||||||||||||||||||||||||||||||
| * @param {IVaultOptions} options A Dictionary of IVaultOptions | ||||||||||||||||||||||||||||||||||||
| * @param {Storage} storageObject Web API Storage object that is being used | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| constructor( | ||||||||||||||||||||||||||||||||||||
| storageKey: string, | ||||||||||||||||||||||||||||||||||||
| storageObject: Storage, | ||||||||||||||||||||||||||||||||||||
| options?: IVaultOptions | ||||||||||||||||||||||||||||||||||||
| storageObject: Storage | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
| this._storageKey = storageKey; | ||||||||||||||||||||||||||||||||||||
| this.storageObject = storageObject; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Add a fake logger in case one is not provided or needed | ||||||||||||||||||||||||||||||||||||
| this.logger = options?.logger || { | ||||||||||||||||||||||||||||||||||||
| verbose: () => {}, | ||||||||||||||||||||||||||||||||||||
| warning: () => {}, | ||||||||||||||||||||||||||||||||||||
| error: () => {}, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| this.contents = this.retrieve(); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
17
to
21
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
@@ -53,13 +37,8 @@ export abstract class BaseVault<StorableItem> { | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| this.storageObject.setItem(this._storageKey, stringifiedItem); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| this.logger.verbose(`Saving item to Storage: ${stringifiedItem}`); | ||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||
| this.logger.error( | ||||||||||||||||||||||||||||||||||||
| `Cannot Save items to Storage: ${stringifiedItem}` | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| this.logger.error(error as string); | ||||||||||||||||||||||||||||||||||||
| throw new Error('Cannot Save items to Storage'); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| throw new Error('Cannot Save items to Storage'); | |
| // Swallow storage errors to avoid breaking normal SDK flows. | |
| // Log a sanitized message without including the stored payload. | |
| try { | |
| const errorMessage = | |
| error && typeof (error as Error).message === 'string' | |
| ? (error as Error).message | |
| : String(error); | |
| if (typeof console !== 'undefined' && typeof console.error === 'function') { | |
| console.error( | |
| `BaseVault.store: failed to persist data for key "${this._storageKey}".`, | |
| errorMessage | |
| ); | |
| } | |
| } catch { | |
| // If logging itself fails, do nothing to maintain non-throwing behavior. | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description mentions obfuscating the
known_identitiespayload, but this change obfuscatesmatched_identitiesin the response log. Please confirm the intended field(s) to obfuscate and update either the implementation or the PR description for consistency.