Allow login dialogs to skip return url replacement

This allows a login dialog which has used getLoginUrl to replace urls
earlier to not have another return url appended to its url.

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' +
-                '&amp_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);
   }
 
   /**

extensions/amp-access/0.1/amp-access.js

@@ -157,7 +157,8 @@ export class AccessService {
     /** @private @const {function(string):Promise<string>} */
     this.openLoginDialog_ = openLoginDialog.bind(null, ampdoc);
 
-    this.getLoginUrl_ = getLoginUrl.bind(null, ampdoc);
+    /** @const {function(string):Promise<string>} */
+    this.getLoginUrl = getLoginUrl.bind(null, ampdoc);
 
     /** @private {?Promise<string>} */
     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);
-  }
-
 }
 
 /**

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<string>} urlOrPromise
+ * @param {boolean} noReturnUrl Don't perform a RETURN_URL replacement
  * @return {!Promise<string>}
  */
-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<string>} urlOrPromise
+   * @param {boolean} noReturnUrl
    */
-  constructor(viewer, urlOrPromise) {
+  constructor(viewer, urlOrPromise, noReturnUrl) {
     /** @const {!Viewer} */
     this.viewer = viewer;
 
     /** @const {string|!Promise<string>} */
     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<string>}
    */
   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<string>} 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 => {

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(() => {