Skip to content

feat(arcgis-rest-request): change getDomainCredentials to handle URLs… #1211

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 7, 2025
Merged

feat(arcgis-rest-request): change getDomainCredentials to handle URLs… #1211

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
013e4da
feat(arcgis-rest-request): change getDomainCredentials to handle URLs…
MikeTschudi Apr 4, 2025
7ae474b
feat(arcgis-rest-request): switched to using toLowerCase
MikeTschudi Apr 7, 2025
65cf186
Merge branch 'main' into 1210-support-domain-case-insensitivity-in-ge…
MikeTschudi Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/arcgis-rest-request/src/ArcGISIdentityManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,14 +1079,14 @@ export class ArcGISIdentityManager

/**
* Returns authentication in a format useable in the [`IdentityManager.registerToken()` method in the ArcGIS API for JavaScript](https://developers.arcgis.com/javascript/latest/api-reference/esri-identity-IdentityManager.html#registerToken).
*
*
* This method can be used with {@linkcode ArcGISIdentityManager.fromCredential} to interop with the ArcGIS API for JavaScript.
*
* ```js
* require(["esri/id"], (esriId) => {
* esriId.registerToken(manager.toCredential());
* })

* ```
*
* @returns ICredential
Expand Down Expand Up @@ -1272,8 +1272,9 @@ export class ArcGISIdentityManager
return "same-origin";
}

url = url.toLocaleLowerCase();
return this.trustedDomains.some((domainWithProtocol) => {
return url.startsWith(domainWithProtocol);
return url.startsWith(domainWithProtocol.toLocaleLowerCase());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using .toLocaleLowerCase() (vs .toLowerCase) intentionally here? I'm wondering what are the potential implications vs using .toLowerCase().

Copy link
Member Author

@MikeTschudi MikeTschudi Apr 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'd read a comment that toLowerCase may not work properly for locales such as Turkish. I'll ask the translation team for advice.

String.prototype.toLocaleLowerCase():

Unlike other methods that use the locales argument, toLocaleLowerCase() does not allow locale matching. Therefore, after checking the validity of the locales argument, toLocaleLowerCase() always uses the first locale in the list (or the default locale if the list is empty), even if this locale is not supported by the implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For domain names (which we are checking here) should it really matter because the character set of so limited I think we can use toLowerCase() here.

})
? "include"
: "same-origin";
Expand Down
130 changes: 130 additions & 0 deletions packages/arcgis-rest-request/test/ArcGISIdentityManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3088,6 +3088,136 @@ describe("ArcGISIdentityManager", () => {
fail(e);
});
});

it("should ignore the case when comparing a server with the authorizedCrossOriginDomains list: variant 1", (done) => {
const session = new ArcGISIdentityManager({
clientId: "id",
token: "token",
refreshToken: "refresh",
tokenExpires: TOMORROW,
portal: "https://gis.city.gov/sharing/rest"
});

fetchMock.postOnce("https://gisservices.city.gov/public/rest/info", {
currentVersion: 10.51,
fullVersion: "10.5.1.120",
owningSystemUrl: "https://gis.city.gov",
authInfo: {
isTokenBasedSecurity: true,
tokenServicesUrl: "https://gis.city.gov/sharing/generateToken"
}
});

fetchMock.getOnce(
"https://gis.city.gov/sharing/rest/portals/self?f=json&token=token",
{
authorizedCrossOriginDomains: ["https://gisservices.city.gov"]
}
);

fetchMock.postOnce("https://gis.city.gov/sharing/rest/info", {
owningSystemUrl: "http://gis.city.gov",
authInfo: {
tokenServicesUrl: "https://gis.city.gov/sharing/generateToken",
isTokenBasedSecurity: true
}
});

fetchMock.postOnce("https://gis.city.gov/sharing/generateToken", {
token: "serverToken",
expires: TOMORROW.getTime()
});

fetchMock.post(
"https://gisservices.city.gov/public/rest/services/trees/FeatureServer/0/query",
{
count: 123
}
);

request(
"https://GISSERVICES.CITY.GOV/public/rest/services/trees/FeatureServer/0/query",
{
authentication: session
}
)
.then((response) => {
const { credentials } = fetchMock.lastOptions(
"https://gisservices.city.gov/public/rest/services/trees/FeatureServer/0/query"
) as RequestInit;
expect(credentials).toEqual("include");

done();
})
.catch((e) => {
fail(e);
});
});

it("should ignore the case when comparing a server with the authorizedCrossOriginDomains list: variant 2", (done) => {
const session = new ArcGISIdentityManager({
clientId: "id",
token: "token",
refreshToken: "refresh",
tokenExpires: TOMORROW,
portal: "https://gis.city.gov/sharing/rest"
});

fetchMock.postOnce("https://gisservices.city.gov/public/rest/info", {
currentVersion: 10.51,
fullVersion: "10.5.1.120",
owningSystemUrl: "https://gis.city.gov",
authInfo: {
isTokenBasedSecurity: true,
tokenServicesUrl: "https://gis.city.gov/sharing/generateToken"
}
});

fetchMock.getOnce(
"https://gis.city.gov/sharing/rest/portals/self?f=json&token=token",
{
authorizedCrossOriginDomains: ["https://GISSERVICES.city.gov"]
}
);

fetchMock.postOnce("https://gis.city.gov/sharing/rest/info", {
owningSystemUrl: "http://gis.city.gov",
authInfo: {
tokenServicesUrl: "https://gis.city.gov/sharing/generateToken",
isTokenBasedSecurity: true
}
});

fetchMock.postOnce("https://gis.city.gov/sharing/generateToken", {
token: "serverToken",
expires: TOMORROW.getTime()
});

fetchMock.post(
"https://gisservices.city.gov/public/rest/services/trees/FeatureServer/0/query",
{
count: 123
}
);

request(
"https://gisservices.city.gov/public/rest/services/trees/FeatureServer/0/query",
{
authentication: session
}
)
.then((response) => {
const { credentials } = fetchMock.lastOptions(
"https://gisservices.city.gov/public/rest/services/trees/FeatureServer/0/query"
) as RequestInit;
expect(credentials).toEqual("include");

done();
})
.catch((e) => {
fail(e);
});
});
});

it("should still send same-origin credentials even if another domain is listed in authorizedCrossOriginDomains", (done) => {
Expand Down