From 0062a61326347ad42e32e00cf6d214bca212ef1a Mon Sep 17 00:00:00 2001 From: Hongfei Ding Date: Thu, 24 Aug 2017 10:11:56 -0700 Subject: [PATCH] Support custom scope and api key for CID API (#11046) * Support custom scope and api key for CID API * Fix tests * Fix presubmit * Address comments * Address comments --- src/service/cid-api.js | 25 +------- src/service/cid-impl.js | 7 +- src/service/viewer-cid-api.js | 88 +++++++++++++++++++------- test/functional/test-cid-api.js | 36 +++++------ test/functional/test-cid.js | 8 +-- test/functional/test-viewer-cid-api.js | 65 +++++++++++++++---- 6 files changed, 139 insertions(+), 90 deletions(-) diff --git a/src/service/cid-api.js b/src/service/cid-api.js index c1f683cfff6c..cb8f809f6f71 100644 --- a/src/service/cid-api.js +++ b/src/service/cid-api.js @@ -22,9 +22,6 @@ import {isProxyOrigin} from '../url'; import {WindowInterface} from '../window-interface'; const GOOGLE_API_URL = 'https://ampcid.google.com/v1/publisher:getClientId?key='; -const API_KEYS = { - 'googleanalytics': 'AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM', -}; const TAG = 'GoogleCidApi'; const AMP_TOKEN = 'AMP_TOKEN'; @@ -64,16 +61,11 @@ export class GoogleCidApi { } /** - * @param {string} apiClient + * @param {string} apiKey * @param {string} scope * @return {!Promise} */ - getScopedCid(apiClient, scope) { - const url = this.getUrl_(apiClient); - if (!url) { - return Promise.resolve(/** @type {?string} */(null)); - } - + getScopedCid(apiKey, scope) { if (this.cidPromise_[scope]) { return this.cidPromise_[scope]; } @@ -100,6 +92,7 @@ export class GoogleCidApi { if (!token || this.isStatusToken_(token)) { this.persistToken_(TokenStatus.RETRIEVING, TIMEOUT); } + const url = GOOGLE_API_URL + apiKey; return this.fetchCid_(dev().assertString(url), scope, token) .then(this.handleResponse_.bind(this)) .catch(e => { @@ -152,18 +145,6 @@ export class GoogleCidApi { } } - /** - * @param {string} apiClient - * @return {?string} - */ - getUrl_(apiClient) { - const key = API_KEYS[apiClient]; - if (!key) { - return null; - } - return GOOGLE_API_URL + key; - } - /** * @param {string|undefined} tokenValue * @param {number} expires diff --git a/src/service/cid-impl.js b/src/service/cid-impl.js index e6fb350780cd..fb58f412d8f9 100644 --- a/src/service/cid-impl.js +++ b/src/service/cid-impl.js @@ -177,10 +177,9 @@ export class Cid { /** @const {!Location} */ const url = parseUrl(this.ampdoc.win.location.href); if (!isProxyOrigin(url)) { - const apiClient = - ViewerCidApi.scopeOptedInForCidApi(this.ampdoc.win, scope); - if (apiClient) { - return this.cidApi_.getScopedCid(apiClient, scope).then(scopedCid => { + const apiKey = this.viewerCidApi_.isScopeOptedIn(scope); + if (apiKey) { + return this.cidApi_.getScopedCid(apiKey, scope).then(scopedCid => { if (scopedCid == TokenStatus.OPT_OUT) { return null; } diff --git a/src/service/viewer-cid-api.js b/src/service/viewer-cid-api.js index 31c65b7c6778..18eac240a957 100644 --- a/src/service/viewer-cid-api.js +++ b/src/service/viewer-cid-api.js @@ -16,40 +16,32 @@ import {Services} from '../services'; import {dict} from '../utils/object'; +import {user} from '../log'; const GOOGLE_CLIENT_ID_API_META_NAME = 'amp-google-client-id-api'; const CID_API_SCOPE_WHITELIST = { 'googleanalytics': 'AMP_ECID_GOOGLE', }; +const API_KEYS = { + 'googleanalytics': 'AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM', +}; +const TAG = 'ViewerCidApi'; /** * Exposes CID API if provided by the Viewer. */ export class ViewerCidApi { - /** - * @param {!Window} win - * @param {string} scope - * @return {?string} - */ - static scopeOptedInForCidApi(win, scope) { - const optInMeta = win.document.head./*OK*/querySelector( - `meta[name=${GOOGLE_CLIENT_ID_API_META_NAME}]`); - if (!optInMeta || !optInMeta.hasAttribute('content')) { - return null; - } - const whiteListedClients = optInMeta.getAttribute('content').split(','); - for (let i = 0; i < whiteListedClients.length; ++i) { - if (CID_API_SCOPE_WHITELIST[whiteListedClients[i]] === scope) { - return whiteListedClients[i]; - } - } - return null; - } - constructor(ampdoc) { + + /** @private {!./ampdoc-impl.AmpDoc} */ this.ampdoc_ = ampdoc; + + /** @private {!./viewer-impl.Viewer} */ this.viewer_ = Services.viewerForDoc(this.ampdoc_); + + /** @private {?Object} */ + this.apiKeyMap_ = null; } /** @@ -69,10 +61,58 @@ export class ViewerCidApi { * @return {!Promise} */ getScopedCid(scope) { - return this.viewer_.sendMessageAwaitResponse('cid', dict({ + const apiKey = this.isScopeOptedIn(scope); + const payload = dict({ 'scope': scope, - 'clientIdApi': - !!ViewerCidApi.scopeOptedInForCidApi(this.ampdoc_.win, scope), - })); + 'clientIdApi': !!apiKey, + }); + if (apiKey) { + payload['apiKey'] = apiKey; + } + return this.viewer_.sendMessageAwaitResponse('cid', payload); + } + + /** + * Checks if the page has opted in CID API for the given scope. + * Returns the API key that should be used, or null if page hasn't opted in. + * + * @param {string} scope + * @return {string|undefined} + */ + isScopeOptedIn(scope) { + if (!this.apiKeyMap_) { + this.apiKeyMap_ = this.getOptedInScopes_(); + } + return this.apiKeyMap_[scope]; + } + + /** + * @return {!Object} + */ + getOptedInScopes_() { + const apiKeyMap = {}; + const optInMeta = this.ampdoc_.win.document.head./*OK*/querySelector( + `meta[name=${GOOGLE_CLIENT_ID_API_META_NAME}]`); + if (optInMeta && optInMeta.hasAttribute('content')) { + const list = optInMeta.getAttribute('content').split(','); + list.forEach(item => { + item = item.trim(); + if (item.indexOf('=') > 0) { + const pair = item.split('='); + const scope = pair[0].trim(); + apiKeyMap[scope] = pair[1].trim(); + } else { + const clientName = item; + const scope = CID_API_SCOPE_WHITELIST[clientName]; + if (scope) { + apiKeyMap[scope] = API_KEYS[clientName]; + } else { + user().error(TAG, + `Unsupported client for Google CID API: ${clientName}`); + } + } + }); + } + return apiKeyMap; } } diff --git a/test/functional/test-cid-api.js b/test/functional/test-cid-api.js index 87a043006039..2db8be6e1611 100644 --- a/test/functional/test-cid-api.js +++ b/test/functional/test-cid-api.js @@ -57,11 +57,11 @@ describes.realWin('test-cid-api', {}, env => { }; }, })); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.equal('amp-12345'); expect(getCookie(win, 'AMP_TOKEN')).to.equal('amp-token-123'); expect(fetchJsonStub).to.be.calledWith( - 'https://ampcid.google.com/v1/publisher:getClientId?key=AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM', + 'https://ampcid.google.com/v1/publisher:getClientId?key=api-key', { method: 'POST', ampCors: false, @@ -83,11 +83,11 @@ describes.realWin('test-cid-api', {}, env => { }; }, })); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.equal('amp-12345'); expect(getCookie(win, 'AMP_TOKEN')).to.equal('amp-token-123'); expect(fetchJsonStub).to.be.calledWith( - 'https://ampcid.google.com/v1/publisher:getClientId?key=AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM', + 'https://ampcid.google.com/v1/publisher:getClientId?key=api-key', { method: 'POST', ampCors: false, @@ -110,7 +110,7 @@ describes.realWin('test-cid-api', {}, env => { }; }, })); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.equal('$OPT_OUT'); expect(getCookie(win, 'AMP_TOKEN')).to.equal('$OPT_OUT'); }); @@ -120,7 +120,7 @@ describes.realWin('test-cid-api', {}, env => { fetchJsonStub.returns(Promise.resolve({ json: () => {return {};}, })); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.be.null; expect(getCookie(win, 'AMP_TOKEN')).to.equal('$NOT_FOUND'); }); @@ -128,7 +128,7 @@ describes.realWin('test-cid-api', {}, env => { it('should return null if API rejects', () => { fetchJsonStub.returns(Promise.reject()); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.be.null; expect(getCookie(win, 'AMP_TOKEN')).to.equal('$ERROR'); }); @@ -136,7 +136,7 @@ describes.realWin('test-cid-api', {}, env => { it('should return null if AMP_TOKEN=$ERROR', () => { persistCookie('AMP_TOKEN', '$ERROR'); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.be.null; expect(getCookie(win, 'AMP_TOKEN')).to.equal('$ERROR'); }); @@ -146,7 +146,7 @@ describes.realWin('test-cid-api', {}, env => { persistCookie('AMP_TOKEN', '$NOT_FOUND'); const windowInterface = mockWindowInterface(env.sandbox); windowInterface.getDocumentReferrer.returns('https://example.org/'); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.be.null; expect(getCookie(win, 'AMP_TOKEN')).to.equal('$NOT_FOUND'); }); @@ -165,7 +165,7 @@ describes.realWin('test-cid-api', {}, env => { const windowInterface = mockWindowInterface(env.sandbox); windowInterface.getDocumentReferrer.returns('https://cdn.ampproject.org/'); persistCookie('AMP_TOKEN', '$NOT_FOUND'); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.equal('amp-12345'); expect(getCookie(win, 'AMP_TOKEN')).to.equal('amp-token-123'); }); @@ -173,24 +173,18 @@ describes.realWin('test-cid-api', {}, env => { it('should return $OPT_OUT if AMP_TOKEN=$OPT_OUT ', () => { persistCookie('AMP_TOKEN', '$OPT_OUT'); - return api.getScopedCid('googleanalytics', 'scope-a').then(cid => { + return api.getScopedCid('api-key', 'scope-a').then(cid => { expect(cid).to.equal('$OPT_OUT'); expect(getCookie(win, 'AMP_TOKEN')).to.equal('$OPT_OUT'); }); }); - it('should return null if apiClient is not supported', () => { - return api.getScopedCid('non-supported', 'scope-a').then(cid => { - expect(cid).to.be.null; - }); - }); - it('should not send another request if one is already out', () => { let responseResolver; fetchJsonStub.returns(new Promise(res => {responseResolver = res;})); - const promise1 = api.getScopedCid('googleanalytics', 'scope-a'); - const promise2 = api.getScopedCid('googleanalytics', 'scope-a'); + const promise1 = api.getScopedCid('api-key', 'scope-a'); + const promise2 = api.getScopedCid('api-key', 'scope-a'); responseResolver({ json: () => { @@ -214,8 +208,8 @@ describes.realWin('test-cid-api', {}, env => { .returns(new Promise(res => {responseResolverA = res;})); fetchJsonStub.onCall(1) .returns(new Promise(res => {responseResolverB = res;})); - const promiseA = api.getScopedCid('googleanalytics', 'scope-a'); - const promiseB = api.getScopedCid('googleanalytics', 'scope-b'); + const promiseA = api.getScopedCid('api-key', 'scope-a'); + const promiseB = api.getScopedCid('api-key', 'scope-b'); responseResolverA({ json: () => { diff --git a/test/functional/test-cid.js b/test/functional/test-cid.js index 3e40992e177a..e0d099161e1d 100644 --- a/test/functional/test-cid.js +++ b/test/functional/test-cid.js @@ -21,7 +21,6 @@ import { optOutOfCid, isOptedOutOfCid, } from '../../src/service/cid-impl'; -import {ViewerCidApi} from '../../src/service/viewer-cid-api'; import {installCryptoService, Crypto} from '../../src/service/crypto-impl'; import {installDocService} from '../../src/service/ampdoc-impl'; import {installDocumentStateService} from '../../src/service/document-state'; @@ -128,7 +127,6 @@ describe('cid', () => { }); sandbox.stub(viewer, 'isTrustedViewer', () => Promise.resolve(trustedViewer)); - sandbox.stub(ViewerCidApi, 'scopeOptedInForCidApi', () => null); viewerSendMessageStub = sandbox.stub(viewer, 'sendMessageAwaitResponse', (eventType, opt_data) => { if (eventType != 'cid') { @@ -144,6 +142,7 @@ describe('cid', () => { }); cid = cidServiceForDocForTesting(ampdoc); + sandbox.stub(cid.viewerCidApi_, 'isScopeOptedIn', () => null); installCryptoService(fakeWin); crypto = Services.cryptoFor(fakeWin); }); @@ -778,8 +777,7 @@ describes.realWin('cid', {amp: true}, env => { beforeEach(() => { sandbox.stub(url, 'isProxyOrigin').returns(false); - sandbox.stub(ViewerCidApi, 'scopeOptedInForCidApi') - .returns('googleanalytics'); + sandbox.stub(cid.viewerCidApi_, 'isScopeOptedIn').returns('api-key'); setCookie(win, '_ga', '', 0); }); @@ -796,7 +794,7 @@ describes.realWin('cid', {amp: true}, env => { createCookieIfNotPresent: true, }, hasConsent).then(scopedCid => { expect(getScopedCidStub) - .to.be.calledWith('googleanalytics', 'AMP_ECID_GOOGLE'); + .to.be.calledWith('api-key', 'AMP_ECID_GOOGLE'); expect(scopedCid).to.equal('cid-from-api'); expect(getCookie(win, '_ga')).to.equal('cid-from-api'); }); diff --git a/test/functional/test-viewer-cid-api.js b/test/functional/test-viewer-cid-api.js index 7d3da0a01f8e..aa71ae9ef5d9 100644 --- a/test/functional/test-viewer-cid-api.js +++ b/test/functional/test-viewer-cid-api.js @@ -59,12 +59,20 @@ describes.realWin('viewerCidApi', {amp: true}, env => { describe('getScopedCid', () => { function verifyClientIdApiInUse(used) { - viewerMock.sendMessageAwaitResponse.withArgs('cid', dict({ - 'scope': 'AMP_ECID_GOOGLE', - 'clientIdApi': used, - })).returns(Promise.resolve('client-id-from-viewer')); - return expect(api.getScopedCid('AMP_ECID_GOOGLE')) - .to.eventually.equal('client-id-from-viewer'); + viewerMock.sendMessageAwaitResponse + .returns(Promise.resolve('client-id-from-viewer')); + return api.getScopedCid('AMP_ECID_GOOGLE').then(cid => { + expect(cid).to.equal('client-id-from-viewer'); + const payload = dict({ + 'scope': 'AMP_ECID_GOOGLE', + 'clientIdApi': used, + }); + if (used) { + payload['apiKey'] = 'AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM'; + } + expect(viewerMock.sendMessageAwaitResponse) + .to.be.calledWith('cid', payload); + }); } it('should use client ID API from api if everything great', () => { @@ -97,10 +105,7 @@ describes.realWin('viewerCidApi', {amp: true}, env => { it('should return undefined if Viewer returns undefined', () => { ampdoc.win.document.head.innerHTML += ''; - viewerMock.sendMessageAwaitResponse.withArgs('cid', dict({ - 'scope': 'AMP_ECID_GOOGLE', - 'clientIdApi': true, - })).returns(Promise.resolve()); + viewerMock.sendMessageAwaitResponse.returns(Promise.resolve()); return expect(api.getScopedCid('AMP_ECID_GOOGLE')) .to.eventually.be.undefined; }); @@ -108,12 +113,44 @@ describes.realWin('viewerCidApi', {amp: true}, env => { it('should reject if Viewer rejects', () => { ampdoc.win.document.head.innerHTML += ''; - viewerMock.sendMessageAwaitResponse.withArgs('cid', dict({ - 'scope': 'AMP_ECID_GOOGLE', - 'clientIdApi': true, - })).returns(Promise.reject('Client API error')); + viewerMock.sendMessageAwaitResponse + .returns(Promise.reject('Client API error')); return expect(api.getScopedCid('AMP_ECID_GOOGLE')) .to.eventually.be.rejectedWith(/Client API error/); }); }); + + describe('isScopeOptedIn', () => { + it('should read predefined clients and custom API keys correctly', () => { + ampdoc.win.document.head.innerHTML += + ''; + expect(api.isScopeOptedIn('AMP_ECID_GOOGLE')) + .to.equal('AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM'); + expect(api.isScopeOptedIn('foo')).to.equal('foo-api-key'); + expect(api.isScopeOptedIn('bar')).to.equal('bar-api-key'); + expect(api.isScopeOptedIn('hello')).to.equal('hello-api-key'); + expect(api.isScopeOptedIn('non-existing')).to.be.undefined; + }); + + it('should work if meta only contains predefined clients', () => { + ampdoc.win.document.head.innerHTML += + ''; + expect(api.isScopeOptedIn('AMP_ECID_GOOGLE')) + .to.equal('AIzaSyA65lEHUEizIsNtlbNo-l2K18dT680nsaM'); + }); + + it('should work if meta only contains custom scopes', () => { + ampdoc.win.document.head.innerHTML += + ''; + expect(api.isScopeOptedIn('foo')).to.equal('foo-api-key'); + expect(api.isScopeOptedIn('bar')).to.equal('bar-api-key'); + }); + }); });