Add opt in callback and redirect variable control

[shawn-dhillon]

@Ryanmtate @ottonomy @bryceteinck

[ottonomy]

@shawn-dhillon thanks for the contribution. I was traveling this week but will look soon. I think probably untrustedVariableAllowList should just be used for this purpose rather than introducing additional categories.

The static headers addition seems practical.

[shawn-dhillon]

@shawn-dhillon thanks for the contribution. I was traveling this week but will look soon. I think probably untrustedVariableAllowList should just be used for this purpose rather than introducing additional categories.

The static headers addition seems practical.

Thank you for the feedback @ottonomy. Apologies for not following up here sooner, I interpreted your message as you were still reviewing and there would be more context to follow; as I may be missing context on how we would safely merge this all under untrustedVariableAllowList without impacting current functionality since these hit different axises.

The premise behind all my changes and the additional variables were for granularity and control, so that they are added configuration that no op if unset. To review:

• untrustedVariableAllowList is inbound where the caller supplies the accepted vars into the exchange.
• callback.variableAllowList is outbound to the relying backend with the intent of adding control for what vars go into the POST and minimizing the data there so that the presentation isn't included
• redirectVariableAllowList is outbound to the caller and dictates what is encoded into the post verification redirect uri

My concern is reusing untrustedVariableAllowList for the callback.variableAllowList would impact existing use cases where they accept some inbound variables and do a callback where the presentation is needed, in this case they would have to define the presentation keys in the untrustedVariableAllowList. Reusing it for redirectVariableAllowList would force all exchange vars into the redirect uri impacting existing use cases that have a lot of inbound vars breaking the caller's ability to follow the redirect uri.

I am interpreting your goal as to minimize the config surface. For callback.variableAllowList which has the goal of minimizing the data we include in the payload and stripping the presentation from it we could potentially use scrubExchangeForPartialScope set with literal instead of an array of vars ex:

  const source = callback.exchangeScope === 'partial' ?
    scrubExchangeForPartialScope(exchange) : exchange;
  const payload = {
    id: `${config.server.baseUri}/workflows/${exchange.workflowId}` +
        `/exchanges/${exchange.id}`,
    variables: source.variables,
    step: 'default'
  };

which would strip the presentation and vp token, though we would loose some control and force all exchange vars and keep the rest of the meta fields but still enact data minimization to prevent sending PII to the relying backend.

For redirectVariableAllowList we could just send responseVariable which contains the callback response and always encode this into the redirect uri when set. This caters a bit more to this specific bespoke use case and we lose the control to set exactly what goes into it, but does accomplish the goal. ex:

        let redirect_uri =
          `${baseUri}/${procedurePath}?exchange_token=${token}`;
        const {variables} = updatedExchange;
        const responseVar = workflow.callback?.responseVariable;
        if(responseVar && variables && variables[responseVar] !== undefined) {
          const raw = variables[responseVar];
          const value = typeof raw === 'string' ? raw : JSON.stringify(raw);
          redirect_uri +=
            `&${encodeURIComponent(responseVar)}=${encodeURIComponent(value)}`;
        }

I am more than happy to align or discuss more in case I may be missing your implication and my concerns are unwarranted. Thank you again for your feedback.