-
Notifications
You must be signed in to change notification settings - Fork 126
fix(arcgis-rest-request): postMessage auth works w/ credential #1228
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
Conversation
const serverInfo = { | ||
hasPortal: true, | ||
hasServer: false, | ||
server: credential.server | ||
} as IServerInfo; | ||
return ArcGISIdentityManager.fromCredential(credential, serverInfo); |
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.
@patrickarlt it seems to me that the serverInfo
argument to fromCredential
could be optional b/c it's only used if (serverInfo.hasServer)
- seems like we could change that to if (serverInfo && serverInfo.hasServer) {
and make it optional.
B/c I wanted this fix to be minimally invasive, I left that out of this and just stubbed in a dummy serverInfo here, but if you agree, I could add that in a future PR.
… IArcGISIdentityManagerOptions
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.
This looks good!
We have released this fix as |
FYI - further evidence that this was likely not an intentional change, the 4.x docs still show |
This looks good in my testing. These are the objects 3.8.0 {
"username": "dpaddock",
"token": "xyzzy...",
"tokenExpires": "2025-05-15T21:52:16.152Z",
"portal": "https://www.arcgis.com/sharing/rest",
"ssl": true,
"tokenDuration": 20160,
"refreshTokenTTL": 20160
} 4.5.0 {
"token": "xyzzy...",
"portal": "https://www.arcgis.com/sharing/rest",
"ssl": true,
"tokenDuration": 20160,
"server": "https://www.arcgis.com"
} 4.5.0-alpha.1 {
"username": "dpaddock",
"token": "xyzzy...",
"tokenExpires": "2025-05-15T21:55:26.980Z",
"portal": "https://www.arcgis.com/sharing/rest",
"ssl": true,
"tokenDuration": 20160
} 4.5.0-alpha.1 is almost the same as 3.8.0 except it doesn't include |
Background
The Hub team is in the process of migrating from ArcGIS REST JS 3.x to 4.x and we discovered a change in the payload that is sent/expected when passing authentication from a parent app to an embedded app via postMessage. For us, this manifested as a failure to pass authentication from the Hub to an embedded Dashboard, but surprisingly not to an embedded Survey123 application.
Furthermore, we consider this change in payload to be a bug, b/c the old payload, an ICredential, was suitable for initiating authentication w/ either ArcGIS REST JS or the Maps SDK for JavaScript, whereas the new payload, an IArcGISIdentityManagerOptions, would need to be transformed to work with the Maps SDK for JavaScript.
We suspect that some ArcGIS apps either have not upgraded to 4.x, or have made accommodations to handle both the old and new payload formats.
Proposal
We propose:
ICredential
format that is compatible w/ the Maps SDK (i.e. can be passed directly to esriId.registerToken()), REST JS 3.x, and (as of this PR) REST JS 4.xICredential
payload, but also fallback to handling theIArcGISIdentityManagerOptions
format that would have been sent by a 4.x parent that has not yet updated to the latest version.Status
We thought we addressed this in #1223, we missed a couple of things, which are now included in this PR:
createPostMessageHandler
), but we did not update the function that an embedded app would use to create an ArcGISIdentityManager instance from the message (parentMessageHandler
) - that's still expecting the shape oftoJSON()
.I have confirmed that this worked in our app for at least dashboards and surveys: