Skip to content

Commit

Permalink
✨ Add "crossorigin" attribute to remote xhr elements to attach id tok…
Browse files Browse the repository at this point in the history
…en. (#21107)

* Add "cross-origin" attribute to remote xhr elements to attach id token.

Change-Id: If2c56a3cc32ccfe97efb64572dbaf86fdb1d54d0

* Lint.

Change-Id: Ia74748cea663883663f0121e4fda9bed0f7b9d8d

* Address check-types, and presubmits.

Change-Id: If19b962e606828b6efd4308b5870480be1c973e9

* Address PR Comments 1.

Change-Id: I8e9410492f018bdbf9702e8cf37e8c1b1a2a9a51

* Fix tests.

Change-Id: Iea2f29fc25ba9de982eb1cc4bc3c3d9459c754ee

* Update validator rules to support crossorigin.

Change-Id: I6859a879e1bee170627f1c06fd473da0bc3d2fd5

* Add deprecation URL and update validator tests.

Change-Id: I510c0778a60db259486d55b116961d1173198402

* Address PR Comments 2.

Change-Id: Id301b039be182e4c3f92de3b790357edea21a340

* Change serviceForDoc impl to getElementServiceIfAvailableForDoc.

Change-Id: I3b04db719fd41f72475e0686aa71e6dd5a0b43fe
  • Loading branch information
hellokoji authored and William Chou committed Mar 14, 2019
1 parent 8542299 commit 76a0c2c
Show file tree
Hide file tree
Showing 20 changed files with 417 additions and 53 deletions.
31 changes: 25 additions & 6 deletions extensions/amp-bind/0.1/amp-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
} from '../../../src/batched-json';
import {dev, devAssert, userAssert} from '../../../src/log';
import {getSourceOrigin} from '../../../src/url';
import {getViewerAuthTokenIfAvailable} from '../../../src/utils/xhr-utils';
import {isJsonScriptTag} from '../../../src/dom';
import {map} from '../../../src/utils/object';
import {toggle} from '../../../src/style';
Expand Down Expand Up @@ -122,11 +123,26 @@ export class AmpState extends AMP.BaseElement {
* Wrapper to stub during testing.
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
* @param {!Element} element
* @param {!UrlReplacementPolicy} policy
* @param {boolean=} opt_refresh
* @param {string=} token
* @return {!Promise<!JsonObject|!Array<JsonObject>>}
* @private
*/
fetch_(ampdoc, element, policy, opt_refresh, token = undefined) {
return batchFetchJsonFor(ampdoc, element, /* opt_expr */ undefined, policy,
opt_refresh, token);
}

/**
* Transforms and prepares the fetch request.
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
* @param {!Element} element
* @param {boolean} isInit
* @param {boolean=} opt_refresh
* @return {!Promise}
* @return {!Promise<!JsonObject|!Array<JsonObject>>}
*/
fetch_(ampdoc, element, isInit, opt_refresh) {
prepareAndSendFetch_(ampdoc, element, isInit, opt_refresh) {
const src = element.getAttribute('src');
// Require opt-in for URL variable replacements on CORS fetches triggered
// by [src] mutation. @see spec/amp-var-substitutions.md
Expand All @@ -135,22 +151,25 @@ export class AmpState extends AMP.BaseElement {
(getSourceOrigin(src) == getSourceOrigin(ampdoc.win.location))) {
policy = UrlReplacementPolicy.ALL;
}
return batchFetchJsonFor(
ampdoc, element, /* opt_expr */ undefined, policy, opt_refresh);

return getViewerAuthTokenIfAvailable(ampdoc.win, element).then(token =>
this.fetch_(ampdoc, element, policy, opt_refresh, token)
);
}

/**
* @param {boolean} isInit
* @param {boolean=} opt_refresh
* @return {!Promise}
* @return {!Promise<undefined>}
* @private
*/
fetchAndUpdate_(isInit, opt_refresh) {
const ampdoc = this.getAmpDoc();
// Don't fetch in prerender mode.
const viewer = Services.viewerForDoc(this.element);
return viewer.whenFirstVisible()
.then(() => this.fetch_(ampdoc, this.element, isInit, opt_refresh))
.then(() => this.prepareAndSendFetch_(
ampdoc, this.element, isInit, opt_refresh))
.then(json => this.updateState_(json, isInit));
}

Expand Down
50 changes: 49 additions & 1 deletion extensions/amp-bind/0.1/test/test-amp-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import '../amp-bind';
import * as xhrUtils from '../../../../src/utils/xhr-utils';
import {ActionTrust} from '../../../../src/action-constants';
import {Services} from '../../../../src/services';

Expand All @@ -26,6 +27,8 @@ describes.realWin('AmpState', {
}, env => {
let win;
let sandbox;
let fetchStub;
let getViewerAuthTokenIfAvailableStub;

let element;
let ampState;
Expand Down Expand Up @@ -57,8 +60,11 @@ describes.realWin('AmpState', {
ampState = element.implementation_;

sandbox.spy(ampState, 'fetchAndUpdate_');
sandbox.spy(ampState, 'prepareAndSendFetch_');
sandbox.stub(ampState, 'updateState_');
sandbox.stub(ampState, 'fetch_')
getViewerAuthTokenIfAvailableStub = sandbox.stub(
xhrUtils, 'getViewerAuthTokenIfAvailable').returns(Promise.resolve());
fetchStub = sandbox.stub(ampState, 'fetch_')
.returns(Promise.resolve({baz: 'qux'}));
});

Expand All @@ -73,6 +79,8 @@ describes.realWin('AmpState', {
whenFirstVisiblePromiseResolve();
return whenFirstVisiblePromise.then(() => {
expect(ampState.fetchAndUpdate_).calledWithExactly(/* isInit */ true);
return getViewerAuthTokenIfAvailableStub();
}).then(() => {
return ampState.fetch_();
}).then(() => {
expect(ampState.updateState_).calledWithMatch({baz: 'qux'});
Expand Down Expand Up @@ -131,6 +139,8 @@ describes.realWin('AmpState', {
return whenFirstVisiblePromise.then(() => {
expect(ampState.updateState_).calledWithMatch({foo: 'bar'});
expect(ampState.fetchAndUpdate_).calledWithExactly(/* isInit */ true);
return getViewerAuthTokenIfAvailableStub();
}).then(() => {
return ampState.fetch_();
}).then(() => {
expect(ampState.updateState_).calledWithMatch({baz: 'qux'});
Expand Down Expand Up @@ -162,9 +172,47 @@ describes.realWin('AmpState', {

whenFirstVisiblePromiseResolve();
return whenFirstVisiblePromise
.then(() => getViewerAuthTokenIfAvailableStub())
.then(() => ampState.fetch_())
.then(() => {
expect(ampState.updateState_).calledWithMatch({baz: 'qux'});
});
});

it('should fetch with auth token if `crossorigin` attribute exists'
+ ' with `amp-viewer-auth-token-via-post`', () => {
sandbox.stub(viewer, 'hasBeenVisible').returns(false);
getViewerAuthTokenIfAvailableStub.returns(Promise.resolve('idToken'));

element.setAttribute('src', 'https://foo.com/bar?baz=1');
element.setAttribute('crossorigin', 'amp-viewer-auth-token-via-post');
element.build();

// IMPORTANT: No CORS fetch should happen until viewer is visible.
expect(ampState.fetchAndUpdate_).to.have.been.calledOnce;
expect(ampState.fetch_).to.not.have.been.called;

allowConsoleError(() => {
element.mutatedAttributesCallback({src: 'https://foo.com/bar?baz=1'});
});

expect(ampState.fetchAndUpdate_).to.have.been.calledOnce;
expect(ampState.fetch_).to.not.have.been.called;

viewer.hasBeenVisible.returns(true);
element.mutatedAttributesCallback({src: 'https://foo.com/bar?baz=1'});

expect(ampState.fetchAndUpdate_).to.have.been.calledTwice;
expect(ampState.fetch_).to.not.have.been.called;

whenFirstVisiblePromiseResolve();
return whenFirstVisiblePromise
.then(() => ampState.prepareAndSendFetch_({win}, element))
.then(() => {
expect(fetchStub).to.have.been.called;
expect(fetchStub.firstCall.args.slice(-1).pop())
.to.be.equal('idToken');
expect(ampState.updateState_).calledWithMatch({baz: 'qux'});
});
});
});
6 changes: 6 additions & 0 deletions extensions/amp-bind/validator-amp-bind.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ tags: { # <amp-state>
attrs: {
name: "cross-origin"
value: "amp-viewer-auth-token-via-post"
deprecation: "crossorigin"
deprecation_url: "https://github.com/ampproject/amphtml/issues/21399"
}
attrs: {
name: "crossorigin"
value: "amp-viewer-auth-token-via-post"
}
attrs: {
name: "overridable"
Expand Down
38 changes: 25 additions & 13 deletions extensions/amp-form/0.1/amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,14 @@ import {
isCheckValiditySupported,
} from './form-validators';
import {getMode} from '../../../src/mode';
import {installFormProxy} from './form-proxy';
import {installStylesForDoc} from '../../../src/style-installer';
import {
getViewerAuthTokenIfAvailable,
setupAMPCors,
setupInit,
setupInput,
} from '../../../src/utils/xhr-utils';
import {installFormProxy} from './form-proxy';
import {installStylesForDoc} from '../../../src/style-installer';
import {toArray, toWin} from '../../../src/types';
import {triggerAnalyticsEvent} from '../../../src/analytics';

Expand Down Expand Up @@ -244,7 +245,7 @@ export class AmpForm {
* @param {string} method
* @param {!Object<string, string>=} opt_extraFields
* @param {!Array<string>=} opt_fieldBlacklist
* @return {!FetchRequestDef}
* @return {!Promise<!FetchRequestDef>}
*/
requestForFormFetch(url, method, opt_extraFields, opt_fieldBlacklist) {
let xhrUrl, body;
Expand Down Expand Up @@ -275,7 +276,9 @@ export class AmpForm {
body.append(key, opt_extraFields[key]);
}
}
return {

/** @type {!FetchRequestDef}*/
const request = {
xhrUrl,
fetchOpt: dict({
'body': body,
Expand All @@ -284,6 +287,15 @@ export class AmpForm {
'headers': dict({'Accept': 'application/json'}),
}),
};

return getViewerAuthTokenIfAvailable(this.win_, this.form_).then(token => {
if (token) {
userAssert(request.fetchOpt['method'] == 'POST',
'Cannot attach auth token with GET request.');
body.append('ampViewerAuthToken', token);
}
return request;
});
}

/**
Expand Down Expand Up @@ -657,12 +669,12 @@ export class AmpForm {
// Render template for the form submitting state.
const values = this.getFormAsObject_();
return this.renderTemplate_(values)
.then(() => {
this.actions_.trigger(
this.form_, FormEvents.SUBMIT, /* event */ null, trust);
}).then(() => {
request = this.requestForFormFetch(
dev().assertString(this.xhrAction_), this.method_);
.then(() => this.actions_.trigger(
this.form_, FormEvents.SUBMIT, /* event */ null, trust))
.then(() => this.requestForFormFetch(
dev().assertString(this.xhrAction_), this.method_))
.then(formRequest => {
request = formRequest;
request.fetchOpt = setupInit(request.fetchOpt);
request.fetchOpt = setupAMPCors(
this.win_, request.xhrUrl, request.fetchOpt);
Expand Down Expand Up @@ -812,9 +824,9 @@ export class AmpForm {
*/
doXhr_(url, method, opt_extraFields, opt_fieldBlacklist) {
this.assertSsrTemplate_(false, 'XHRs should be proxied.');
const request = this.requestForFormFetch(
url, method, opt_extraFields, opt_fieldBlacklist);
return this.xhr_.fetch(request.xhrUrl, request.fetchOpt);
return this.requestForFormFetch(
url, method, opt_extraFields, opt_fieldBlacklist)
.then(request => this.xhr_.fetch(request.xhrUrl, request.fetchOpt));
}

/**
Expand Down
38 changes: 38 additions & 0 deletions extensions/amp-form/0.1/test/test-amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -2336,6 +2336,44 @@ describes.repeated('', {
});
});

it('should attach auth token with crossorigin attribute', () => {
sandbox.stub(Services, 'viewerAssistanceForDocOrNull').returns(
Promise.resolve({
getIdTokenPromise: (() => Promise.resolve('idToken')),
}));
return getAmpForm(getForm()).then(ampForm => {
const form = ampForm.form_;
form.id = 'registration';

const emailInput = createElement('input');
emailInput.setAttribute('name', 'email');
emailInput.setAttribute('type', 'email');
emailInput.setAttribute('value', 'j@hnmiller.com');
form.appendChild(emailInput);

const unnamedInput = createElement('input');
unnamedInput.setAttribute('type', 'text');
unnamedInput.setAttribute('value', 'unnamed');
form.appendChild(unnamedInput);

ampForm.method_ = 'POST';
ampForm.form_.setAttribute(
'crossorigin', 'amp-viewer-auth-token-via-post');
sandbox.stub(ampForm.xhr_, 'fetch').returns(
Promise.resolve({json: (() => Promise.resolve())}));

return ampForm.handleSubmitAction_(/* invocation */ {}).then(() => {
return ampForm.xhrSubmitPromiseForTesting().then(() => {
expect(Services.viewerAssistanceForDocOrNull).to.be.called;
const fetchCallFormData =
ampForm.xhr_.fetch.firstCall.args[1].body.getFormData();
expect(fetchCallFormData.get('ampViewerAuthToken'))
.to.equal('idToken');
});
});
});
});

describe('Async Inputs', () => {

it('NonXHRGet should submit sync if no Async Input', () => {
Expand Down
36 changes: 24 additions & 12 deletions extensions/amp-list/0.1/amp-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ import {dict} from '../../../src/utils/object';
import {getMode} from '../../../src/mode';
import {getSourceOrigin} from '../../../src/url';
import {getValueForExpr} from '../../../src/json';
import {
getViewerAuthTokenIfAvailable,
setupAMPCors,
setupInput,
setupJsonFetchInit,
} from '../../../src/utils/xhr-utils';
import {
installOriginExperimentsForDoc,
originExperimentsForDoc,
Expand All @@ -43,11 +49,6 @@ import {isArray, toArray} from '../../../src/types';
import {isExperimentOn} from '../../../src/experiments';
import {px, setStyles, toggle} from '../../../src/style';
import {removeChildren} from '../../../src/dom';
import {
setupAMPCors,
setupInput,
setupJsonFetchInit,
} from '../../../src/utils/xhr-utils';

/** @const {string} */
const TAG = 'amp-list';
Expand Down Expand Up @@ -413,7 +414,7 @@ export class AmpList extends AMP.BaseElement {
fetch = this.ssrTemplate_(opt_refresh);
} else {
const itemsExpr = this.element.getAttribute('items') || 'items';
fetch = this.fetch_(opt_refresh).then(data => {
fetch = this.prepareAndSendFetch_(opt_refresh).then(data => {
let items = data;
if (itemsExpr != '.') {
items = getValueForExpr(/**@type {!JsonObject}*/ (data), itemsExpr);
Expand Down Expand Up @@ -928,15 +929,15 @@ export class AmpList extends AMP.BaseElement {
});
}


/**
* @param {boolean} opt_refresh
* @return {!Promise<(!Array<JsonObject>|!JsonObject)>}
* @param {boolean=} opt_refresh
* @param {string=} token
* @return {!Promise<!JsonObject|!Array<JsonObject>>}
* @private
*/
fetch_(opt_refresh = false) {
return batchFetchJsonFor(
this.getAmpDoc(), this.element, '.', this.getPolicy_(), opt_refresh);
fetch_(opt_refresh = false, token = undefined) {
return batchFetchJsonFor(this.getAmpDoc(), this.element, '.',
this.getPolicy_(), opt_refresh, token);
}

/**
Expand Down Expand Up @@ -968,6 +969,17 @@ export class AmpList extends AMP.BaseElement {

}

/**
* @param {boolean=} opt_refresh
* @return {!Promise<!JsonObject|!Array<JsonObject>>}
* @private
*/
prepareAndSendFetch_(opt_refresh = false) {
return getViewerAuthTokenIfAvailable(this.win, this.element).then(token =>
this.fetch_(opt_refresh, token)
);
}

/**
* @return {!UrlReplacementPolicy}
*/
Expand Down

0 comments on commit 76a0c2c

Please sign in to comment.