From 4259ce3e2c2f6a2a8cbb5ef40182ee8cfd3e2579 Mon Sep 17 00:00:00 2001 From: Santhosh Vaddi Date: Mon, 22 Jul 2019 11:27:52 -0700 Subject: [PATCH 1/5] Reafactored tests and updated google-auth-library source link. --- functions/tokenservice/README.md | 2 +- .../tokenservice/functions/.eslintrc.json | 123 ++++++++++++++++++ functions/tokenservice/functions/package.json | 6 +- .../tokenservice/functions/test/index.test.js | 118 ++++++++++------- 4 files changed, 199 insertions(+), 50 deletions(-) create mode 100644 functions/tokenservice/functions/.eslintrc.json diff --git a/functions/tokenservice/README.md b/functions/tokenservice/README.md index 9eddbc37a4..5bc90ea2e6 100644 --- a/functions/tokenservice/README.md +++ b/functions/tokenservice/README.md @@ -85,7 +85,7 @@ firebase use --add [0]: https://cloud.google.com -[1]: https://github.com/googleapis/google-auth-library-swift/tree/master/Sources/OAuth2/FirebaseFunctionTokenProvider +[1]: https://github.com/googleapis/google-auth-library-swift/tree/master/Sources/OAuth2/FCMTokenProvider [2]: https://github.com/GoogleCloudPlatform/ios-docs-samples.git [3]: https://github.com/GoogleCloudPlatform/android-docs-samples [4]: https://nodejs.org/en/ diff --git a/functions/tokenservice/functions/.eslintrc.json b/functions/tokenservice/functions/.eslintrc.json new file mode 100644 index 0000000000..ea4352b993 --- /dev/null +++ b/functions/tokenservice/functions/.eslintrc.json @@ -0,0 +1,123 @@ +{ + "parserOptions": { + // Required for certain syntax usages + "ecmaVersion": 8 + }, + "plugins": [ + "promise" + ], + "extends": "eslint:recommended", + "rules": { + // Removed rule "disallow the use of console" from recommended eslint rules + "no-console": "off", + + // Removed rule "disallow multiple spaces in regular expressions" from recommended eslint rules + "no-regex-spaces": "off", + + // Removed rule "disallow the use of debugger" from recommended eslint rules + "no-debugger": "off", + + // Removed rule "disallow unused variables" from recommended eslint rules + "no-unused-vars": "off", + + // Removed rule "disallow mixed spaces and tabs for indentation" from recommended eslint rules + "no-mixed-spaces-and-tabs": "off", + + // Removed rule "disallow the use of undeclared variables unless mentioned in /*global */ comments" from recommended eslint rules + "no-undef": "off", + + // Warn against template literal placeholder syntax in regular strings + "no-template-curly-in-string": 1, + + // Warn if return statements do not either always or never specify values + "consistent-return": 1, + + // Warn if no return statements in callbacks of array methods + "array-callback-return": 1, + + // Require the use of === and !== + "eqeqeq": 2, + + // Disallow the use of alert, confirm, and prompt + "no-alert": 2, + + // Disallow the use of arguments.caller or arguments.callee + "no-caller": 2, + + // Disallow null comparisons without type-checking operators + "no-eq-null": 2, + + // Disallow the use of eval() + "no-eval": 2, + + // Warn against extending native types + "no-extend-native": 1, + + // Warn against unnecessary calls to .bind() + "no-extra-bind": 1, + + // Warn against unnecessary labels + "no-extra-label": 1, + + // Disallow leading or trailing decimal points in numeric literals + "no-floating-decimal": 2, + + // Warn against shorthand type conversions + "no-implicit-coercion": 1, + + // Warn against function declarations and expressions inside loop statements + "no-loop-func": 1, + + // Disallow new operators with the Function object + "no-new-func": 2, + + // Warn against new operators with the String, Number, and Boolean objects + "no-new-wrappers": 1, + + // Disallow throwing literals as exceptions + "no-throw-literal": 2, + + // Require using Error objects as Promise rejection reasons + "prefer-promise-reject-errors": 2, + + // Enforce “for” loop update clause moving the counter in the right direction + "for-direction": 2, + + // Enforce return statements in getters + "getter-return": 2, + + // Disallow await inside of loops + "no-await-in-loop": 2, + + // Disallow comparing against -0 + "no-compare-neg-zero": 2, + + // Warn against catch clause parameters from shadowing variables in the outer scope + "no-catch-shadow": 1, + + // Disallow identifiers from shadowing restricted names + "no-shadow-restricted-names": 2, + + // Enforce return statements in callbacks of array methods + "callback-return": 2, + + // Require error handling in callbacks + "handle-callback-err": 2, + + // Warn against string concatenation with __dirname and __filename + "no-path-concat": 1, + + // Prefer using arrow functions for callbacks + "prefer-arrow-callback": 1, + + // Return inside each then() to create readable and reusable Promise chains. + // Forces developers to return console logs and http calls in promises. + "promise/always-return": 2, + + //Enforces the use of catch() on un-returned promises + "promise/catch-or-return": 2, + + // Warn against nested then() or catch() statements + "promise/no-nesting": 1 + } +} diff --git a/functions/tokenservice/functions/package.json b/functions/tokenservice/functions/package.json index c1a76f341a..1583ab1590 100644 --- a/functions/tokenservice/functions/package.json +++ b/functions/tokenservice/functions/package.json @@ -12,12 +12,16 @@ "start": "functions-framework --target=getOAuthToken" }, "dependencies": { + "child-process-promise": "^2.2.1", "firebase-admin": "^8.0.0", - "firebase-functions": "^3.0.0" + "firebase-functions": "^3.0.0", + "request": "^2.88.0", + "requestretry": "^4.0.0" }, "devDependencies": { "@google-cloud/functions-framework": "^1.1.1", "@google-cloud/nodejs-repo-tools": "^3.3.0", + "babel-eslint": "^10.0.2", "eslint": "^6.0.0", "eslint-config-prettier": "^6.0.0", "eslint-plugin-node": "^9.1.0", diff --git a/functions/tokenservice/functions/test/index.test.js b/functions/tokenservice/functions/test/index.test.js index 45a764366d..2137f52664 100644 --- a/functions/tokenservice/functions/test/index.test.js +++ b/functions/tokenservice/functions/test/index.test.js @@ -1,58 +1,80 @@ const assert = require('assert'); const index = require('../index.js'); -const supertest = require('supertest'); -const request = supertest(process.env.BASE_URL); +const execPromise = require('child-process-promise').exec; +const path = require('path'); +const cwd = path.join(__dirname, '..'); +const BASE_URL = process.env.BASE_URL || 'http://localhost:8080'; +const FF_TIMEOUT = 3000; +let requestRetry = require('requestretry'); -describe('Firebase OAuth Token', () => { - it('should give 400 if no argument is provided.', done => { - request - .get('/getOAuthToken') - .send() - .expect( - 400, - '{"error":{"status":"INVALID_ARGUMENT","message":"Bad Request"}}', - done - ); - }); - it('should hit retrieve credentials API', done => { - const context = (uid = 'test-uid', email_verified = true) => ({ - auth: { - uid, - token: { - firebase: { - email_verified, - }, - }, +// let requestRetryForCredentials = require('requestretry'); + +const defaults = { + uri: `${BASE_URL}/getOAuthToken`, + maxAttempts: 1, + fullResponse: true, +}; + +requestRetry = requestRetry.defaults({ + retryStrategy: requestRetry.RetryStrategies.NetworkError, + method: 'GET', + json: true, + url: `${BASE_URL}/getOAuthToken`, +}); + +const contextValue = (uid = 'test-uid', email_verified = true) => ({ + auth: { + uid, + token: { + firebase: { + email_verified, }, - }); - const result = index.retrieveCredentials(context); - result - .then(doc => { - return doc; - }) - .catch(err => { - console.log('Error in retrieve credentials', err); - return 'Error retrieving token'; - }); - done(); + }, + }, +}); + +describe('getOAuthToken tests', () => { + let ffProc; + before(() => { + ffProc = execPromise( + `functions-framework --target=getOAuthToken --signature-type=http`, + {timeout: FF_TIMEOUT, shell: true, cwd} + ); }); - it('Should throw on no auth', () => { - assert.throws(() => index.getOAuthToken({}), Error); + after(async () => { + try { + await ffProc; + } catch (err) { + // Timeouts always cause errors on Linux, so catch them + if (err.name && err.name === 'ChildProcessError') { + return; + } + throw err; + } }); - it('Should return token', () => { - const context = (uid = 'test-uid', email_verified = true) => ({ - auth: { - uid, - token: { - firebase: { - email_verified, - }, + + describe('Firebase OAuth Token', () => { + // no argument 400 error + it('should give 400 if no Context is provided', async () => { + //new requestRetry(defaults, (error, response, body) => done()) + const response = await requestRetry({ + body: { + deviceID: '', }, - }, + }); + //Context is missing in the input parameter. + assert.strictEqual(response.statusCode, 400); + assert.strictEqual(response.statusMessage, 'Bad Request'); + }); + + it('should give 400 if no deviceID is provided', async () => { + //new requestRetry(defaults, (error, response, body) => done()) + const response = await requestRetry({ + contextValue, + }); + //Context is missing in the input parameter. + assert.strictEqual(response.statusCode, 400); + assert.strictEqual(response.statusMessage, 'Bad Request'); }); - request - .get('/getOAuthToken') - .send(context) - .expect(200); }); }); From 42f672ce0279d203cc00035c1c245ae4ee1c5675 Mon Sep 17 00:00:00 2001 From: "Leah E. Cole" <6719667+leahecole@users.noreply.github.com> Date: Mon, 22 Jul 2019 15:41:45 -0700 Subject: [PATCH 2/5] Remove unused parameter (#1424) --- functions/composer-storage-trigger/index.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/functions/composer-storage-trigger/index.js b/functions/composer-storage-trigger/index.js index 8ebe15e186..ade0ffccc2 100644 --- a/functions/composer-storage-trigger/index.js +++ b/functions/composer-storage-trigger/index.js @@ -53,13 +53,7 @@ exports.triggerDag = async data => { try { const iap = await authorizeIap(CLIENT_ID, PROJECT_ID, USER_AGENT); - return makeIapPostRequest( - WEBSERVER_URL, - BODY, - iap.idToken, - USER_AGENT, - iap.jwt - ); + return makeIapPostRequest(WEBSERVER_URL, BODY, iap.idToken, USER_AGENT); } catch (err) { console.error('Error authorizing IAP:', err.message); throw new Error(err); @@ -142,7 +136,6 @@ const authorizeIap = async (clientId, projectId, userAgent) => { } return { - jwt: jwt, idToken: tokenJson.id_token, }; }; From f5277e4b8025f59f000d7803f34cf0722ca0c6a2 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Tue, 23 Jul 2019 15:56:40 +0300 Subject: [PATCH 3/5] Update dependency @google-cloud/spanner to v4 (#1420) --- functions/spanner/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/spanner/package.json b/functions/spanner/package.json index a343573c79..5bfd0ae5ef 100644 --- a/functions/spanner/package.json +++ b/functions/spanner/package.json @@ -15,7 +15,7 @@ "test": "mocha test/*.test.js --timeout=20000" }, "dependencies": { - "@google-cloud/spanner": "^3.1.0" + "@google-cloud/spanner": "^4.0.0" }, "devDependencies": { "@google-cloud/nodejs-repo-tools": "^3.3.0", From 1f7206dec32b58dd8f2a0c8cd1a43a869f418a36 Mon Sep 17 00:00:00 2001 From: Santhosh Vaddi Date: Tue, 23 Jul 2019 13:23:33 -0700 Subject: [PATCH 4/5] Resolve timeout errors. --- functions/tokenservice/functions/package.json | 6 +-- .../tokenservice/functions/test/index.test.js | 52 +++++++++++++------ 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/functions/tokenservice/functions/package.json b/functions/tokenservice/functions/package.json index 1583ab1590..d48cfabd77 100644 --- a/functions/tokenservice/functions/package.json +++ b/functions/tokenservice/functions/package.json @@ -2,17 +2,16 @@ "name": "functions", "description": "Cloud Functions for Firebase", "scripts": { + "test": "mocha test/*.test.js --timeout=5000", "lint": "eslint .", "serve": "firebase serve --only functions", "shell": "firebase functions:shell", "deploy": "firebase deploy --only functions", "logs": "firebase functions:log", "local-test": "mocha test/index.test.js", - "test": "npm run local-test", "start": "functions-framework --target=getOAuthToken" }, "dependencies": { - "child-process-promise": "^2.2.1", "firebase-admin": "^8.0.0", "firebase-functions": "^3.0.0", "request": "^2.88.0", @@ -22,6 +21,7 @@ "@google-cloud/functions-framework": "^1.1.1", "@google-cloud/nodejs-repo-tools": "^3.3.0", "babel-eslint": "^10.0.2", + "child-process-promise": "^2.2.1", "eslint": "^6.0.0", "eslint-config-prettier": "^6.0.0", "eslint-plugin-node": "^9.1.0", @@ -32,7 +32,7 @@ "supertest": "^4.0.0" }, "engines": { - "node": "8" + "node": ">=8.0.0" }, "private": true } diff --git a/functions/tokenservice/functions/test/index.test.js b/functions/tokenservice/functions/test/index.test.js index 2137f52664..1fdb1f2361 100644 --- a/functions/tokenservice/functions/test/index.test.js +++ b/functions/tokenservice/functions/test/index.test.js @@ -1,3 +1,20 @@ +/** + * Copyright 2019, Google, LLC + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* eslint-env node, mocha */ + const assert = require('assert'); const index = require('../index.js'); const execPromise = require('child-process-promise').exec; @@ -5,10 +22,9 @@ const path = require('path'); const cwd = path.join(__dirname, '..'); const BASE_URL = process.env.BASE_URL || 'http://localhost:8080'; const FF_TIMEOUT = 3000; +const PORT = 8080; let requestRetry = require('requestretry'); -// let requestRetryForCredentials = require('requestretry'); - const defaults = { uri: `${BASE_URL}/getOAuthToken`, maxAttempts: 1, @@ -16,6 +32,7 @@ const defaults = { }; requestRetry = requestRetry.defaults({ + retryDelay: 500, retryStrategy: requestRetry.RetryStrategies.NetworkError, method: 'GET', json: true, @@ -33,30 +50,34 @@ const contextValue = (uid = 'test-uid', email_verified = true) => ({ }, }); +const handleLinuxFailures = async proc => { + try { + return await proc; + } catch (err) { + // Timeouts always cause errors on Linux, so catch them + // Don't return proc, as await-ing it re-throws the error + if (!err.name || err.name !== 'ChildProcessError') { + throw err; + } + } +}; + describe('getOAuthToken tests', () => { let ffProc; before(() => { ffProc = execPromise( - `functions-framework --target=getOAuthToken --signature-type=http`, + `functions-framework --target=getOAuthToken --signature-type=http --port=${PORT}`, {timeout: FF_TIMEOUT, shell: true, cwd} ); }); + after(async () => { - try { - await ffProc; - } catch (err) { - // Timeouts always cause errors on Linux, so catch them - if (err.name && err.name === 'ChildProcessError') { - return; - } - throw err; - } + await handleLinuxFailures(ffProc); }); describe('Firebase OAuth Token', () => { // no argument 400 error it('should give 400 if no Context is provided', async () => { - //new requestRetry(defaults, (error, response, body) => done()) const response = await requestRetry({ body: { deviceID: '', @@ -68,10 +89,7 @@ describe('getOAuthToken tests', () => { }); it('should give 400 if no deviceID is provided', async () => { - //new requestRetry(defaults, (error, response, body) => done()) - const response = await requestRetry({ - contextValue, - }); + const response = await requestRetry(contextValue); //Context is missing in the input parameter. assert.strictEqual(response.statusCode, 400); assert.strictEqual(response.statusMessage, 'Bad Request'); From 57bdbc6798a4921fbb8492ce94f3f00c605b2818 Mon Sep 17 00:00:00 2001 From: Santhosh Vaddi Date: Tue, 23 Jul 2019 14:50:01 -0700 Subject: [PATCH 5/5] Removed .eslintrc.json file and fixed lint issues. --- .../tokenservice/functions/.eslintrc.json | 123 ------------------ .../tokenservice/functions/test/index.test.js | 6 - 2 files changed, 129 deletions(-) delete mode 100644 functions/tokenservice/functions/.eslintrc.json diff --git a/functions/tokenservice/functions/.eslintrc.json b/functions/tokenservice/functions/.eslintrc.json deleted file mode 100644 index ea4352b993..0000000000 --- a/functions/tokenservice/functions/.eslintrc.json +++ /dev/null @@ -1,123 +0,0 @@ -{ - "parserOptions": { - // Required for certain syntax usages - "ecmaVersion": 8 - }, - "plugins": [ - "promise" - ], - "extends": "eslint:recommended", - "rules": { - // Removed rule "disallow the use of console" from recommended eslint rules - "no-console": "off", - - // Removed rule "disallow multiple spaces in regular expressions" from recommended eslint rules - "no-regex-spaces": "off", - - // Removed rule "disallow the use of debugger" from recommended eslint rules - "no-debugger": "off", - - // Removed rule "disallow unused variables" from recommended eslint rules - "no-unused-vars": "off", - - // Removed rule "disallow mixed spaces and tabs for indentation" from recommended eslint rules - "no-mixed-spaces-and-tabs": "off", - - // Removed rule "disallow the use of undeclared variables unless mentioned in /*global */ comments" from recommended eslint rules - "no-undef": "off", - - // Warn against template literal placeholder syntax in regular strings - "no-template-curly-in-string": 1, - - // Warn if return statements do not either always or never specify values - "consistent-return": 1, - - // Warn if no return statements in callbacks of array methods - "array-callback-return": 1, - - // Require the use of === and !== - "eqeqeq": 2, - - // Disallow the use of alert, confirm, and prompt - "no-alert": 2, - - // Disallow the use of arguments.caller or arguments.callee - "no-caller": 2, - - // Disallow null comparisons without type-checking operators - "no-eq-null": 2, - - // Disallow the use of eval() - "no-eval": 2, - - // Warn against extending native types - "no-extend-native": 1, - - // Warn against unnecessary calls to .bind() - "no-extra-bind": 1, - - // Warn against unnecessary labels - "no-extra-label": 1, - - // Disallow leading or trailing decimal points in numeric literals - "no-floating-decimal": 2, - - // Warn against shorthand type conversions - "no-implicit-coercion": 1, - - // Warn against function declarations and expressions inside loop statements - "no-loop-func": 1, - - // Disallow new operators with the Function object - "no-new-func": 2, - - // Warn against new operators with the String, Number, and Boolean objects - "no-new-wrappers": 1, - - // Disallow throwing literals as exceptions - "no-throw-literal": 2, - - // Require using Error objects as Promise rejection reasons - "prefer-promise-reject-errors": 2, - - // Enforce “for” loop update clause moving the counter in the right direction - "for-direction": 2, - - // Enforce return statements in getters - "getter-return": 2, - - // Disallow await inside of loops - "no-await-in-loop": 2, - - // Disallow comparing against -0 - "no-compare-neg-zero": 2, - - // Warn against catch clause parameters from shadowing variables in the outer scope - "no-catch-shadow": 1, - - // Disallow identifiers from shadowing restricted names - "no-shadow-restricted-names": 2, - - // Enforce return statements in callbacks of array methods - "callback-return": 2, - - // Require error handling in callbacks - "handle-callback-err": 2, - - // Warn against string concatenation with __dirname and __filename - "no-path-concat": 1, - - // Prefer using arrow functions for callbacks - "prefer-arrow-callback": 1, - - // Return inside each then() to create readable and reusable Promise chains. - // Forces developers to return console logs and http calls in promises. - "promise/always-return": 2, - - //Enforces the use of catch() on un-returned promises - "promise/catch-or-return": 2, - - // Warn against nested then() or catch() statements - "promise/no-nesting": 1 - } -} diff --git a/functions/tokenservice/functions/test/index.test.js b/functions/tokenservice/functions/test/index.test.js index 8d15dd39d7..1bd2adf90b 100644 --- a/functions/tokenservice/functions/test/index.test.js +++ b/functions/tokenservice/functions/test/index.test.js @@ -16,7 +16,6 @@ /* eslint-env node, mocha */ const assert = require('assert'); -const index = require('../index.js'); const execPromise = require('child-process-promise').exec; const path = require('path'); const cwd = path.join(__dirname, '..'); @@ -24,11 +23,6 @@ const BASE_URL = process.env.BASE_URL || 'http://localhost:8080'; const FF_TIMEOUT = 3000; const PORT = 8080; let requestRetry = require('requestretry'); -const defaults = { - uri: `${BASE_URL}/getOAuthToken`, - maxAttempts: 1, - fullResponse: true, -}; requestRetry = requestRetry.defaults({ retryDelay: 500,