diff --git a/extensions/amp-access-laterpay/0.1/laterpay-impl.js b/extensions/amp-access-laterpay/0.1/laterpay-impl.js index 4f7905499a941..8763faf56b49e 100644 --- a/extensions/amp-access-laterpay/0.1/laterpay-impl.js +++ b/extensions/amp-access-laterpay/0.1/laterpay-impl.js @@ -219,7 +219,7 @@ export class LaterpayVendor { const urlPromise = this.accessService_.buildUrl( url, /* useAuthData */ false); return urlPromise.then(url => { - return this.accessService_.buildLoginUrl(url); + return this.accessService_.getLoginUrl(url); }).then(url => { dev().info(TAG, 'Authorization URL: ', url); return this.timer_.timeoutPromise( @@ -465,17 +465,11 @@ export class LaterpayVendor { */ handlePurchase_(ev, purchaseUrl) { ev.preventDefault(); - const configuredUrl = purchaseUrl + - '?return_url=RETURN_URL' + - '&article_url=SOURCE_URL' + - '&_reader_id=READER_ID'; - const urlPromise = this.accessService_.buildUrl( - configuredUrl, /* useAuthData */ false); - return urlPromise.then(url => { - dev().fine(TAG, 'Authorization URL: ', url); - this.accessService_.loginWithUrl( - url, this.selectedPurchaseOption_.dataset.purchaseType); - }); + dev().fine(TAG, 'Authorization URL: ', purchaseUrl); + this.accessService_.loginWithUrl( + purchaseUrl, + this.selectedPurchaseOption_.dataset.purchaseType, + /* noReturnUrl */ true); } /** diff --git a/extensions/amp-access/0.1/amp-access.js b/extensions/amp-access/0.1/amp-access.js index a5172c99c594a..42023ca54de83 100644 --- a/extensions/amp-access/0.1/amp-access.js +++ b/extensions/amp-access/0.1/amp-access.js @@ -157,7 +157,8 @@ export class AccessService { /** @private @const {function(string):Promise} */ this.openLoginDialog_ = openLoginDialog.bind(null, ampdoc); - this.getLoginUrl_ = getLoginUrl.bind(null, ampdoc); + /** @const {function(string):Promise} */ + this.getLoginUrl = getLoginUrl.bind(null, ampdoc); /** @private {?Promise} */ this.readerIdPromise_ = null; @@ -816,10 +817,11 @@ export class AccessService { * * @param {string} url * @param {string} eventLabel A label used for the analytics event for this action + * @param {boolean} noReturnUrl Don't perform a RETURN_URL replacement * @return {!Promise} */ - loginWithUrl(url, eventLabel = '') { - return this.login_(url, eventLabel); + loginWithUrl(url, eventLabel = '', noReturnUrl) { + return this.login_(url, eventLabel, noReturnUrl); } /** @@ -833,9 +835,10 @@ export class AccessService { * @private * @param {string} loginUrl * @param {string} eventLabel A label used for the analytics event for this action + * @param {boolean} noReturnUrl Don't perform a RETURN_URL replacement * @return {!Promise} */ - login_(loginUrl, eventLabel) { + login_(loginUrl, eventLabel, noReturnUrl) { const now = Date.now(); // If login is pending, block a new one from starting for 1 second. After @@ -850,7 +853,7 @@ export class AccessService { this.loginAnalyticsEvent_(eventLabel, 'started'); const dialogPromise = this.signIn_.requestSignIn(loginUrl) || - this.openLoginDialog_(loginUrl); + this.openLoginDialog_(loginUrl, noReturnUrl); const loginPromise = dialogPromise.then(result => { dev().fine(TAG, 'Login dialog completed: ', eventLabel, result); this.loginPromise_ = null; @@ -922,14 +925,6 @@ export class AccessService { return Promise.all(promises); } - /** - * @param {string} url - * @return {string} - */ - buildLoginUrl(url) { - return this.getLoginUrl_(url); - } - } /** diff --git a/extensions/amp-access/0.1/login-dialog.js b/extensions/amp-access/0.1/login-dialog.js index f267dba14ffa8..4d170025c225c 100644 --- a/extensions/amp-access/0.1/login-dialog.js +++ b/extensions/amp-access/0.1/login-dialog.js @@ -35,24 +35,29 @@ const RETURN_URL_REGEX = new RegExp('RETURN_URL'); * returned promise is rejected. * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {string|!Promise} urlOrPromise + * @param {boolean} noReturnUrl Don't perform a RETURN_URL replacement * @return {!Promise} */ -export function openLoginDialog(ampdoc, urlOrPromise) { +export function openLoginDialog(ampdoc, urlOrPromise, noReturnUrl) { const viewer = viewerForDoc(ampdoc); const overrideDialog = parseInt(viewer.getParam('dialog'), 10); if (overrideDialog) { - return new ViewerLoginDialog(viewer, urlOrPromise).open(); + return new ViewerLoginDialog(viewer, urlOrPromise, noReturnUrl) + .open(); } - return new WebLoginDialog(ampdoc.win, viewer, urlOrPromise).open(); + return new WebLoginDialog(ampdoc.win, viewer, urlOrPromise, noReturnUrl) + .open(); } export function getLoginUrl(ampdoc, urlOrPromise) { const viewer = viewerForDoc(ampdoc); const overrideDialog = parseInt(viewer.getParam('dialog'), 10); if (overrideDialog) { - return new ViewerLoginDialog(viewer, urlOrPromise).getLoginUrl(); + return new ViewerLoginDialog(viewer, urlOrPromise) + .getLoginUrl(urlOrPromise); } - return new WebLoginDialog(ampdoc.win, viewer, urlOrPromise).getLoginUrl(); + return new WebLoginDialog(ampdoc.win, viewer, urlOrPromise) + .getLoginUrl(urlOrPromise); } @@ -63,24 +68,28 @@ class ViewerLoginDialog { /** * @param {!Viewer} viewer * @param {string|!Promise} urlOrPromise + * @param {boolean} noReturnUrl */ - constructor(viewer, urlOrPromise) { + constructor(viewer, urlOrPromise, noReturnUrl) { /** @const {!Viewer} */ this.viewer = viewer; /** @const {string|!Promise} */ this.urlOrPromise = urlOrPromise; + + /** @const {boolean} */ + this.noReturnUrl = noReturnUrl; } - getLoginUrl() { + getLoginUrl(urlOrPromise) { let urlPromise; - if (typeof this.urlOrPromise == 'string') { - urlPromise = Promise.resolve(this.urlOrPromise); + if (typeof urlOrPromise == 'string') { + urlPromise = Promise.resolve(urlOrPromise); } else { - urlPromise = this.urlOrPromise; + urlPromise = urlOrPromise; } return urlPromise.then(url => { - return buildLoginUrl(url, 'RETURN_URL'); + return this.noReturnUrl ? url : buildLoginUrl(url, 'RETURN_URL'); }); } @@ -92,7 +101,7 @@ class ViewerLoginDialog { * @return {!Promise} */ open() { - return this.getLoginUrl().then(loginUrl => { + return this.getLoginUrl(this.urlOrPromise).then(loginUrl => { dev().fine(TAG, 'Open viewer dialog: ', loginUrl); return this.viewer.sendMessageAwaitResponse('openDialog', { 'url': loginUrl, @@ -112,8 +121,9 @@ export class WebLoginDialog { * @param {!Window} win * @param {!Viewer} viewer * @param {string|!Promise} urlOrPromise + * @param {boolean} noReturnUrl */ - constructor(win, viewer, urlOrPromise) { + constructor(win, viewer, urlOrPromise, noReturnUrl) { /** @const {!Window} */ this.win = win; @@ -140,6 +150,9 @@ export class WebLoginDialog { /** @private {?Unlisten} */ this.messageUnlisten_ = null; + + /** @const {boolean} */ + this.noReturnUrl = noReturnUrl; } /** @@ -188,9 +201,9 @@ export class WebLoginDialog { } } - getLoginUrl() { + getLoginUrl(url) { const returnUrl = this.getReturnUrl_(); - return buildLoginUrl(this.urlOrPromise, returnUrl); + return this.noReturnUrl ? url : buildLoginUrl(url, returnUrl); } /** @private */ @@ -206,7 +219,7 @@ export class WebLoginDialog { this.dialogReadyPromise_ = null; if (typeof this.urlOrPromise == 'string') { - const loginUrl = buildLoginUrl(this.urlOrPromise, returnUrl); + const loginUrl = this.getLoginUrl(this.urlOrPromise); dev().fine(TAG, 'Open dialog: ', loginUrl, returnUrl, w, h, x, y); this.dialog_ = openWindowDialog(this.win, loginUrl, '_blank', options); if (this.dialog_) { @@ -217,7 +230,7 @@ export class WebLoginDialog { this.dialog_ = openWindowDialog(this.win, '', '_blank', options); if (this.dialog_) { this.dialogReadyPromise_ = this.urlOrPromise.then(url => { - const loginUrl = buildLoginUrl(url, returnUrl); + const loginUrl = this.getLoginUrl(url); dev().fine(TAG, 'Set dialog url: ', loginUrl); this.dialog_.location.replace(loginUrl); }, error => { diff --git a/extensions/amp-access/0.1/test/test-amp-access.js b/extensions/amp-access/0.1/test/test-amp-access.js index 31e0bab3855df..1a7a63c1677c2 100644 --- a/extensions/amp-access/0.1/test/test-amp-access.js +++ b/extensions/amp-access/0.1/test/test-amp-access.js @@ -1416,7 +1416,7 @@ describes.fakeWin('AccessService login', { const viewStub = sandbox.stub(service, 'scheduleView_'); const broadcastStub = sandbox.stub(service.viewer_, 'broadcast'); serviceMock.expects('openLoginDialog_') - .withExactArgs('https://acme.com/l?rid=R') + .withExactArgs('https://acme.com/l?rid=R', undefined) .returns(Promise.resolve('#success=true')) .once(); return service.loginWithType_('').then(() => { @@ -1442,7 +1442,7 @@ describes.fakeWin('AccessService login', { it('should fail login with success=no', () => { service.runAuthorization_ = sandbox.spy(); serviceMock.expects('openLoginDialog_') - .withExactArgs('https://acme.com/l?rid=R') + .withExactArgs('https://acme.com/l?rid=R', undefined) .returns(Promise.resolve('#success=no')) .once(); return service.loginWithType_('').then(() => { @@ -1461,7 +1461,7 @@ describes.fakeWin('AccessService login', { const viewStub = sandbox.stub(service, 'scheduleView_'); const broadcastStub = sandbox.stub(service.viewer_, 'broadcast'); serviceMock.expects('openLoginDialog_') - .withExactArgs('https://acme.com/l?rid=R') + .withExactArgs('https://acme.com/l?rid=R', undefined) .returns(Promise.resolve('')) .once(); return service.loginWithType_('').then(() => { @@ -1486,7 +1486,7 @@ describes.fakeWin('AccessService login', { it('should fail login with aborted dialog', () => { service.runAuthorization_ = sandbox.spy(); serviceMock.expects('openLoginDialog_') - .withExactArgs('https://acme.com/l?rid=R') + .withExactArgs('https://acme.com/l?rid=R', undefined) .returns(Promise.reject('abort')) .once(); return service.loginWithType_('') @@ -1514,7 +1514,7 @@ describes.fakeWin('AccessService login', { () => Promise.resolve()); const broadcastStub = sandbox.stub(service.viewer_, 'broadcast'); serviceMock.expects('openLoginDialog_') - .withExactArgs('https://acme.com/l2?rid=R') + .withExactArgs('https://acme.com/l2?rid=R', undefined) .returns(Promise.resolve('#success=true')) .once(); return service.loginWithType_('login2').then(() => { @@ -1569,7 +1569,7 @@ describes.fakeWin('AccessService login', { it('should login with url only', () => { serviceMock.expects('login_') - .withExactArgs('https://url', '') + .withExactArgs('https://url', '', undefined) .once(); service.loginWithUrl('https://url'); }); @@ -1594,7 +1594,7 @@ describes.fakeWin('AccessService login', { const viewStub = sandbox.stub(service, 'scheduleView_'); const broadcastStub = sandbox.stub(service.viewer_, 'broadcast'); serviceMock.expects('openLoginDialog_') - .withExactArgs('https://acme.com/l?rid=R') + .withExactArgs('https://acme.com/l?rid=R', undefined) .returns(Promise.resolve('#success=true')) .once(); return service.loginWithType_('').then(() => {