Skip to content

Commit

Permalink
Merge pull request #782 from Esri/f/credential-based-on-header
Browse files Browse the repository at this point in the history
fix: ensure platformSelf sends cookie
  • Loading branch information
dbouwman committed Nov 30, 2020
2 parents a74975d + 29e33cb commit 6fd5f34
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 139 deletions.
42 changes: 21 additions & 21 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions packages/arcgis-rest-auth/src/app-tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ export function platformSelf(
"X-Esri-Auth-Client-Id": clientId,
"X-Esri-Auth-Redirect-Uri": redirectUri,
},
// We need to ensure the cookies are sent as that's
// what the API uses to convert things
credentials: "include",
// Note: request has logic to include the cookie
// for platformSelf calls w/ the X-Esri-Auth-Client-Id header
params: {
f: "json",
},
Expand Down
4 changes: 4 additions & 0 deletions packages/arcgis-rest-auth/test/app-tokens.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ describe("app-token functions: ", () => {
expect(headers["X-Esri-Auth-Redirect-Uri"]).toBe(
"https://hub.arcgis.com/torii-provider-arcgis/redirect.html"
);
expect(options.credentials).toBe(
"include",
"fetch should send cookie"
);
expect(headers["X-Esri-Auth-Client-Id"]).toBe("CLIENT-ID-ABC123");
expect(response.token).toEqual("APP-TOKEN");
expect(response.username).toEqual("jsmith");
Expand Down
60 changes: 37 additions & 23 deletions packages/arcgis-rest-request/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export const NODEJS_DEFAULT_REFERER_HEADER = `@esri/arcgis-rest-js`;
let DEFAULT_ARCGIS_REQUEST_OPTIONS: IRequestOptions = {
httpMethod: "POST",
params: {
f: "json"
}
f: "json",
},
};

/**
Expand Down Expand Up @@ -74,19 +74,19 @@ export class ArcGISAuthError extends ArcGISRequestError {

const retryRequest = (resolve: any, reject: any) => {
getSession(this.url, this.options)
.then(session => {
.then((session) => {
const newOptions = {
...this.options,
...{ authentication: session }
...{ authentication: session },
};

tries = tries + 1;
return request(this.url, newOptions);
})
.then(response => {
.then((response) => {
resolve(response);
})
.catch(e => {
.catch((e) => {
if (e.name === "ArcGISAuthError" && tries < retryLimit) {
retryRequest(resolve, reject);
} else if (e.name === "ArcGISAuthError" && tries >= retryLimit) {
Expand Down Expand Up @@ -197,13 +197,13 @@ export function request(
...{
params: {
...DEFAULT_ARCGIS_REQUEST_OPTIONS.params,
...requestOptions.params
...requestOptions.params,
},
headers: {
...DEFAULT_ARCGIS_REQUEST_OPTIONS.headers,
...requestOptions.headers
}
}
...requestOptions.headers,
},
},
};

const missingGlobals: string[] = [];
Expand Down Expand Up @@ -245,7 +245,7 @@ export function request(

const params: IParams = {
...{ f: "json" },
...options.params
...options.params,
};

let originalAuthError: ArcGISAuthError = null;
Expand All @@ -254,11 +254,22 @@ export function request(
method: httpMethod,
/* ensures behavior mimics XMLHttpRequest.
needed to support sending IWA cookies */
credentials: "same-origin"
credentials: "same-origin",
};

// the /oauth2/platformSelf route will add X-Esri-Auth-Client-Id header
// and that request needs to send cookies cross domain
// so we need to set the credentials to "include"
if (
options.headers &&
options.headers["X-Esri-Auth-Client-Id"] &&
url.indexOf("/oauth2/platformSelf") > -1
) {
fetchOptions.credentials = "include";
}

return (authentication
? authentication.getToken(url, { fetch: options.fetch }).catch(err => {
? authentication.getToken(url, { fetch: options.fetch }).catch((err) => {
/**
* append original request url and requestOptions
* to the error thrown by getToken()
Expand All @@ -276,7 +287,7 @@ export function request(
})
: Promise.resolve("")
)
.then(token => {
.then((token) => {
if (token.length) {
params.token = token;
}
Expand All @@ -285,14 +296,17 @@ export function request(
const requestHeaders: {
[key: string]: any;
} = {};

if (fetchOptions.method === "GET") {
// Prevents token from being passed in query params when hideToken option is used.
/* istanbul ignore if - window is always defined in a browser. Test case is covered by Jasmine in node test */
if (params.token && options.hideToken &&
if (
params.token &&
options.hideToken &&
// Sharing API does not support preflight check required by modern browsers https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
typeof window === 'undefined') {
requestHeaders["X-Esri-Authorization"] = `Bearer ${params.token}`
typeof window === "undefined"
) {
requestHeaders["X-Esri-Authorization"] = `Bearer ${params.token}`;
delete params.token;
}
// encode the parameters into the query string
Expand All @@ -304,7 +318,7 @@ export function request(
if (
// This would exceed the maximum length for URLs specified by the consumer and requires POST
(options.maxUrlLength &&
urlWithQueryString.length > options.maxUrlLength) ||
urlWithQueryString.length > options.maxUrlLength) ||
// Or if the customer requires the token to be hidden and it has not already been hidden in the header (for browsers)
(params.token && options.hideToken)
) {
Expand Down Expand Up @@ -336,7 +350,7 @@ export function request(
// Mixin headers from request options
fetchOptions.headers = {
...requestHeaders,
...options.headers
...options.headers,
};

/* istanbul ignore next - karma reports coverage on browser tests only */
Expand All @@ -352,7 +366,7 @@ export function request(

return options.fetch(url, fetchOptions);
})
.then(response => {
.then((response) => {
if (!response.ok) {
// server responded w/ an actual error (404, 500, etc)
const { status, statusText } = response;
Expand Down Expand Up @@ -381,7 +395,7 @@ export function request(
return response.blob();
}
})
.then(data => {
.then((data) => {
if ((params.f === "json" || params.f === "geojson") && !rawResponse) {
const response = checkForErrors(
data,
Expand All @@ -401,7 +415,7 @@ export function request(
(options.authentication as any).trustedServers[truncatedUrl] = {
token: [],
// default to 24 hours
expires: new Date(Date.now() + 86400 * 1000)
expires: new Date(Date.now() + 86400 * 1000),
};
originalAuthError = null;
}
Expand Down
Loading

0 comments on commit 6fd5f34

Please sign in to comment.