Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add "crossorigin" attribute to remote xhr elements to attach id token. #21107

Merged
merged 10 commits into from
Mar 14, 2019
32 changes: 26 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,26 @@ 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)
);
// return this.fetch_(ampdoc, element, policy, opt_refresh);
hellokoji marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @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'});
});
});
});
5 changes: 5 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,11 @@ tags: { # <amp-state>
attrs: {
name: "cross-origin"
value: "amp-viewer-auth-token-via-post"
deprecation: "crossorigin"
}
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) {
hellokoji marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -926,15 +927,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 @@ -966,6 +967,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