From 33891d9aef31fb87d149570980dac096d32b4e71 Mon Sep 17 00:00:00 2001 From: Sourav Chanduka Date: Thu, 12 Aug 2021 10:07:18 +0530 Subject: [PATCH] addressed comments --- packages/core/README.md | 2 +- packages/core/__tests__/core.test.ts | 15 ------- packages/core/package-lock.json | 10 ++--- packages/core/package.json | 4 +- packages/core/src/oidc-utils.ts | 62 ++++++++-------------------- 5 files changed, 26 insertions(+), 67 deletions(-) diff --git a/packages/core/README.md b/packages/core/README.md index f352c42e..ac75489d 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -210,7 +210,7 @@ process.kill(pid); #### OIDC Token -You can use this library to interact with the GitHub OIDC provider and get a JWT ID token which would help to get access token from third party cloud providers. +You can use these methods to interact with the GitHub OIDC provider and get a JWT ID token which would help to get access token from third party cloud providers. **Method Name**: getIDToken() diff --git a/packages/core/__tests__/core.test.ts b/packages/core/__tests__/core.test.ts index b1844457..ccb6ab34 100644 --- a/packages/core/__tests__/core.test.ts +++ b/packages/core/__tests__/core.test.ts @@ -419,24 +419,9 @@ describe('oidc-client-tests', () => { expect(res.message.statusCode).toBe(200) }) - it('check if success status return true, if succeeded', () => { - expect(oidcClient.isSuccessStatusCode(200)).toBeTruthy() - }) - - it('check if success status return false, if failed', () => { - expect(oidcClient.isSuccessStatusCode(400)).toBeFalsy() - }) - it('check if we get correct ID Token Request url with right api version', () => { process.env.ACTIONS_ID_TOKEN_REQUEST_URL = "https://www.example.com/" expect(oidcClient.getIDTokenUrl()).toBe("https://www.example.com/?api-version=" + oidcClient.getApiVersion()) }) - it('check if invalid json throws error', () => { - expect(() => oidcClient.parseJson("{}")).toThrow() - }) - - it('check if valid json returns parsed id token', () => { - expect(oidcClient.parseJson('{"value" : "abc" }')).toBe("abc") - }) }) \ No newline at end of file diff --git a/packages/core/package-lock.json b/packages/core/package-lock.json index 4fa508f7..eadc6cc5 100644 --- a/packages/core/package-lock.json +++ b/packages/core/package-lock.json @@ -8,8 +8,10 @@ "name": "@actions/core", "version": "1.5.0", "license": "MIT", + "dependencies": { + "@actions/http-client": "^1.0.11" + }, "devDependencies": { - "@actions/http-client": "^1.0.11", "@types/node": "^12.0.2" } }, @@ -17,7 +19,6 @@ "version": "1.0.11", "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-1.0.11.tgz", "integrity": "sha512-VRYHGQV1rqnROJqdMvGUbY/Kn8vriQe/F9HR2AlYHzmKuM/p3kjNuXhmdBfcVgsvRWTz5C5XW5xvndZrVBuAYg==", - "dev": true, "dependencies": { "tunnel": "0.0.6" } @@ -32,7 +33,6 @@ "version": "0.0.6", "resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz", "integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==", - "dev": true, "engines": { "node": ">=0.6.11 <=0.7.0 || >=0.7.3" } @@ -43,7 +43,6 @@ "version": "1.0.11", "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-1.0.11.tgz", "integrity": "sha512-VRYHGQV1rqnROJqdMvGUbY/Kn8vriQe/F9HR2AlYHzmKuM/p3kjNuXhmdBfcVgsvRWTz5C5XW5xvndZrVBuAYg==", - "dev": true, "requires": { "tunnel": "0.0.6" } @@ -57,8 +56,7 @@ "tunnel": { "version": "0.0.6", "resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz", - "integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==", - "dev": true + "integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==" } } } diff --git a/packages/core/package.json b/packages/core/package.json index 78b29316..33a7fcb4 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -35,8 +35,10 @@ "bugs": { "url": "https://github.com/actions/toolkit/issues" }, + "dependencies": { + "@actions/http-client": "^1.0.11" + }, "devDependencies": { - "@actions/http-client": "^1.0.11", "@types/node": "^12.0.2" } } diff --git a/packages/core/src/oidc-utils.ts b/packages/core/src/oidc-utils.ts index 0fea0af3..ad8d2714 100644 --- a/packages/core/src/oidc-utils.ts +++ b/packages/core/src/oidc-utils.ts @@ -14,11 +14,7 @@ interface IOidcClient { getIDTokenUrl(): string - isSuccessStatusCode(statusCode?: number): boolean - - postCall(id_token_url: string, audience: string): Promise - - parseJson(body: string): string + postCall(httpclient: actions_http_client.HttpClient, id_token_url: string, audience: string): Promise getIDToken(audience: string): Promise } @@ -54,67 +50,45 @@ export class OidcClient implements IOidcClient { return runtimeUrl + '?api-version=' + this.getApiVersion() } - isSuccessStatusCode(statusCode?: number): boolean { - if (!statusCode) { - return false - } - return statusCode >= 200 && statusCode < 300 - } - - async postCall(id_token_url: string, audience: string): Promise { - - const httpclient = this.createHttpClient() - if (httpclient === undefined) { - throw new Error(`Failed to get Httpclient `) - } - - let additionalHeaders: IHeaders = {} - additionalHeaders[actions_http_client.Headers.ContentType] = actions_http_client.MediaTypes.ApplicationJson - additionalHeaders[actions_http_client.Headers.Accept] = actions_http_client.MediaTypes.ApplicationJson + async postCall(httpclient: actions_http_client.HttpClient, id_token_url: string, audience: string): Promise { + const data = audience !== null ? {aud: audience} : '' debug(`audience is ${audience !== null ? audience : 'null'}`) - const data: string = audience !== null ? JSON.stringify({aud: audience}) : '' - const response = await httpclient.post(id_token_url, data, additionalHeaders) - const body: string = await response.readBody() - - if (!this.isSuccessStatusCode(response.message.statusCode)) { + const res = await httpclient.postJson(id_token_url,data).catch((error) => { throw new Error( `Failed to get ID Token. \n - Error Code : ${response.message.statusCode} Error message : ${response.message.statusMessage} \n - Response body: ${body}` + Error Code : ${error.statusCode}\n + Response body: ${error.result}` ) - } + }) - return body - } - - parseJson(body: string): string { - const val = JSON.parse(body) - let id_token = '' - if ('value' in val) { - id_token = val['value'] - } else { + let val :any = res.result + let id_token = val['value'] + if (id_token === undefined) { throw new Error('Response json body do not have ID Token field') } - setSecret(id_token) return id_token + } async getIDToken(audience: string): Promise { try { + const httpclient = this.createHttpClient() + if (httpclient === undefined) { + throw new Error(`Failed to get Httpclient `) + } + // New ID Token is requested from action service let id_token_url: string = this.getIDTokenUrl() debug(`ID token url is ${id_token_url}`) - let body: string = await this.postCall(id_token_url, audience) - let id_token = this.parseJson(body) + let id_token = await this.postCall(httpclient ,id_token_url, audience) + setSecret(id_token) return id_token - } catch (error) { throw new Error(`Error message: ${error.message}`) } } - } \ No newline at end of file