blackflux / object-scan

Traverse object hierarchies using matching and callbacks.
MIT License
164 stars 16 forks source link

Data sanitisation question #1676

Closed rickysullivan-gallagher closed 2 years ago

rickysullivan-gallagher commented 2 years ago

Not a question, just a request for comment :)

I'm using object-scan for data sanitisation before sending my apps sate to Sentry.

const rules = [
    'keycloakJWT.(refreshToken|refreshTokenParsed.(jti|nonce)|idToken|token|(idTokenParsed|tokenParsed).(email|family_name|jti|given_name|name|nonce|preferred_username))',
    'meData.(userName|profiles[*].(siteName|sitegroupName))',
    'fidoJWT.token',
    'router.location.hash',
    'site.(email|firstname|installerName|installerServiceEmail|installerServicePhone|name|organization|organizationName|phone|surname)',
    'users.byID.*.(email|firstName|lastName|phone)',
    'history.arming.events.*.(organization|relatedItems|names)',
    'incidents.byID.*.(claimedByName|closedByName|initialAlarm.userName)'
];

const OMIT_MESSAGE = '***<OMITTED>***';

const sanitiseState = objectScan(['**'], {
    joined: true,
    rtn: 'context',
    filterFn: ({ key, property, parent }) => {
        if (picomatch.isMatch(key, rules, { nobracket: true })) {
            parent[property] = OMIT_MESSAGE;
        }
        return true;
    }
});

const sentryReduxEnhancer = Sentry.createReduxEnhancer({
    stateTransformer: state => {
        if (!FEATURE_FLAGS.REDUX_FOR_SENTRY.isEnabled()) return null;

        const clonedState = cloneDeep(state);

        sanitiseState(clonedState);

        return clonedState;
    }
});

I'm currently using picomatch-browser to perform the check on the keys of the state to filter out the values.

Would you do this differently? Could I use object-scan alone to perform this task?

simlu commented 2 years ago

Hello! Absolutely you can do that. You'd just have to rewrite your expressions to conform to the object scan syntax. Here is a library that does this already using object-scan: https://github.com/blackflux/lambda-serverless-api/blob/2f8dbcdd695403d315c6f4eafe51257c985a3d5b/src/plugin/logger.js#L28

simlu commented 2 years ago

Oops, actually hit the wrong button...

Let me know if you have any questions and I'd be very happy to assist! If you need to retain the original data you might need to do a deep clone first.

rickysullivan-gallagher commented 2 years ago

Rad. Thanks for your help.

I have a better understanding of how it works now.

const redactor = (obj, rules, REDACTED_MESSAGE = '***REDACTED***') => {
    if (!FEATURE_FLAGS.REDUX_FOR_SENTRY.isEnabled()) return null;

    objectScan(rules, {
        joined: false,
        useArraySelector: true,
        filterFn: ({ key, parents }) => {
            parents[0][key[key.length - 1]] = REDACTED_MESSAGE;
        }
    })(obj);

    return obj;
};

const redactRules = [
    'error.config.headers.Authorization',
    'fidoJWT.{jti,preferred_username,token}',
    '{incidents.byID.*,incidents.*}.{*ByName,initialAlarm.{userName,source}}',
    'invitations.*[*].{invitationUrl,siteName}',
    'keycloakJWT.{refreshToken,idToken,token,*Parsed.{jti,nonce,*email*,*name*}}',
    'meData.**.*Name',
    'router.location.hash',
    '{response.content[*],history.arming.events.*}.{names[*].value,organization,relatedItems[*].value}',
    '{payload,**}site.{*name,email,phone,organization*,installerService*,installerName}',
    'userCredential*.**.{id,sessionUrl,facilityName}',
    'userDeviceData.notificationToken',
    '{data,{{payload,*}.users.{byID.*,*}}}.{email,*Name,phone}'
];

const sentryReduxEnhancer = Sentry.createReduxEnhancer({
    actionTransformer: action => redactor(cloneDeep(action), redactRules),
    stateTransformer: state => redactor(cloneDeep(state), redactRules)
});
simlu commented 2 years ago

That looks good. Glad you figured it out! Using parent and property might make this a bit more readable, so you could do

parent[property] = REDACTED_MESSAGE;

And I don't think you need the flags. The defaults should be the same.

objectScan(rules, {
        filterFn: ({ parent, property }) => {
            parent[property] = REDACTED_MESSAGE;
        }
    })(obj);

Also keep in mind that if performance is a concern, you could separate the expensive compilation stage from the rewrite stage, ie initialize one with the rules and then call with different obj. But I suspect this is plenty fast enough for you.

const Init = (rules, REDACTED_MESSAGE) => {
  const rewriter = objectScan(rules, {
        filterFn: ({ parent, property }) => {
            parent[property] = REDACTED_MESSAGE;
        }
    });
  return (obj) => rewriter(obj);
} 

Otherwise your code looks good!

rickysullivan-gallagher commented 2 years ago

Oh right.

I turned it into a class:

const action = {
  type: "LOGIN_SUCCESS",
  keycloak: {
    authenticated: true,
    loginRequired: true,
    redirectUri: "http://localhost:8081/index.html",
    silentCheckSsoFallback: true,
    enableLogging: false,
    messageReceiveTimeout: 10000,
    responseMode: "fragment",
    responseType: "code",
    flow: "standard"
  }
};

const redactRules = ["keycloak.{flow,redirectUri}"];

class Redactor {
  #replacerFunc;

  constructor(rules, REDACTED_MESSAGE = "***REDACTED***") {
    this.rules = rules;
    this.REDACTED_MESSAGE = REDACTED_MESSAGE;
    this.#init();
  }

  #init() {
    this.#replacerFunc = objectScan(this.rules, {
      filterFn: ({ parent, property }) => {
        parent[property] = this.REDACTED_MESSAGE;
      }
    });
  }

  run(obj) {
    this.#replacerFunc(obj);
    return obj;
  }
}

const redactor = new Redactor(redactRules);

console.log(redactor.run(cloneDeep(action)));
rickysullivan-gallagher commented 2 years ago

On the flipside, how would I mark all values as redacted except the ones specified in the rules? Keeping all leaf nodes.

simlu commented 2 years ago

That class looks good 👍

On the flipside, how would I mark all values as redacted except the ones specified in the rules? Keeping all leaf nodes.

You could use '**' and then exclude the needles by prefixing with an exclamation mark. In the filterFn you'd have to check isLeaf and then rewrite the value.

So for example

objectScan(['**', '!keep.me'], { filterFn: ({ isLeaf, parent, property }) => {
  if(isLeaf) {
   // rewrite here
 } 
}})(obj)

Let me know if that makes sense!

rickysullivan-gallagher commented 2 years ago

Perfect. I was trying to ['**'] but it was the isLeaf that I was missing. Thanks for much for your help. ⭐️