Skip to content

Commit

Permalink
Merge pull request #697 from ssylvia/secure-token
Browse files Browse the repository at this point in the history
Secure token in POST body or auth header
  • Loading branch information
tomwayson authored May 7, 2020
2 parents 2229f7b + bc13405 commit 8595fab
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 11 deletions.
41 changes: 30 additions & 11 deletions package-lock.json

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

17 changes: 17 additions & 0 deletions packages/arcgis-rest-request/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,17 @@ export function request(
params.token = token;
}

// Custom headers to add to request. IRequestOptions.headers with merge over requestHeaders.
const requestHeaders: {
[key: string]: any;
} = {};

if (fetchOptions.method === "GET") {
// Prevents token from being passed in query params when hideToken option is used.
if (params.token && options.hideToken) {
requestHeaders["X-Esri-Authorization"] = `Bearer ${params.token}`
delete params.token;
}
// encode the parameters into the query string
const queryParams = encodeQueryString(params);
// dont append a '?' unless parameters are actually present
Expand All @@ -295,6 +305,12 @@ export function request(
// the consumer specified a maximum length for URLs
// and this would exceed it, so use post instead
fetchOptions.method = "POST";

// Add token back to body with other params instead of header
if (token.length && options.hideToken) {
params.token = token;
delete requestHeaders["X-Esri-Authorization"];
}
} else {
// just use GET
url = urlWithQueryString;
Expand All @@ -312,6 +328,7 @@ export function request(

// Mixin headers from request options
fetchOptions.headers = {
...requestHeaders,
...options.headers
};

Expand Down
5 changes: 5 additions & 0 deletions packages/arcgis-rest-request/src/utils/IRequestOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ export interface IRequestOptions {
* The instance of `IAuthenticationManager` to use to authenticate this request.
*/
authentication?: IAuthenticationManager;
/**
* Prevents the token from being passed in a URL Query param that is saved in browser history.
* Instead, the token will be passed in POST request body or through X-Esri-Authorization header
*/
hideToken?: boolean;
/**
* Base url for the portal you want to make the request to. Defaults to 'https://www.arcgis.com/sharing/rest'.
*/
Expand Down
63 changes: 63 additions & 0 deletions packages/arcgis-rest-request/test/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,69 @@ describe("request()", () => {
});
});

it("should use the `authentication` option to authenticate a request and use `X-ESRI_AUTHORIZATION` header", done => {
fetchMock.once("*", SharingRestInfo);

const MOCK_AUTH = {
portal: "https://www.arcgis.com/sharing/rest",
getToken() {
return Promise.resolve("token");
}
};

request("https://www.arcgis.com/sharing/rest/info", {
authentication: MOCK_AUTH,
httpMethod: 'GET',
hideToken: true
})
.then(response => {
const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
expect(url).toEqual("https://www.arcgis.com/sharing/rest/info?f=json");
expect(options.method).toBe("GET");
expect(response).toEqual(SharingRestInfo);
expect((options.headers as any)["X-Esri-Authorization"]).toBe("Bearer token");
done();
})
.catch(e => {
fail(e);
});
});

it("should switch from GET to POST when url is longer than specified and replace token in header with token in POST body", done => {
fetchMock.once("*", SharingRestInfo);
const restInfoUrl = "https://www.arcgis.com/sharing/rest/info";

const MOCK_AUTH = {
portal: "https://www.arcgis.com/sharing/rest",
getToken() {
return Promise.resolve("token");
}
};

request(restInfoUrl, {
authentication: MOCK_AUTH,
httpMethod: "GET",
hideToken: true,
// typically consumers would base maxUrlLength on browser/server limits
// but for testing, we use an artificially low limit
// like this one that assumes no parameters will be added
maxUrlLength: restInfoUrl.length
})
.then(response => {
const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
expect(url).toEqual("https://www.arcgis.com/sharing/rest/info");
expect(options.method).toBe("POST");
expect(options.body).toContain("f=json");
expect(options.body).toContain("token=token");
expect((options.headers as any)["X-Esri-Authorization"]).toBe(undefined);
expect(response).toEqual(SharingRestInfo);
done();
})
.catch(e => {
fail(e);
});
});

it("should re-throw HTTP errors (404, 500, etc)", done => {
fetchMock.once("*", 404);

Expand Down

0 comments on commit 8595fab

Please sign in to comment.