Open skitterm opened 3 years ago
@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.
Allow optional, broader origin-matching for
enablePostMessageAuth
to include anything on.arcgis.com
domain.
👍🏻
Sounds good, I'll get a PR in within the next few workdays. 2 questions:
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.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)
One more:
*.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.http://localhost/ is considered a secure context. Also, Enterprise still supports http:
.
- 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.
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:Calling
enablePostMessageAuth
would either look like:or (current behavior)
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 parameterwin
). Not sure how to handle that; ifwin
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: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
andenablePostMessageAuth
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