From ede6f8f8597833053b0b5d62ca1e33687c350dd4 Mon Sep 17 00:00:00 2001 From: Gavin Rehkemper Date: Tue, 18 May 2021 16:53:16 -0500 Subject: [PATCH 1/5] fix(UserSession): switch "duration" to "expiration" in IOAuth2Options AFFECTS PACKAGES: @esri/arcgis-rest-auth ISSUES CLOSED: #843 --- packages/arcgis-rest-auth/src/UserSession.ts | 33 +++++-- .../arcgis-rest-auth/test/UserSession.test.ts | 92 +++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index 2ce722a12b..05e2d3a9b4 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -102,10 +102,17 @@ export interface IOAuth2Options { provider?: AuthenticationProvider; + /** + * The requested validity in minutes for a token. Defaults to 20160 (two weeks). + */ + expiration?: number; + /** * Duration (in minutes) that a token will be valid. Defaults to 20160 (two weeks). + * + * @deprecated */ - duration?: number; + duration?: number; /** * Determines whether to open the authorization window in a new tab/window or in the current window. @@ -297,11 +304,17 @@ export class UserSession implements IAuthenticationManager { */ /* istanbul ignore next */ public static beginOAuth2(options: IOAuth2Options, win: any = window) { + + if(options.duration) { + console.log("DEPRECATED: 'duration' is deprecated - use 'expiration' instead"); + options.expiration = options.duration; + } + const { portal, provider, clientId, - duration, + expiration, redirectUri, popup, popupWindowFeatures, @@ -312,7 +325,7 @@ export class UserSession implements IAuthenticationManager { ...{ portal: "https://www.arcgis.com/sharing/rest", provider: "arcgis", - duration: 20160, + expiration: 20160, popup: true, popupWindowFeatures: "height=400,width=600,menubar=no,location=yes,resizable=yes,scrollbars=yes,status=yes", @@ -323,11 +336,11 @@ export class UserSession implements IAuthenticationManager { }; let url: string; if (provider === "arcgis") { - url = `${portal}/oauth2/authorize?client_id=${clientId}&response_type=token&expiration=${duration}&redirect_uri=${encodeURIComponent( + url = `${portal}/oauth2/authorize?client_id=${clientId}&response_type=token&expiration=${expiration}&redirect_uri=${encodeURIComponent( redirectUri )}&state=${state}&locale=${locale}`; } else { - url = `${portal}/oauth2/social/authorize?client_id=${clientId}&socialLoginProviderName=${provider}&autoAccountCreateForSocial=true&response_type=token&expiration=${duration}&redirect_uri=${encodeURIComponent( + url = `${portal}/oauth2/social/authorize?client_id=${clientId}&socialLoginProviderName=${provider}&autoAccountCreateForSocial=true&response_type=token&expiration=${expiration}&redirect_uri=${encodeURIComponent( redirectUri )}&state=${state}&locale=${locale}`; } @@ -528,13 +541,17 @@ export class UserSession implements IAuthenticationManager { options: IOAuth2Options, response: http.ServerResponse ) { - const { portal, clientId, duration, redirectUri }: IOAuth2Options = { - ...{ portal: "https://arcgis.com/sharing/rest", duration: 20160 }, + if(options.duration) { + console.log("DEPRECATED: 'duration' is deprecated - use 'expiration' instead"); + options.expiration = options.duration; + } + const { portal, clientId, expiration, redirectUri }: IOAuth2Options = { + ...{ portal: "https://arcgis.com/sharing/rest", expiration: 20160 }, ...options, }; response.writeHead(301, { - Location: `${portal}/oauth2/authorize?client_id=${clientId}&expiration=${duration}&response_type=code&redirect_uri=${encodeURIComponent( + Location: `${portal}/oauth2/authorize?client_id=${clientId}&expiration=${expiration}&response_type=code&redirect_uri=${encodeURIComponent( redirectUri )}`, }); diff --git a/packages/arcgis-rest-auth/test/UserSession.test.ts b/packages/arcgis-rest-auth/test/UserSession.test.ts index 78c3237c19..5a82c8b3fd 100644 --- a/packages/arcgis-rest-auth/test/UserSession.test.ts +++ b/packages/arcgis-rest-auth/test/UserSession.test.ts @@ -967,6 +967,52 @@ describe("UserSession", () => { "https://www.arcgis.com/sharing/rest/oauth2/social/authorize?client_id=clientId123&socialLoginProviderName=google&autoAccountCreateForSocial=true&response_type=token&expiration=20160&redirect_uri=http%3A%2F%2Fexample-app.com%2Fredirect&state=clientId123&locale=" ); }); + + it("should pass custom expiration", () => { + const MockWindow: any = { + location: { + href: "", + }, + }; + + // https://github.com/palantir/tslint/issues/3056 + void UserSession.beginOAuth2( + { + clientId: "clientId123", + redirectUri: "http://example-app.com/redirect", + popup: false, + expiration: 9000 + }, + MockWindow + ); + + expect(MockWindow.location.href).toBe( + "https://www.arcgis.com/sharing/rest/oauth2/authorize?client_id=clientId123&response_type=token&expiration=9000&redirect_uri=http%3A%2F%2Fexample-app.com%2Fredirect&state=clientId123&locale=" + ); + }); + + it("should pass custom duration (DEPRECATED)", () => { + const MockWindow: any = { + location: { + href: "", + }, + }; + + // https://github.com/palantir/tslint/issues/3056 + void UserSession.beginOAuth2( + { + clientId: "clientId123", + redirectUri: "http://example-app.com/redirect", + popup: false, + duration: 9001 + }, + MockWindow + ); + + expect(MockWindow.location.href).toBe( + "https://www.arcgis.com/sharing/rest/oauth2/authorize?client_id=clientId123&response_type=token&expiration=9001&redirect_uri=http%3A%2F%2Fexample-app.com%2Fredirect&state=clientId123&locale=" + ); + }); }); describe(".completeOAuth2()", () => { @@ -1488,6 +1534,52 @@ describe("UserSession", () => { MockResponse ); }); + + it("should redirect the request to the authorization page with custom expiration", (done) => { + const spy = jasmine.createSpy("spy"); + const MockResponse: any = { + writeHead: spy, + end() { + expect(spy.calls.mostRecent().args[0]).toBe(301); + expect(spy.calls.mostRecent().args[1].Location).toBe( + "https://arcgis.com/sharing/rest/oauth2/authorize?client_id=clientId&expiration=10000&response_type=code&redirect_uri=https%3A%2F%2Fexample-app.com%2Fredirect-uri" + ); + done(); + }, + }; + + UserSession.authorize( + { + clientId: "clientId", + redirectUri: "https://example-app.com/redirect-uri", + expiration: 10000 + }, + MockResponse + ); + }); + + it("should redirect the request to the authorization page with custom duration (DEPRECATED)", (done) => { + const spy = jasmine.createSpy("spy"); + const MockResponse: any = { + writeHead: spy, + end() { + expect(spy.calls.mostRecent().args[0]).toBe(301); + expect(spy.calls.mostRecent().args[1].Location).toBe( + "https://arcgis.com/sharing/rest/oauth2/authorize?client_id=clientId&expiration=10001&response_type=code&redirect_uri=https%3A%2F%2Fexample-app.com%2Fredirect-uri" + ); + done(); + }, + }; + + UserSession.authorize( + { + clientId: "clientId", + redirectUri: "https://example-app.com/redirect-uri", + duration: 10001 + }, + MockResponse + ); + }); }); describe(".exchangeAuthorizationCode()", () => { From aa02b7ec98c10f1f96b24541e5e075616db462a3 Mon Sep 17 00:00:00 2001 From: Gavin Rehkemper Date: Wed, 19 May 2021 09:50:31 -0500 Subject: [PATCH 2/5] deprecated tag - only warn since we're deprecating things, we want to allow the builds but warn. --- tslint.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tslint.json b/tslint.json index 3f76dccf8a..8cb0cf5260 100644 --- a/tslint.json +++ b/tslint.json @@ -11,6 +11,9 @@ "object-literal-sort-keys": false, "interface-name": [true, "always-prefix"], "no-string-literal": false, - "no-console": false + "no-console": false, + "deprecation": { + "severity": "warning" + } } } From 799b6663c74a537bb934365cefa64f3d87549362 Mon Sep 17 00:00:00 2001 From: Gavin Rehkemper Date: Wed, 19 May 2021 18:46:21 -0500 Subject: [PATCH 3/5] Update packages/arcgis-rest-auth/src/UserSession.ts Co-authored-by: Tom Wayson --- packages/arcgis-rest-auth/src/UserSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index 05e2d3a9b4..4ca791f684 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -110,7 +110,7 @@ export interface IOAuth2Options { /** * Duration (in minutes) that a token will be valid. Defaults to 20160 (two weeks). * - * @deprecated + * @deprecated use 'expiration' instead */ duration?: number; From 0a3c828718249f115fd0d13a855265729849c24a Mon Sep 17 00:00:00 2001 From: Gavin Rehkemper Date: Wed, 19 May 2021 19:05:16 -0500 Subject: [PATCH 4/5] do not mutate options --- packages/arcgis-rest-auth/src/UserSession.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index 4ca791f684..ecd791a7c3 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -307,7 +307,6 @@ export class UserSession implements IAuthenticationManager { if(options.duration) { console.log("DEPRECATED: 'duration' is deprecated - use 'expiration' instead"); - options.expiration = options.duration; } const { @@ -336,7 +335,7 @@ export class UserSession implements IAuthenticationManager { }; let url: string; if (provider === "arcgis") { - url = `${portal}/oauth2/authorize?client_id=${clientId}&response_type=token&expiration=${expiration}&redirect_uri=${encodeURIComponent( + url = `${portal}/oauth2/authorize?client_id=${clientId}&response_type=token&expiration=${options.duration || expiration}&redirect_uri=${encodeURIComponent( redirectUri )}&state=${state}&locale=${locale}`; } else { @@ -543,7 +542,6 @@ export class UserSession implements IAuthenticationManager { ) { if(options.duration) { console.log("DEPRECATED: 'duration' is deprecated - use 'expiration' instead"); - options.expiration = options.duration; } const { portal, clientId, expiration, redirectUri }: IOAuth2Options = { ...{ portal: "https://arcgis.com/sharing/rest", expiration: 20160 }, @@ -551,7 +549,7 @@ export class UserSession implements IAuthenticationManager { }; response.writeHead(301, { - Location: `${portal}/oauth2/authorize?client_id=${clientId}&expiration=${expiration}&response_type=code&redirect_uri=${encodeURIComponent( + Location: `${portal}/oauth2/authorize?client_id=${clientId}&expiration=${options.duration || expiration}&response_type=code&redirect_uri=${encodeURIComponent( redirectUri )}`, }); From 50e47d80337fb52604240056883c6f09afb67989 Mon Sep 17 00:00:00 2001 From: Gavin Rehkemper Date: Wed, 19 May 2021 19:07:04 -0500 Subject: [PATCH 5/5] missed options.duration --- packages/arcgis-rest-auth/src/UserSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index ecd791a7c3..a7d351423c 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -339,7 +339,7 @@ export class UserSession implements IAuthenticationManager { redirectUri )}&state=${state}&locale=${locale}`; } else { - url = `${portal}/oauth2/social/authorize?client_id=${clientId}&socialLoginProviderName=${provider}&autoAccountCreateForSocial=true&response_type=token&expiration=${expiration}&redirect_uri=${encodeURIComponent( + url = `${portal}/oauth2/social/authorize?client_id=${clientId}&socialLoginProviderName=${provider}&autoAccountCreateForSocial=true&response_type=token&expiration=${options.duration || expiration}&redirect_uri=${encodeURIComponent( redirectUri )}&state=${state}&locale=${locale}`; }