From f03ed1382e41a04484c23ab8d28f7cf9189b55b8 Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Thu, 25 Aug 2022 14:13:58 -0700 Subject: [PATCH] fix: #995 and #1006, issues with redirect urls --- demos/oauth2-browser/authenticate.html | 42 +- demos/oauth2-browser/index.html | 386 +++++++++--------- package-lock.json | 2 +- .../src/ArcGISIdentityManager.ts | 60 ++- .../test/ArcGISIdentityManager.test.ts | 57 ++- .../arcgis-rest-request/test/request.test.ts | 12 +- 6 files changed, 332 insertions(+), 227 deletions(-) diff --git a/demos/oauth2-browser/authenticate.html b/demos/oauth2-browser/authenticate.html index 724d88aab1..546ddcb0bf 100644 --- a/demos/oauth2-browser/authenticate.html +++ b/demos/oauth2-browser/authenticate.html @@ -1,21 +1,25 @@ - - - ArcGIS Rest JS OAuth redirect - - - - - - - + + + + ArcGIS Rest JS OAuth redirect + + + + + + + + + \ No newline at end of file diff --git a/demos/oauth2-browser/index.html b/demos/oauth2-browser/index.html index e169658648..09ffbff9b3 100644 --- a/demos/oauth2-browser/index.html +++ b/demos/oauth2-browser/index.html @@ -1,223 +1,231 @@ - - - ArcGIS REST JS Browser OAuth2 - - - - -
-
-
-
-
- -
-
-

- ArcGIS REST JS Browser OAuth2 -

-

- An application demonstrating browser-based named user login. -

-
-
-
-
+ + + + ArcGIS REST JS Browser OAuth2 + + + + + +
+
-
-
-
- - - -
-
- - - -
-
- - - -
-

- To use this demo copy the config.js.template file and rename it to config.js. Then fill in your values for clientId, popupRedirectUri and inlineRedirectUri. To get you started quickly an app registered by the ArcGIS REST JS team has been configured. -

-

- Consult the documentation for registering an app and adding a redirect URI for more information. +

+
+ +
+
+

+ ArcGIS REST JS Browser OAuth2 +

+

+ An application demonstrating browser-based named user login.

-
-
- - +
+
+
+
+
+
+ + + +
+
+ + +
-
- +
+ + +
+

+ To use this demo copy the config.js.template file and rename it to config.js. Then + fill in your values for clientId, popupRedirectUri and + inlineRedirectUri. To get you started quickly an app registered by the ArcGIS REST JS team has + been configured. +

+

+ Consult the documentation for registering + an app and adding + a redirect URI for more information. +

+
+
+
+
+ + +
+
+
+
-
-
-

- -

-
-
+
+
+

+ +

