Skip to content
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

Request tokens ignored on dev/qa servers if portal URL and UserSession.portal don't match #583

Closed
ssylvia opened this issue Jun 11, 2019 · 12 comments · Fixed by #596
Closed
Labels

Comments

@ssylvia
Copy link
Member

ssylvia commented Jun 11, 2019

When using request with ArcGIS Online Dev/QA environments, a token will not be added to the request unless the portal URL matches the authentication UserSession.portal.

The dev/qa owningSystemUrl is not recognized as a ArcGIS Online server
https://github.com/Esri/arcgis-rest-js/blob/master/packages/arcgis-rest-auth/src/UserSession.ts#L826

@tomwayson tomwayson added the bug label Jun 12, 2019
@timmorey
Copy link

timmorey commented Jun 20, 2019

I think I just ran into this issue, due to a scheme mismatch. However, I don't think the issue is limited to the dev/qa environments.

My UserSession.portal had an https: url. However, I was signed in to an org that did not not require https access (allSSL: false). The info endpoint returned an owningSystemUrl with an http: url, so the comparison cited above failed.

Should the comparison ignore the scheme?

Edit: for example, see https://services8.arcgis.com/Q1W9j3Lr1BaxMiWi/arcgis/rest/info?f=json

@qlqllu
Copy link
Contributor

qlqllu commented Jul 1, 2019

I receive the NOT_FEDERATED, which I think it's caused by this reason.

The portal in session is the org URL: beijing.mapsdevext.arcgis.com, and the feature service URL is: servicedev.arcgis.com.

I think these 2 URLs should be considered as federated.

@tomwayson
Copy link
Member

tomwayson commented Jul 3, 2019

@qlqllu

If your portal was production org, then I believe it would pass this test and the user's token current would be used (or refreshed if needed):

(arcgisOnlinePortalRegex.test(this.portal) ||
arcgisOnlineOrgPortalRegex.test(this.portal)) &&
arcgisOnlineUrlRegex.test(url)

This fails in your case b/c your portal is a devext org and fails this test:

/**
* Used to test if a URL is an ArcGIS Online Organization Portal
*/
const arcgisOnlineOrgPortalRegex = /^https?:\/\/(?:[a-z0-9-]+\.maps)?.\arcgis\.com\/sharing\/rest/;

We could modify that test to allow for dev/qa environments like:

/^https?:\/\/(?:[a-z0-9-]+\.maps(?:devext|qaext)?)?.\arcgis\.com\/sharing\/rest/.test('https://beijing.mapsdevext.arcgis.com/sharing/rest') // true

However, my concern is that the arcgisOnlineUrlRegex does not currently differentiate between prod/qa/dev environments.

/**
* Used to test if a URL is an ArcGIS Online URL
*/
const arcgisOnlineUrlRegex = /^https?:\/\/\S+\.arcgis\.com.+/;

What that means is that we currently allow passing prod tokens to dev/qa servers, and if we were to open up arcgisOnlineOrgPortalRegex (and arcgisOnlinePortalRegex) to pass for qa/dev environments as I propose above, then we could be sending tokens from any environment (prod/qa/dev) to servers from any enviroment.

In other words, I think we want to re-work the logic to be:

"if the users's portal is either AGO itself or an AGO org for a given environment, AND the (server or portal API) URL is from the same AGO environment, then use or refresh the user's current token"

@patrickarlt do you agree?

@tomwayson
Copy link
Member

tomwayson commented Jul 3, 2019

Hm... Now that I think about it, wonder if we can distinguish between the various environements of hosted server URLs. In other words, are dev servers always servicedev?

@tomwayson
Copy link
Member

Ha! related:

#594 (comment)

@tomwayson
Copy link
Member

To clarify, my concer above is not so much that it's insecure to send a token from one ArcGIS Online environment to another (i.e. a dev token to a prod server), it's that the token will be invalid.

@qlqllu
Copy link
Contributor

qlqllu commented Jul 3, 2019

Look at some services, it seems the hosted service in dev is servicedev, in qa is serviceqa, I guest this is the correct rule, may someone help check?

@jgravois
Copy link
Contributor

jgravois commented Jul 3, 2019

are dev servers always servicedev?

FWIW, when I was helping the ArcGIS Urban folks a few months back they had a dev enviorment in which attempting to create a hosted feature service using the admin API with one specific name resulted in a production url (because there was an orphaned service definition of the same name on 'servicesdev').

I'd call that an edge-edge case though. ArcGIS REST JS is probably safe to expect the pattern. I guess I'm just bringing my own experience up to mention that logging the result of your expectations if things go pear shaped as a breadcrumb wouldn't be a waste of time.

reading 🌈

@patrickarlt
Copy link
Contributor

I think this is happening in 2 similar but different cases because unless the URL passes the tests in https://github.com/Esri/arcgis-rest-js/blob/master/packages/arcgis-rest-auth/src/UserSession.ts#L725-L729 the UserSession.portal must exactly matches the owningSystemUrl in order to be considered for federation.

This means that the following cases fail to federate:

  1. UserSession.portal === "https://someorg.mapsdev.arcgis.com/sharing/rest/" this actually appears to be a fairly common case for apps like Storymaps. since in dev owningSystemUrl is always https://devext.arcgis.com these don't match and federation doesn't happen. This was the original issue from @qlqllu https://github.com/Esri/arcgis-rest-js/blob/b956d910aadf76729190a54b6af0d3286cc11079/packages/arcgis-rest-auth/src/UserSession.ts#issuecomment-507092212. I think this is also the original issue reported by @ssylvia.
  2. In cases where there is a scheme mismatch between http:// and https:// which is what @timmorey reported in https://github.com/Esri/arcgis-rest-js/blob/b956d910aadf76729190a54b6af0d3286cc11079/packages/arcgis-rest-auth/src/UserSession.ts#issuecomment-504144004.

@tomwayson has the right solution since we cant consider devext to be federated with production or vica versa so you would have to:

  1. Determine if UserSession.portal is ANY ArcGIS Online Environment.
  2. Get the ArcGIS Online Environment of UserSession.portal.
  3. Determine if url is ANY ArcGIS Online Environment.
  4. Get the ArcGIS Online Environment of url.
  5. If 1 and 3 are true AND 2 and 4 match then we can short cut federation.

The alternative would be to modify https://github.com/Esri/arcgis-rest-js/blob/master/packages/arcgis-rest-auth/src/UserSession.ts#L826 with new tests to make sure that things like https://someorg.mapsdev.arcgis.com/sharing/rest/ and https://devext.arcgis.com are considered federated and to resolve the http/https issue.

@patrickarlt
Copy link
Contributor

@ssylvia @timmorey @qlqllu I can find some time to work on this but testing it is going to be a huge pain so it will take some time. In the meantime you can just use 2.0.1 since I introduced this behavior in 2.0.2.

@tomwayson
Copy link
Member

@patrickarlt

The alternative would be to modify https://github.com/Esri/arcgis-rest-js/blob/master/packages/arcgis-rest-auth/src/UserSession.ts#L826 with new tests to make sure that things like https://someorg.mapsdev.arcgis.com/sharing/rest/ and https://devext.arcgis.com are considered federated

I thought about that, and it does seem easier. However, my concern there is that our apps would be using different auth flows in dev/qa enviroments than prod environments. I think checking the conditions you enumerated are the best way to go.

@tomwayson
Copy link
Member

@ssylvia @timmorey - this is released in v2.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants