From 1786197f5a365cd7c1035ad6e5eaf04639b0265c Mon Sep 17 00:00:00 2001 From: Max P Date: Wed, 28 Sep 2016 12:46:17 -0700 Subject: [PATCH 1/4] Extend isAuthenticated command with error message and status. --- sdk/script/management-sdk.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sdk/script/management-sdk.ts b/sdk/script/management-sdk.ts index c913a794..8de54a11 100644 --- a/sdk/script/management-sdk.ts +++ b/sdk/script/management-sdk.ts @@ -76,8 +76,8 @@ class AccountManager { return this._accessKey; } - public isAuthenticated(): Promise { - return Promise((resolve, reject, notify) => { + public isAuthenticatedWithMessage(): Promise { + return Promise((resolve, reject, notify) => { var request: superagent.Request = superagent.get(this._serverUrl + urlEncode `/authenticated`); if (this._proxy) (request).proxy(this._proxy); this.attachCredentials(request); @@ -90,12 +90,20 @@ class AccountManager { } var authenticated: boolean = status === 200; + var errorMessage: string = authenticated ? null : this.getErrorMessage(err, res); - resolve(authenticated); + resolve({authenticated, status, errorMessage}); }); }); } + public isAuthenticated(): Promise { + return this.isAuthenticatedWithMessage() + .then((res: any) => { + return res.authenticated; + }); + } + public addAccessKey(friendlyName: string, ttl?: number): Promise { if (!friendlyName) { throw new Error("A name must be specified when adding an access key."); From 4b6df54ab42c9857a45f82b99d085c61f0d9d949 Mon Sep 17 00:00:00 2001 From: Max P Date: Wed, 28 Sep 2016 12:48:01 -0700 Subject: [PATCH 2/4] Check that session is still authenticated before uploading new release. --- cli/script/command-executor.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/cli/script/command-executor.ts b/cli/script/command-executor.ts index 857a190f..e7c2b7ab 100644 --- a/cli/script/command-executor.ts +++ b/cli/script/command-executor.ts @@ -1172,7 +1172,17 @@ export var release = (command: cli.IReleaseCommand): Promise => { return getPackageFilePromise .then((file: IPackageFile): Promise => { - return sdk.release(command.appName, command.deploymentName, file.path, command.appStoreVersion, updateMetadata, uploadProgress) + return sdk.isAuthenticatedWithMessage() + .then((result: any): Promise => { + if (result.authenticated){ + return sdk.release(command.appName, command.deploymentName, file.path, command.appStoreVersion, updateMetadata, uploadProgress); + } else { + throw { + statusCode: result.status, + message: result.errorMessage + }; + } + }) .then((): void => { log("Successfully released an update containing the \"" + command.package + "\" " + (isSingleFilePackage ? "file" : "directory") + " to the \"" + command.deploymentName + "\" deployment of the \"" + command.appName + "\" app."); }) From e410244739cff42f94e359f1397237e09ad5f8f4 Mon Sep 17 00:00:00 2001 From: Max P Date: Thu, 29 Sep 2016 10:44:18 -0700 Subject: [PATCH 3/4] refactor isAuthenticated SDK api to erase session state when authorization failed on 'code-push release' call. --- cli/script/command-executor.ts | 13 +++---------- sdk/script/management-sdk.ts | 17 +++++++---------- sdk/test/management-sdk.ts | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/cli/script/command-executor.ts b/cli/script/command-executor.ts index e7c2b7ab..6748463d 100644 --- a/cli/script/command-executor.ts +++ b/cli/script/command-executor.ts @@ -1172,16 +1172,9 @@ export var release = (command: cli.IReleaseCommand): Promise => { return getPackageFilePromise .then((file: IPackageFile): Promise => { - return sdk.isAuthenticatedWithMessage() - .then((result: any): Promise => { - if (result.authenticated){ - return sdk.release(command.appName, command.deploymentName, file.path, command.appStoreVersion, updateMetadata, uploadProgress); - } else { - throw { - statusCode: result.status, - message: result.errorMessage - }; - } + return sdk.isAuthenticated(true) + .then((isAuth: boolean): Promise => { + return sdk.release(command.appName, command.deploymentName, file.path, command.appStoreVersion, updateMetadata, uploadProgress); }) .then((): void => { log("Successfully released an update containing the \"" + command.package + "\" " + (isSingleFilePackage ? "file" : "directory") + " to the \"" + command.deploymentName + "\" deployment of the \"" + command.appName + "\" app."); diff --git a/sdk/script/management-sdk.ts b/sdk/script/management-sdk.ts index 8de54a11..71464c41 100644 --- a/sdk/script/management-sdk.ts +++ b/sdk/script/management-sdk.ts @@ -76,7 +76,7 @@ class AccountManager { return this._accessKey; } - public isAuthenticatedWithMessage(): Promise { + public isAuthenticated(throwIfUnauthorized?: boolean): Promise { return Promise((resolve, reject, notify) => { var request: superagent.Request = superagent.get(this._serverUrl + urlEncode `/authenticated`); if (this._proxy) (request).proxy(this._proxy); @@ -90,18 +90,15 @@ class AccountManager { } var authenticated: boolean = status === 200; - var errorMessage: string = authenticated ? null : this.getErrorMessage(err, res); - resolve({authenticated, status, errorMessage}); - }); - }); - } + if (!authenticated && throwIfUnauthorized){ + reject(this.getCodePushError(err, res)); + return; + } - public isAuthenticated(): Promise { - return this.isAuthenticatedWithMessage() - .then((res: any) => { - return res.authenticated; + resolve(authenticated); }); + }); } public addAccessKey(friendlyName: string, ttl?: number): Promise { diff --git a/sdk/test/management-sdk.ts b/sdk/test/management-sdk.ts index 96cc35bc..cf2c850e 100644 --- a/sdk/test/management-sdk.ts +++ b/sdk/test/management-sdk.ts @@ -88,6 +88,20 @@ describe("Management SDK", () => { }); }); + it("isAuthenticated handles unsuccessful auth with promise rejection", (done: MochaDone) => { + mockReturn("Unauthorized", 401, {}); + + // use optional parameter to ask for rejection of the promise if not authenticated + manager.isAuthenticated(true) + .done((authenticated: boolean) => { + assert.fail("isAuthenticated should have rejected the promise"); + done(); + }, (err) => { + assert.equal(err.message, "Unauthorized", "Error message should be 'Unauthorized'"); + done(); + }); + }); + it("isAuthenticated handles unexpected status codes", (done: MochaDone) => { mockReturn("Not Found", 404, {}); manager.isAuthenticated() From 9b2c9a8cc12323f875a0fee2c35c87bd0fd0243e Mon Sep 17 00:00:00 2001 From: Max P Date: Thu, 29 Sep 2016 12:14:49 -0700 Subject: [PATCH 4/4] Rename sdk.isAuthrnticated method to ensureAuthenticated and make behavior consistent. --- cli/script/command-executor.ts | 28 +++++++++++++--------------- sdk/script/management-sdk.ts | 4 ++-- sdk/test/management-sdk.ts | 27 ++++++++------------------- 3 files changed, 23 insertions(+), 36 deletions(-) diff --git a/cli/script/command-executor.ts b/cli/script/command-executor.ts index 6748463d..86fd8189 100644 --- a/cli/script/command-executor.ts +++ b/cli/script/command-executor.ts @@ -592,13 +592,12 @@ function login(command: cli.ILoginCommand): Promise { if (command.accessKey) { var proxy = getProxy(command.proxy, command.noProxy); sdk = getSdk(command.accessKey, CLI_HEADERS, command.serverUrl, proxy); - return sdk.isAuthenticated() - .then((isAuthenticated: boolean): void => { - if (isAuthenticated) { - serializeConnectionInfo(command.accessKey, /*preserveAccessKeyOnLogout*/ true, command.serverUrl, command.proxy, command.noProxy); - } else { - throw new Error("Invalid access key."); - } + return sdk.ensureAuthenticated() + .catch((err: CodePushError) =>{ + throw new Error("Invalid access key."); + }) + .then((): void => { + serializeConnectionInfo(command.accessKey, /*preserveAccessKeyOnLogout*/ true, command.serverUrl, command.proxy, command.noProxy); }); } else { return loginWithExternalAuthentication("login", command.serverUrl, command.proxy, command.noProxy); @@ -618,13 +617,12 @@ function loginWithExternalAuthentication(action: string, serverUrl?: string, pro sdk = getSdk(accessKey, CLI_HEADERS, serverUrl, getProxy(proxy, noProxy)); - return sdk.isAuthenticated() - .then((isAuthenticated: boolean): void => { - if (isAuthenticated) { - serializeConnectionInfo(accessKey, /*preserveAccessKeyOnLogout*/ false, serverUrl, proxy, noProxy); - } else { - throw new Error("Invalid access key."); - } + return sdk.ensureAuthenticated() + .catch((err: CodePushError) => { + throw new Error("Invalid access key."); + }) + .then((): void => { + serializeConnectionInfo(accessKey, /*preserveAccessKeyOnLogout*/ false, serverUrl, proxy, noProxy); }); }); } @@ -1172,7 +1170,7 @@ export var release = (command: cli.IReleaseCommand): Promise => { return getPackageFilePromise .then((file: IPackageFile): Promise => { - return sdk.isAuthenticated(true) + return sdk.ensureAuthenticated() .then((isAuth: boolean): Promise => { return sdk.release(command.appName, command.deploymentName, file.path, command.appStoreVersion, updateMetadata, uploadProgress); }) diff --git a/sdk/script/management-sdk.ts b/sdk/script/management-sdk.ts index 71464c41..7fb22691 100644 --- a/sdk/script/management-sdk.ts +++ b/sdk/script/management-sdk.ts @@ -76,7 +76,7 @@ class AccountManager { return this._accessKey; } - public isAuthenticated(throwIfUnauthorized?: boolean): Promise { + public ensureAuthenticated(): Promise { return Promise((resolve, reject, notify) => { var request: superagent.Request = superagent.get(this._serverUrl + urlEncode `/authenticated`); if (this._proxy) (request).proxy(this._proxy); @@ -91,7 +91,7 @@ class AccountManager { var authenticated: boolean = status === 200; - if (!authenticated && throwIfUnauthorized){ + if (!authenticated ){ reject(this.getCodePushError(err, res)); return; } diff --git a/sdk/test/management-sdk.ts b/sdk/test/management-sdk.ts index cf2c850e..9f4fd78f 100644 --- a/sdk/test/management-sdk.ts +++ b/sdk/test/management-sdk.ts @@ -70,31 +70,20 @@ describe("Management SDK", () => { } }); - it("isAuthenticated handles successful auth", (done: MochaDone) => { + it("ensureAuthenticated handles successful auth", (done: MochaDone) => { mockReturn(JSON.stringify({ authenticated: true }), 200, {}); - manager.isAuthenticated() + manager.ensureAuthenticated() .done((authenticated: boolean) => { assert(authenticated, "Should be authenticated"); done(); }); }); - it("isAuthenticated handles unsuccessful auth", (done: MochaDone) => { + it("ensureAuthenticated handles unsuccessful auth", (done: MochaDone) => { mockReturn("Unauthorized", 401, {}); - manager.isAuthenticated() + manager.ensureAuthenticated() .done((authenticated: boolean) => { - assert(!authenticated, "Should not be authenticated"); - done(); - }); - }); - - it("isAuthenticated handles unsuccessful auth with promise rejection", (done: MochaDone) => { - mockReturn("Unauthorized", 401, {}); - - // use optional parameter to ask for rejection of the promise if not authenticated - manager.isAuthenticated(true) - .done((authenticated: boolean) => { - assert.fail("isAuthenticated should have rejected the promise"); + assert.fail("ensureAuthenticated should have rejected the promise"); done(); }, (err) => { assert.equal(err.message, "Unauthorized", "Error message should be 'Unauthorized'"); @@ -102,11 +91,11 @@ describe("Management SDK", () => { }); }); - it("isAuthenticated handles unexpected status codes", (done: MochaDone) => { + it("ensureAuthenticated handles unexpected status codes", (done: MochaDone) => { mockReturn("Not Found", 404, {}); - manager.isAuthenticated() + manager.ensureAuthenticated() .done((authenticated: boolean) => { - assert.fail("isAuthenticated should have rejected the promise"); + assert.fail("ensureAuthenticated should have rejected the promise"); done(); }, (err) => { assert.equal(err.message, "Not Found", "Error message should be 'Not Found'");