+
+
-
-
-
-
- - -
-
-
+
+
+
+
+ + +
+
+
- - - + + + - // Attach a listener to the sign out button. - document.getElementById('signOutButton').addEventListener('click', function (event) { - // destroy the session and remove the item in local storage - arcgisRest.ArcGISIdentityManager.destroy(session).then(({success})=>{ - if(success) { - // Clear the previous session. - session = null; - localStorage.removeItem('__ARCGIS_REST_USER_SESSION__'); - } - }).finally(()=>{ - updateSessionInfo(); - }); - }); - - - + \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index f60b7c81a7..f4bb68fc12 100644 --- a/package-lock.json +++ b/package-lock.json @@ -39874,7 +39874,7 @@ }, "packages/arcgis-rest-portal": { "name": "@esri/arcgis-rest-portal", - "version": "4.0.3", + "version": "4.0.4", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.0" diff --git a/packages/arcgis-rest-request/src/ArcGISIdentityManager.ts b/packages/arcgis-rest-request/src/ArcGISIdentityManager.ts index 34d26dff61..af47094a65 100644 --- a/packages/arcgis-rest-request/src/ArcGISIdentityManager.ts +++ b/packages/arcgis-rest-request/src/ArcGISIdentityManager.ts @@ -27,11 +27,34 @@ import { NODEJS_DEFAULT_REFERER_HEADER } from "./index.js"; * Options for {@linkcode ArcGISIdentityManager.fromToken}. */ export interface IFromTokenOptions { + /** + * The token you want to create the {@linkcode ArcGISIdentityManager} instance with. + */ token: string; + /** + * Date when this token will expire. + */ tokenExpires?: Date; + /** + * The portal that the token was generated from. Defaults to `https://www.arcgis.com/sharing/rest`. Required if you are not using the default portal. + */ portal?: string; + /** + * If the token is for a specific instance of ArcGIS Server, set `portal` to `null` or `undefined` and set `server` the URL of the ArcGIS Server. + */ server?: string; + /** + * Optionally set the username. Recommended if available. + */ username?: string; + /** + * Optional client ID. Used for refreshing expired tokens. + */ + clientId?: string; + /** + * Optional set a valid redirect URL for the registered client ID. Used internally to refresh expired tokens. + */ + redirectUri?: string; } /** @@ -237,7 +260,7 @@ export interface IArcGISIdentityManagerOptions { * * **It is not recommended to construct `ArcGISIdentityManager` directly**. Instead there are several static methods used for specific workflows. The 2 primary workflows relate to oAuth 2.0: * - * * {@linkcode ArcGISIdentityManager.beginOAuth2} and {@linkcode ArcGISIdentityManager.completeOAuth2} for oAuth 2.0 in browser-only environment. + * * {@linkcode ArcGISIdentityManager.beginOAuth2} and {@linkcode ArcGISIdentityManager.completeOAuth2()} for oAuth 2.0 in browser-only environment. * * {@linkcode ArcGISIdentityManager.authorize} and {@linkcode ArcGISIdentityManager.exchangeAuthorizationCode} for oAuth 2.0 for server-enabled application. * * Other more specialized helpers for less common workflows also exist: @@ -302,7 +325,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { return true; } - if (this.clientId && this.refreshToken) { + if (this.clientId && this.refreshToken && this.redirectUri) { return true; } @@ -312,7 +335,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { /** * Begins a new browser-based OAuth 2.0 sign in. If `options.popup` is `true` the authentication window will open in a new tab/window. Otherwise, the user will be redirected to the authorization page in their current tab/window and the function will return `undefined`. * - * If `popup` is `true` (the default) this method will return a `Promise` that resolves to an `ArcGISIdentityManager` instance and you must call {@linkcode ArcGISIdentityManager.completeOAuth2} on the page defined in the `redirectUri`. Otherwise it will return undefined and the {@linkcode ArcGISIdentityManager.completeOAuth2} method will return a `Promise` that resolves to an `ArcGISIdentityManager` instance. + * If `popup` is `true` (the default) this method will return a `Promise` that resolves to an `ArcGISIdentityManager` instance and you must call {@linkcode ArcGISIdentityManager.completeOAuth2()} on the page defined in the `redirectUri`. Otherwise it will return undefined and the {@linkcode ArcGISIdentityManager.completeOAuth2()} method will return a `Promise` that resolves to an `ArcGISIdentityManager` instance. * * A {@linkcode ArcGISAccessDeniedError} error will be thrown if the user denies the request on the authorization screen. * @@ -435,7 +458,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { if (popup) { // If we are authenticating a popup we need to return a Promise that will resolve to an ArcGISIdentityManager later. return new Promise((resolve, reject) => { - // Add an event listener to listen for when a user calls `ArcGISIdentityManager.completeOAuth2()` in the popup. + // Add an event listener to listen for when a user calls `ArcGISIdentityManager.completeOAuth2()()` in the popup. win.addEventListener( `arcgis-rest-js-popup-auth-${clientId}`, (e: CustomEvent) => { @@ -445,7 +468,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { return error; } - if (e.detail.error) { + if (e.detail.errorMessage) { const error = new ArcGISAuthError( e.detail.errorMessage, e.detail.error @@ -463,7 +486,8 @@ export class ArcGISIdentityManager implements IAuthenticationManager { tokenExpires: e.detail.expires, username: e.detail.username, refreshToken: e.detail.refreshToken, - refreshTokenExpires: e.detail.refreshTokenExpires + refreshTokenExpires: e.detail.refreshTokenExpires, + redirectUri }) ); }, @@ -499,7 +523,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { } // pull out necessary options - const { portal, clientId, popup, pkce }: IOAuth2Options = { + const { portal, clientId, popup, pkce, redirectUri }: IOAuth2Options = { ...{ portal: "https://www.arcgis.com/sharing/rest", popup: true, @@ -539,6 +563,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { ); win.close(); + return; } @@ -570,6 +595,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { ); win.close(); + return; } @@ -583,7 +609,15 @@ export class ArcGISIdentityManager implements IAuthenticationManager { tokenExpires: oauthInfo.expires, username: oauthInfo.username, refreshToken: oauthInfo.refreshToken, - refreshTokenExpires: oauthInfo.refreshTokenExpires + refreshTokenExpires: oauthInfo.refreshTokenExpires, + // At 4.0.0 it was possible (in JS code) to not pass redirectUri and fallback to win.location.href, however this broke support for redirect URIs with query params. + // Now similar to 3.x.x you must pass the redirectUri parameter explicitly. See https://github.com/Esri/arcgis-rest-js/issues/995 + redirectUri: + redirectUri || + /* istanbul ignore next: TypeScript wont compile if we omit redirectUri */ location.href.replace( + location.search, + "" + ) }); } @@ -625,7 +659,9 @@ export class ArcGISIdentityManager implements IAuthenticationManager { client_id: clientId, code_verifier: codeVerifier, grant_type: "authorization_code", - redirect_uri: location.href.replace(location.search, ""), + // using location.href here does not support query params but shipped with 4.0.0. See https://github.com/Esri/arcgis-rest-js/issues/995 + redirect_uri: + redirectUri || location.href.replace(location.search, ""), code: params.code } }) @@ -636,7 +672,7 @@ export class ArcGISIdentityManager implements IAuthenticationManager { ); }) .catch((e) => { - return reportError(e.message, e.error, state.originalUrl); + return reportError(e.originalMessage, e.code, state.originalUrl); }); } @@ -1742,11 +1778,11 @@ UserSession.completeOAuth2 = function ( ...args: Parameters ) { console.warn( - "DEPRECATED:, 'UserSession.completeOAuth2' is deprecated. Use 'ArcGISIdentityManager.completeOAuth2' instead." + "DEPRECATED:, 'UserSession.completeOAuth2()' is deprecated. Use 'ArcGISIdentityManager.completeOAuth2()' instead." ); if (args.length <= 1) { console.warn( - "WARNING:, 'UserSession.completeOAuth2' is now async and returns a promise the resolves to an instance of `ArcGISIdentityManager`." + "WARNING:, 'UserSession.completeOAuth2()' is now async and returns a promise the resolves to an instance of `ArcGISIdentityManager`." ); } diff --git a/packages/arcgis-rest-request/test/ArcGISIdentityManager.test.ts b/packages/arcgis-rest-request/test/ArcGISIdentityManager.test.ts index 68686b0b59..1f52c0fdcb 100644 --- a/packages/arcgis-rest-request/test/ArcGISIdentityManager.test.ts +++ b/packages/arcgis-rest-request/test/ArcGISIdentityManager.test.ts @@ -13,7 +13,8 @@ import { ArcGISTokenRequestError, ArcGISTokenRequestErrorCodes, IServerInfo, - ITokenRequestOptions + ITokenRequestOptions, + IOAuth2Options } from "../src/index.js"; import { FormData } from "@esri/arcgis-rest-form-data"; import { @@ -832,7 +833,8 @@ describe("ArcGISIdentityManager", () => { token: "token", username: "c@sey", refreshToken: "refreshToken", - refreshTokenExpires: TOMORROW + refreshTokenExpires: TOMORROW, + redirectUri: "https://example-app.com/redirect-uri" }); expect(session.canRefresh).toBe(true); @@ -1533,6 +1535,57 @@ describe("ArcGISIdentityManager", () => { }); }); + it("should fallback to window.location.href if redirect URI is omitted in a popup workflow", () => { + let PopupMockWindow: any; + + fetchMock.once("*", { + access_token: "token", + expires_in: 1800, + username: "c@sey", + ssl: true, + refresh_token: "refresh_token", + refresh_token_expires_in: 1209600 + }); + + window.addEventListener("arcgis-rest-js-popup-auth-start", () => { + PopupMockWindow = createMock(); + PopupMockWindow.location.href = "http://example-app.com/redirect"; + PopupMockWindow.location.search = + "?code=auth_code&state=%7B%22id%22%3A%22AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA%22%2C%22originalUrl%22%3A%22https%3A%2F%2Ftest.com%22%7D"; + PopupMockWindow.opener = MockWindow; + + ArcGISIdentityManager.completeOAuth2( + { + clientId: "clientId1234" + } as any, + PopupMockWindow + ); + }); + + return ArcGISIdentityManager.beginOAuth2( + { + clientId: "clientId1234", + redirectUri: "http://example-app.com/redirect" + }, + MockWindow + ) + .then((session) => { + expect(MockWindow.open).toHaveBeenCalledWith( + "https://www.arcgis.com/sharing/rest/oauth2/authorize?client_id=clientId1234&response_type=code&expiration=20160&redirect_uri=http%3A%2F%2Fexample-app.com%2Fredirect&state=%7B%22id%22%3A%22AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA%22%2C%22originalUrl%22%3A%22https%3A%2F%2Ftest.com%22%7D&locale=&style=&code_challenge_method=S256&code_challenge=DwBzhbb51LfusnSGBa_hqYSgo7-j8BTQnip4TOnlzRo", + "oauth-window", + "height=400,width=600,menubar=no,location=yes,resizable=yes,scrollbars=yes,status=yes" + ); + + expect(PopupMockWindow.close).toHaveBeenCalled(); + expect(session.redirectUri).toBe( + "http://example-app.com/redirect" + ); + }) + .catch((e) => { + fail(e); + }); + }); + it("should authorize with an inline redirect with a plain challange", () => { MockWindow.isSecureContext = false; diff --git a/packages/arcgis-rest-request/test/request.test.ts b/packages/arcgis-rest-request/test/request.test.ts index d044955ad2..913777c4e8 100644 --- a/packages/arcgis-rest-request/test/request.test.ts +++ b/packages/arcgis-rest-request/test/request.test.ts @@ -589,7 +589,8 @@ describe("request()", () => { token: "INVALID_TOKEN", username: "c@sey", refreshToken: "refreshToken", - refreshTokenExpires: FIVE_DAYS_FROM_NOW + refreshTokenExpires: FIVE_DAYS_FROM_NOW, + redirectUri: "https://example-app.com/redirect-uri" }); expect(session.canRefresh).toBe(true); @@ -632,7 +633,8 @@ describe("request()", () => { token: "INVALID_TOKEN", username: "c@sey", refreshToken: "refreshToken", - refreshTokenExpires: TOMORROW + refreshTokenExpires: TOMORROW, + redirectUri: "https://example-app.com/redirect-uri" }); expect(session.canRefresh).toBe(true); @@ -670,7 +672,8 @@ describe("request()", () => { token: "INVALID_TOKEN", username: "c@sey", refreshToken: "refreshToken", - refreshTokenExpires: TOMORROW + refreshTokenExpires: TOMORROW, + redirectUri: "https://example-app.com/redirect-uri" }); fetchMock.post("https://www.arcgis.com/sharing/rest/oauth2/token", { @@ -708,7 +711,8 @@ describe("request()", () => { token: "TOKEN", username: "c@sey", refreshToken: "refreshToken", - refreshTokenExpires: FIVE_DAYS_FROM_NOW + refreshTokenExpires: FIVE_DAYS_FROM_NOW, + redirectUri: "https://example-app.com/redirect-uri" }); fetchMock.post("https://www.arcgis.com/sharing/rest/oauth2/token", {