From a350f766dcd8e04d34b5c5cc662e16e38947e407 Mon Sep 17 00:00:00 2001 From: skitterm Date: Wed, 19 Sep 2018 17:07:47 -0700 Subject: [PATCH 1/5] #295: SSL property added to UserSession class. SSL calculated in completeOAuth2 and used in to/fromCredential. --- packages/arcgis-rest-auth/src/UserSession.ts | 22 ++++++++++++++++++- packages/arcgis-rest-auth/src/fetch-token.ts | 2 ++ .../arcgis-rest-auth/test/UserSession.test.ts | 19 +++++++++++----- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index c302b70630..b3cf580d97 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -165,6 +165,11 @@ export interface IUserSessionOptions { */ portal?: string; + /** + * Whether requests should be made exlusively over HTTPS. + */ + ssl?: boolean; + /** * ArcGIS Authentication is used by default. Specifying an alternative will take users directly to the corresponding provider's OAuth page. */ @@ -215,6 +220,11 @@ export class UserSession implements IAuthenticationManager { */ readonly portal: string; + /** + * Whether requests should be made exlusively over HTTPS. + */ + readonly ssl: boolean; + /** * The authentication provider to use. */ @@ -301,6 +311,7 @@ export class UserSession implements IAuthenticationManager { this._token = options.token; this._tokenExpires = options.tokenExpires; this.portal = options.portal || "https://www.arcgis.com/sharing/rest"; + this.ssl = options.ssl; this.provider = options.provider || "arcgis"; this.tokenDuration = options.tokenDuration || 20160; this.redirectUri = options.redirectUri; @@ -373,6 +384,7 @@ export class UserSession implements IAuthenticationManager { new UserSession({ clientId, portal, + ssl: oauthInfo.ssl, token: oauthInfo.token, tokenExpires: new Date(oauthInfo.expires), username: oauthInfo.username @@ -430,6 +442,7 @@ export class UserSession implements IAuthenticationManager { return new UserSession({ clientId, portal, + ssl: oauthInfo.ssl, token: oauthInfo.token, tokenExpires: oauthInfo.expires, username: oauthInfo.username @@ -456,10 +469,14 @@ export class UserSession implements IAuthenticationManager { Date.now() + parseInt(match[2], 10) * 1000 - 60 * 1000 ); const username = decodeURIComponent(match[3]); + const ssl = + win.location.href.indexOf("&ssl=true") !== -1 || + win.location.href.indexOf("#ssl=true") !== -1; return completeSignIn(undefined, { token, expires, + ssl, username }); } @@ -536,6 +553,7 @@ export class UserSession implements IAuthenticationManager { token: options.token, tokenExpires: new Date(options.tokenExpires), portal: options.portal, + ssl: options.ssl, tokenDuration: options.tokenDuration, redirectUri: options.redirectUri, refreshTokenTTL: options.refreshTokenTTL @@ -557,6 +575,7 @@ export class UserSession implements IAuthenticationManager { static fromCredential(credential: ICredential) { return new UserSession({ portal: credential.server + `/sharing/rest`, + ssl: credential.ssl, token: credential.token, username: credential.userId, tokenExpires: new Date(credential.expires) @@ -576,7 +595,7 @@ export class UserSession implements IAuthenticationManager { return { expires: this.tokenExpires.getTime(), server: this.portal, - ssl: true, + ssl: this.ssl, token: this.token, userId: this.username }; @@ -644,6 +663,7 @@ export class UserSession implements IAuthenticationManager { token: this.token, tokenExpires: this.tokenExpires, portal: this.portal, + ssl: this.ssl, tokenDuration: this.tokenDuration, redirectUri: this.redirectUri, refreshTokenTTL: this.refreshTokenTTL diff --git a/packages/arcgis-rest-auth/src/fetch-token.ts b/packages/arcgis-rest-auth/src/fetch-token.ts index da1bef7ef0..37f3807c63 100644 --- a/packages/arcgis-rest-auth/src/fetch-token.ts +++ b/packages/arcgis-rest-auth/src/fetch-token.ts @@ -13,6 +13,7 @@ interface IFetchTokenRawResponse { expires_in: number; username: string; refresh_token?: string; + ssl?: boolean; } export interface IFetchTokenResponse { @@ -20,6 +21,7 @@ export interface IFetchTokenResponse { expires: Date; username: string; refreshToken?: string; + ssl?: boolean; } export function fetchToken( diff --git a/packages/arcgis-rest-auth/test/UserSession.test.ts b/packages/arcgis-rest-auth/test/UserSession.test.ts index dd219271aa..27ae7fbb2f 100644 --- a/packages/arcgis-rest-auth/test/UserSession.test.ts +++ b/packages/arcgis-rest-auth/test/UserSession.test.ts @@ -19,6 +19,7 @@ describe("UserSession", () => { const session = new UserSession({ clientId: "clientId", redirectUri: "https://example-app.com/redirect-uri", + ssl: false, token: "token", tokenExpires: TOMORROW, refreshToken: "refreshToken", @@ -34,6 +35,7 @@ describe("UserSession", () => { expect(session2.redirectUri).toEqual( "https://example-app.com/redirect-uri" ); + expect(session2.ssl).toEqual(false); expect(session2.token).toEqual("token"); expect(session2.tokenExpires).toEqual(TOMORROW); expect(session2.refreshToken).toEqual("refreshToken"); @@ -525,6 +527,7 @@ describe("UserSession", () => { .then(session => { expect(session.token).toBe("token"); expect(session.username).toBe("c@sey"); + expect(session.ssl).toBe(true); expect(session.tokenExpires).toEqual(TOMORROW); done(); }) @@ -543,7 +546,8 @@ describe("UserSession", () => { JSON.stringify({ token: "token", expires: TOMORROW, - username: "c@sey" + username: "c@sey", + ssl: true }) ); }); @@ -652,7 +656,7 @@ describe("UserSession", () => { const MockWindow = { location: { href: - "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&persist=true" + "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true&persist=true" }, get parent() { return this; @@ -670,6 +674,7 @@ describe("UserSession", () => { expect(session.token).toBe("token"); expect(session.tokenExpires.getTime()).toBeGreaterThan(Date.now()); expect(session.username).toBe("c@sey"); + expect(session.ssl).toBe(true); }); it("should callback to create a new user session if finds a valid opener", done => { @@ -683,6 +688,7 @@ describe("UserSession", () => { const oauthInfo = JSON.parse(oauthInfoString); expect(oauthInfo.token).toBe("token"); expect(oauthInfo.username).toBe("c@sey"); + expect(oauthInfo.ssl).toBe(false); expect(new Date(oauthInfo.expires).getTime()).toBeGreaterThan( Date.now() ); @@ -717,6 +723,7 @@ describe("UserSession", () => { const oauthInfo = JSON.parse(oauthInfoString); expect(oauthInfo.token).toBe("token"); expect(oauthInfo.username).toBe("c@sey"); + expect(oauthInfo.ssl).toBe(true); expect(new Date(oauthInfo.expires).getTime()).toBeGreaterThan( Date.now() ); @@ -727,7 +734,7 @@ describe("UserSession", () => { }, location: { href: - "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey" + "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true" } }; @@ -872,7 +879,7 @@ describe("UserSession", () => { const MOCK_CREDENTIAL: ICredential = { expires: TOMORROW.getTime(), server: "https://www.arcgis.com", - ssl: true, + ssl: false, token: "token", userId: "jsmith" }; @@ -882,6 +889,7 @@ describe("UserSession", () => { clientId: "clientId", redirectUri: "https://example-app.com/redirect-uri", token: "token", + ssl: false, tokenExpires: TOMORROW, refreshToken: "refreshToken", refreshTokenExpires: TOMORROW, @@ -893,7 +901,7 @@ describe("UserSession", () => { const creds = session.toCredential(); expect(creds.userId).toEqual("jsmith"); expect(creds.server).toEqual("https://www.arcgis.com/sharing/rest"); - expect(creds.ssl).toEqual(true); + expect(creds.ssl).toEqual(false); expect(creds.token).toEqual("token"); expect(creds.expires).toEqual(TOMORROW.getTime()); }); @@ -902,6 +910,7 @@ describe("UserSession", () => { const session = UserSession.fromCredential(MOCK_CREDENTIAL); expect(session.username).toEqual("jsmith"); expect(session.portal).toEqual("https://www.arcgis.com/sharing/rest"); + expect(session.ssl).toEqual(false); expect(session.token).toEqual("token"); expect(session.tokenExpires).toEqual(new Date(TOMORROW)); }); From 1dba15d9345131b114f2c2d9c1fa29f8a928c154 Mon Sep 17 00:00:00 2001 From: skitterm Date: Thu, 20 Sep 2018 10:22:59 -0700 Subject: [PATCH 2/5] #295: fetchToken returns ssl property with response. --- packages/arcgis-rest-auth/src/UserSession.ts | 1 + packages/arcgis-rest-auth/src/fetch-token.ts | 7 ++++--- packages/arcgis-rest-auth/test/UserSession.test.ts | 8 +++++++- packages/arcgis-rest-auth/test/fetchToken.test.ts | 8 ++++++-- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index cb3d7adc23..f054435058 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -529,6 +529,7 @@ export class UserSession implements IAuthenticationManager { return new UserSession({ clientId, portal, + ssl: response.ssl, redirectUri, refreshToken: response.refreshToken, refreshTokenTTL, diff --git a/packages/arcgis-rest-auth/src/fetch-token.ts b/packages/arcgis-rest-auth/src/fetch-token.ts index 37f3807c63..e39fcb1b30 100644 --- a/packages/arcgis-rest-auth/src/fetch-token.ts +++ b/packages/arcgis-rest-auth/src/fetch-token.ts @@ -12,16 +12,16 @@ interface IFetchTokenRawResponse { access_token: string; expires_in: number; username: string; + ssl: boolean; refresh_token?: string; - ssl?: boolean; } export interface IFetchTokenResponse { token: string; expires: Date; username: string; + ssl: boolean; refreshToken?: string; - ssl?: boolean; } export function fetchToken( @@ -40,7 +40,8 @@ export function fetchToken( username: response.username, expires: new Date( Date.now() + (response.expires_in * 60 * 1000 - 60 * 1000) - ) + ), + ssl: response.ssl === true }; if (response.refresh_token) { r.refreshToken = response.refresh_token; diff --git a/packages/arcgis-rest-auth/test/UserSession.test.ts b/packages/arcgis-rest-auth/test/UserSession.test.ts index bd9f4f31c4..5e801f5341 100644 --- a/packages/arcgis-rest-auth/test/UserSession.test.ts +++ b/packages/arcgis-rest-auth/test/UserSession.test.ts @@ -861,7 +861,8 @@ describe("UserSession", () => { access_token: "token", expires_in: 1800, refresh_token: "refreshToken", - username: "Casey" + username: "Casey", + ssl: true }); UserSession.exchangeAuthorizationCode( @@ -872,6 +873,11 @@ describe("UserSession", () => { "code" ) .then(session => { + expect(session.token).toBe("token"); + expect(session.tokenExpires.getTime()).toBeGreaterThan(Date.now()); + expect(session.username).toBe("Casey"); + expect(session.refreshToken).toBe("refreshToken"); + expect(session.ssl).toBe(true); done(); }) .catch(e => { diff --git a/packages/arcgis-rest-auth/test/fetchToken.test.ts b/packages/arcgis-rest-auth/test/fetchToken.test.ts index 7defc34937..3e0af4d179 100644 --- a/packages/arcgis-rest-auth/test/fetchToken.test.ts +++ b/packages/arcgis-rest-auth/test/fetchToken.test.ts @@ -12,7 +12,8 @@ describe("fetchToken()", () => { it("should request a token with `client_credentials`, `client_id` and `client_secret`", done => { fetchMock.postOnce(TOKEN_URL, { access_token: "token", - expires_in: 1800 + expires_in: 1800, + ssl: false }); fetchToken(TOKEN_URL, { @@ -33,6 +34,7 @@ describe("fetchToken()", () => { expect(options.body).toContain("grant_type=client_credentials"); expect(response.token).toEqual("token"); expect(response.expires).toBeGreaterThan(Date.now()); + expect(response.ssl).toEqual(false); done(); }) .catch(e => { @@ -45,7 +47,8 @@ describe("fetchToken()", () => { access_token: "token", expires_in: 1800, refresh_token: "refreshToken", - username: "Casey" + username: "Casey", + ssl: true }); fetchToken(TOKEN_URL, { @@ -74,6 +77,7 @@ describe("fetchToken()", () => { expect(response.refreshToken).toEqual("refreshToken"); expect(response.username).toEqual("Casey"); expect(response.expires).toBeGreaterThan(Date.now()); + expect(response.ssl).toEqual(true); done(); }) .catch(e => { From eb96e2b89f3f3f32cf967c274af18ae026d247e2 Mon Sep 17 00:00:00 2001 From: skitterm Date: Thu, 20 Sep 2018 11:01:15 -0700 Subject: [PATCH 3/5] #295: Syntax fixes. --- packages/arcgis-rest-auth/src/UserSession.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index f054435058..9e88727a90 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -166,7 +166,7 @@ export interface IUserSessionOptions { portal?: string; /** - * Whether requests should be made exlusively over HTTPS. + * Whether requests should be made exclusively over HTTPS. */ ssl?: boolean; @@ -221,7 +221,7 @@ export class UserSession implements IAuthenticationManager { readonly portal: string; /** - * Whether requests should be made exlusively over HTTPS. + * Whether requests should be made exclusively over HTTPS. */ readonly ssl: boolean; @@ -470,8 +470,8 @@ export class UserSession implements IAuthenticationManager { ); const username = decodeURIComponent(match[3]); const ssl = - win.location.href.indexOf("&ssl=true") !== -1 || - win.location.href.indexOf("#ssl=true") !== -1; + win.location.href.indexOf("&ssl=true") > -1 || + win.location.href.indexOf("#ssl=true") > -1; return completeSignIn(undefined, { token, From 584b0e54bec7d19361d2568cf47763a7b8c0fec5 Mon Sep 17 00:00:00 2001 From: skitterm Date: Thu, 20 Sep 2018 11:17:32 -0700 Subject: [PATCH 4/5] #295: Test cases added for when ssl property/parameter is not returned or present. --- packages/arcgis-rest-auth/src/fetch-token.ts | 2 +- .../arcgis-rest-auth/test/UserSession.test.ts | 25 ++++++++++++++-- .../arcgis-rest-auth/test/fetchToken.test.ts | 29 +++++++++++++++++-- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/packages/arcgis-rest-auth/src/fetch-token.ts b/packages/arcgis-rest-auth/src/fetch-token.ts index e39fcb1b30..73e8e30f0c 100644 --- a/packages/arcgis-rest-auth/src/fetch-token.ts +++ b/packages/arcgis-rest-auth/src/fetch-token.ts @@ -12,7 +12,7 @@ interface IFetchTokenRawResponse { access_token: string; expires_in: number; username: string; - ssl: boolean; + ssl?: boolean; refresh_token?: string; } diff --git a/packages/arcgis-rest-auth/test/UserSession.test.ts b/packages/arcgis-rest-auth/test/UserSession.test.ts index 5e801f5341..6a0eaccafa 100644 --- a/packages/arcgis-rest-auth/test/UserSession.test.ts +++ b/packages/arcgis-rest-auth/test/UserSession.test.ts @@ -728,6 +728,27 @@ describe("UserSession", () => { expect(session.ssl).toBe(true); }); + it("should return a new user session with ssl as false when callback hash does not have ssl parameter", () => { + const MockWindow = { + location: { + href: + "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&persist=true" + }, + get parent() { + return this; + } + }; + + const session = UserSession.completeOAuth2( + { + clientId: "clientId", + redirectUri: "https://example-app.com/redirect-uri" + }, + MockWindow + ); + expect(session.ssl).toBe(false); + }); + it("should callback to create a new user session if finds a valid opener", done => { const MockWindow = { opener: { @@ -739,7 +760,7 @@ describe("UserSession", () => { const oauthInfo = JSON.parse(oauthInfoString); expect(oauthInfo.token).toBe("token"); expect(oauthInfo.username).toBe("c@sey"); - expect(oauthInfo.ssl).toBe(false); + expect(oauthInfo.ssl).toBe(true); expect(new Date(oauthInfo.expires).getTime()).toBeGreaterThan( Date.now() ); @@ -751,7 +772,7 @@ describe("UserSession", () => { }, location: { href: - "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey" + "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true" } }; diff --git a/packages/arcgis-rest-auth/test/fetchToken.test.ts b/packages/arcgis-rest-auth/test/fetchToken.test.ts index 3e0af4d179..b30ee04b2f 100644 --- a/packages/arcgis-rest-auth/test/fetchToken.test.ts +++ b/packages/arcgis-rest-auth/test/fetchToken.test.ts @@ -13,7 +13,7 @@ describe("fetchToken()", () => { fetchMock.postOnce(TOKEN_URL, { access_token: "token", expires_in: 1800, - ssl: false + ssl: true }); fetchToken(TOKEN_URL, { @@ -34,7 +34,7 @@ describe("fetchToken()", () => { expect(options.body).toContain("grant_type=client_credentials"); expect(response.token).toEqual("token"); expect(response.expires).toBeGreaterThan(Date.now()); - expect(response.ssl).toEqual(false); + expect(response.ssl).toEqual(true); done(); }) .catch(e => { @@ -84,4 +84,29 @@ describe("fetchToken()", () => { fail(e); }); }); + + it("should return ssl: false when there is no ssl property returned from endpoint response", done => { + fetchMock.postOnce(TOKEN_URL, { + access_token: "token", + expires_in: 1800, + refresh_token: "refreshToken", + username: "Casey" + }); + + fetchToken(TOKEN_URL, { + params: { + client_id: "clientId", + redirect_uri: "https://example-app.com/redirect-uri", + code: "authorizationCode", + grant_type: "authorization_code" + } + }) + .then(response => { + expect(response.ssl).toEqual(false); + done(); + }) + .catch(e => { + fail(e); + }); + }); }); From f80f4f814bb96a3edbaa655cc281e9ac889c3fbd Mon Sep 17 00:00:00 2001 From: john gravois Date: Thu, 20 Sep 2018 13:12:32 -0700 Subject: [PATCH 5/5] make it clearer in doc that ssl shouldnt be set in constructor --- packages/arcgis-rest-auth/src/UserSession.ts | 4 ++-- packages/arcgis-rest-auth/test/UserSession.test.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts index 9e88727a90..6f0ec4d26c 100644 --- a/packages/arcgis-rest-auth/src/UserSession.ts +++ b/packages/arcgis-rest-auth/src/UserSession.ts @@ -166,7 +166,7 @@ export interface IUserSessionOptions { portal?: string; /** - * Whether requests should be made exclusively over HTTPS. + * This value is set to true automatically if the ArcGIS Organization requires that requests be made over https. */ ssl?: boolean; @@ -221,7 +221,7 @@ export class UserSession implements IAuthenticationManager { readonly portal: string; /** - * Whether requests should be made exclusively over HTTPS. + * This value is set to true automatically if the ArcGIS Organization requires that requests be made over https. */ readonly ssl: boolean; diff --git a/packages/arcgis-rest-auth/test/UserSession.test.ts b/packages/arcgis-rest-auth/test/UserSession.test.ts index 6a0eaccafa..a5dbf373e5 100644 --- a/packages/arcgis-rest-auth/test/UserSession.test.ts +++ b/packages/arcgis-rest-auth/test/UserSession.test.ts @@ -19,7 +19,6 @@ describe("UserSession", () => { const session = new UserSession({ clientId: "clientId", redirectUri: "https://example-app.com/redirect-uri", - ssl: false, token: "token", tokenExpires: TOMORROW, refreshToken: "refreshToken", @@ -35,7 +34,7 @@ describe("UserSession", () => { expect(session2.redirectUri).toEqual( "https://example-app.com/redirect-uri" ); - expect(session2.ssl).toEqual(false); + expect(session2.ssl).toBe(undefined); expect(session2.token).toEqual("token"); expect(session2.tokenExpires).toEqual(TOMORROW); expect(session2.refreshToken).toEqual("refreshToken");