enablePostMessageAuth: Allow domain-wide origin matching

[skitterm]

Proposal: Allow optional, broader origin-matching for enablePostMessageAuth to include anything on .arcgis.com domain (and the current origin).

Problem

enablePostMessageAuth currently takes in a list of allowed origins to which credentials can be passed and the origin is matched strictly against those. In some cases, however, this list of allowed origins may not known ahead of time.

Consider an app with several embeds. These persisted embed URLs may be very different from the actual URLs used in the iframe at runtime. For example, the URL could be forced-upgraded to HTTPS, or the entire URL could be constructed at runtime from a stored ID. Another example is an embed with an ArcGIS Online organization-specific origin. All of these embeds may be on the same domain, but have different subdomains that aren't known ahead of time.

This becomes a chicken-and-egg problem, since enablePostMessageAuth needs to be called with the list of allowed origins before the embeds are rendered, but the list of allowed origins can't be known until the embeds are about ready to be rendered.

Proposed Solution

Allow calling enablePostMessageAuth without any arguments, so when no array of allowed origins is passed in, it matches against any .arcgis.com domain as well as the current origin of the app (the same logic as the ArcGIS API for JavaScript has). This can be handled with simple string matching on the origin instead of having to do a regex, as follows:

if ((event.origin.endsWith('.arcgis.com') || origin === window.location.origin) && /* other non-origin-related checks */) {
  // pass credentials
}

Calling enablePostMessageAuth would either look like:

// allow all .arcgis.com domains (and current domain)
userSession.enablePostMessageAuth();

or (current behavior)

// specify origins to allow
userSession.enablePostMessageAuth([allowedOrigins]);

In the latter case, it'd only validate against the supplied origins, just like it does today.

There'd need to be a change to enablePostMessageAuth’s signature, since currently the first argument is required (as there’s an optional 2nd parameter win). Not sure how to handle that; if win is for internal testing only, perhaps tests could be written differently, but I’m not sure of the impact or if any client apps are using that parameter.

Drawbacks

Checking that a domain ends with a specific set of characters is not as strict as doing exact matching on an entire origin. However, since string.prototype.endsWith() takes in a string instead of a regex pattern, I don't see a way that it could be fooled into allowing a wrong domain, especially since we're matching one end of the string.

Matching on the current domain also seems fine since I see no way that the window’s origin could change programmatically (besides redirecting the entire page) after the enablePostMessageAuth is called.

Alternatives

For even more customization, enablePostMessageAuth could take in a validator function that returns a boolean -- true if the origin is allowed for post messaging. This would allow the client app to have fine-grained control over what domains are allowed, without having to know the exact origins ahead-of-time:

userSession.enablePostMessageAuth((origin) => {
  return origin.endsWith('.arcgis.com')
});

However, this is risky because if a client app didn’t validate properly (i.e. a poorly-designed regex, or really any regex 😊 ), credentials could get passed to unwanted origins.

Alternatively, an updater method could be used to add to the list of allowed origins (right before each embed was rendered, for example), or the app could simply call disablePostMessageAuth and enablePostMessageAuth with an updated list each time a new embed is ready. This kind of stateful updating seems bug-prone and wasteful, however.

cc @ssylvia @dbouwman

[dbouwman]

@skitterm I think defaulting to *arcgis.com || same-origin if allowedOrigins is not passed in. I'm not wild about a validator function, for the reasons you mention.

Unless @dasa has any ZOMG issues, I'd suggest @skitterm open a PR with the proposed changes.

[dasa]

Allow optional, broader origin-matching for enablePostMessageAuth to include anything on .arcgis.com domain.

👍🏻

[skitterm]

Sounds good, I'll get a PR in within the next few workdays. 2 questions:

1. Are there any cases where we wouldn't want to allow credential passing on the same origin (since this would happen implicitly if no validChildOrigins parameter is passed)? I'm thinking of developing on domains like codesandbox.io or codepen where untrusted content can be hosted without the developer's knowledge. But I don't know why someone would develop an app with credentials on such a domain.
2. With this change, enablePostMessageAuth would have 2 optional parameters, as shown below, which could be kind of weird. If someone wanted to only pass in win, they'd have to explicitly pass undefined for validChildOrigins. This shouldn't be an issue for any existing call-sites though.

public enablePostMessageAuth(validChildOrigins?: string[], win?: any)

[skitterm]

One more:
3. For the default allowed origins (*.arcgis.com / current origin), do we want to include a protocol check so these embedded pages can only receive credentials when they are using HTTPS protocol? Should that be happening anyway, even in the current case of a specified allow-list? Since postMessage can happen from http -> https or vice versa, and I assume it's not intended for OAuth credentials to be used on/passed to an HTTP page anymore.

[dasa]

http://localhost/ is considered a secure context. Also, Enterprise still supports http:.

[dasa]

1. Are there any cases where we wouldn't want to allow credential passing on the same origin (since this would happen implicitly if no validChildOrigins parameter is passed)?

I don't think so since the child will have full access to the parent's globals if they are on the same origin.