Merge pull request #1456 from dvoytenko/access19

Add AUTH var substitutions to pingback/login URLs.

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

@@ -144,6 +144,9 @@ export class AccessService {
     /** @private {?Promise<string>} */
     this.readerIdPromise_ = null;
 
+    /** @private {?JSONObject} */
+    this.authResponse_ = null;
+
     /** @private {!Promise} */
     this.firstAuthorizationPromise_ = new Promise(resolve => {
       /** @private {!Promise} */
@@ -277,14 +280,24 @@ export class AccessService {
 
   /**
    * @param {string} url
+   * @param {boolean} useAuthData Allows `AUTH(field)` URL var substitutions.
    * @return {!Promise<string>}
    * @private
    */
-  buildUrl_(url) {
+  buildUrl_(url, useAuthData) {
     return this.getReaderId_().then(readerId => {
-      return this.urlReplacements_.expand(url, {
+      const vars = {
         'READER_ID': readerId
-      });
+      };
+      if (useAuthData) {
+        vars['AUTHDATA'] = field => {
+          if (this.authResponse_) {
+            return this.authResponse_[field];
+          }
+          return undefined;
+        };
+      }
+      return this.urlReplacements_.expand(url, vars);
     });
   }
 
@@ -301,12 +314,14 @@ export class AccessService {
 
     log.fine(TAG, 'Start authorization via ', this.config_.authorization);
     this.toggleTopClass_('amp-access-loading', true);
-    return this.buildUrl_(this.config_.authorization).then(url => {
+    const promise = this.buildUrl_(
+        this.config_.authorization, /* useAuthData */ false);
+    return promise.then(url => {
       log.fine(TAG, 'Authorization URL: ', url);
       return this.xhr_.fetchJson(url, {credentials: 'include'});
     }).then(response => {
       log.fine(TAG, 'Authorization response: ', response);
-      this.firstAuthorizationResolver_();
+      this.setAuthResponse_(response);
       this.toggleTopClass_('amp-access-loading', false);
       return new Promise((resolve, reject) => {
         onDocumentReady(this.win.document, () => {
@@ -319,6 +334,15 @@ export class AccessService {
     });
   }
 
+  /**
+   * @param {!JSONObject} authResponse
+   * @private
+   */
+  setAuthResponse_(authResponse) {
+    this.authResponse_ = authResponse;
+    this.firstAuthorizationResolver_();
+  }
+
   /**
    * @param {!JSONObjectDef} response
    * @return {!Promise}
@@ -515,7 +539,9 @@ export class AccessService {
       log.fine(TAG, 'Ignore pingback');
       return Promise.resolve();
     }
-    return this.buildUrl_(this.config_.pingback).then(url => {
+    const promise = this.buildUrl_(
+        this.config_.pingback, /* useAuthData */ true);
+    return promise.then(url => {
       log.fine(TAG, 'Pingback URL: ', url);
       return this.xhr_.sendSignal(url, {
         method: 'POST',
@@ -567,7 +593,7 @@ export class AccessService {
 
     log.fine(TAG, 'Start login');
     const urlPromise = this.buildUrl_(assert(this.config_.login,
-        'Login URL is not configured'));
+        'Login URL is not configured'), /* useAuthData */ true);
     this.loginPromise_ = this.openLoginDialog_(urlPromise).then(result => {
       log.fine(TAG, 'Login dialog completed: ', result);
       this.loginPromise_ = null;

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

@@ -298,6 +298,8 @@ describe('AccessService authorization', () => {
       expect(document.documentElement).not.to.have.class('amp-access-loading');
       expect(elementOn).not.to.have.attribute('amp-access-hide');
       expect(elementOff).to.have.attribute('amp-access-hide');
+      expect(service.authResponse_).to.exist;
+      expect(service.authResponse_.access).to.be.true;
     });
   });
 
@@ -536,7 +538,7 @@ describe('AccessService pingback', () => {
     configElement.setAttribute('type', 'application/json');
     configElement.textContent = JSON.stringify({
       'authorization': 'https://acme.com/a?rid=READER_ID',
-      'pingback': 'https://acme.com/p?rid=READER_ID',
+      'pingback': 'https://acme.com/p?rid=READER_ID&type=AUTHDATA(type)',
       'login': 'https://acme.com/l?rid=READER_ID'
     });
     document.body.appendChild(configElement);
@@ -724,13 +726,30 @@ describe('AccessService pingback', () => {
   it('should send POST pingback', () => {
     expectGetReaderId('reader1');
     xhrMock.expects('sendSignal')
-        .withExactArgs('https://acme.com/p?rid=reader1', sinon.match(init => {
-          return (init.method == 'POST' &&
-              init.credentials == 'include' &&
-              init.body == '' &&
-              init.headers['Content-Type'] ==
-                  'application/x-www-form-urlencoded');
-        }))
+        .withExactArgs('https://acme.com/p?rid=reader1&type=',
+            sinon.match(init => {
+              return (init.method == 'POST' &&
+                  init.credentials == 'include' &&
+                  init.body == '' &&
+                  init.headers['Content-Type'] ==
+                      'application/x-www-form-urlencoded');
+            }))
+        .returns(Promise.resolve())
+        .once();
+    return service.reportViewToServer_().then(() => {
+      return 'SUCCESS';
+    }, error => {
+      return 'ERROR ' + error;
+    }).then(result => {
+      expect(result).to.equal('SUCCESS');
+    });
+  });
+
+  it('should resolve AUTH vars in POST pingback', () => {
+    expectGetReaderId('reader1');
+    service.setAuthResponse_({type: 'premium'});
+    xhrMock.expects('sendSignal')
+        .withArgs('https://acme.com/p?rid=reader1&type=premium')
         .returns(Promise.resolve())
         .once();
     return service.reportViewToServer_().then(() => {

extensions/amp-access/amp-access.md

@@ -101,7 +101,7 @@ Here’s an example of the AMP Access configuration:
 ```html
 <script id="amp-access" type="application/json">
 {
-  "authorization": 
+  "authorization":
       "https://pub.com/amp-access?rid={READER_ID}&url={AMPDOC_URL}",
   "pingback":
       "https://pub.com/amp-ping?rid={READER_ID}&url={AMPDOC_URL}",
@@ -117,6 +117,7 @@ When configuring the URLs for various endpoints, the Publisher can use substitut
 Var               | Description
 ----------------- | -----------
 READER_ID         | The AMP Reader ID.
+AUTHDATA(field)   | The value of the field in the authorization response.
 AMPDOC_URL        | The URL of this AMP Document.
 CANONICAL_URL     | The canonical URL of this AMP Document.
 DOCUMENT_REFERRER | The Referrer URL.
@@ -131,9 +132,13 @@ https://pub.com/access?
   &ref={DOCUMENT_REFERRER}
   &_={RANDOM}
 ```
+
+AUTHDATA variable is availbale to Pingback and Login URLs. It allows passing any field in the authorization
+response as an URL parameter. E.g. `AUTHDATA(isSubscriber)`.
+
 ##Access Content Markup
 
-Access Content Markup describes which sections are visible or hidden. It is comprised of two AMP attributes: ```amp-access``` and ```amp-access-hide``` that can be placed on any HTML element. 
+Access Content Markup describes which sections are visible or hidden. It is comprised of two AMP attributes: ```amp-access``` and ```amp-access-hide``` that can be placed on any HTML element.
 
 The ```amp-access``` attribute provides the expression that yields true or false based on the authorization response returned by the Authorization endpoint. The resulting value indicates whether or not the element and its contents are visible.
 
@@ -252,7 +257,7 @@ The publisher may choose to use the pingback as:
  - One of the main purposes for pingback is to count down meter when it is used.
  - As a credentialed CORS endpoint it may contain publisher cookies. Thus it can be used to map AMP Reader ID to the Publisher’s identity.
 
-The request format is:  
+The request format is:
 ```
 https://publisher.com/amp-pingback?
    rid={READER_ID}
@@ -323,7 +328,7 @@ PROPERTY ::= [a-zA-Z][a-zA-Z0-9\_\-]*
 VALUE ::= QUOTE [a-zA-Z0-9\_\-]* QUOTE
 ```
 
-Notice that ```amp-access``` expressions are evaluated by the AMP Runtime and AMP Cache. This is NOT part of the specification that the Publisher needs to implement. It is here simply for informational properties. 
+Notice that ```amp-access``` expressions are evaluated by the AMP Runtime and AMP Cache. This is NOT part of the specification that the Publisher needs to implement. It is here simply for informational properties.
 
 #Detailed Discussion
 This section will cover a detailed explanation of the design underlying the amp-access spec, and clarify design choices. Coming soon

src/url-replacements.js

@@ -172,9 +172,7 @@ class UrlReplacements {
     const expr = this.getExpr_(opt_bindings);
     let replacementPromise;
     const encodeValue = val => {
-      // Value 0 is specialcased because the numeric 0 is a valid substitution
-      // value.
-      if (!val && val !== 0) {
+      if (val == null) {
         val = '';
       }
       return encodeURIComponent(val);

test/functional/test-url-replacements.js

@@ -239,4 +239,44 @@ describe('UrlReplacements', () => {
       expect(res).to.match(/rid=func_abc\?$/);
     });
   });
+
+  it('should expand null as empty string', () => {
+    return expand('v=VALUE', false, {
+      'VALUE': null
+    }).then(res => {
+      expect(res).to.equal('v=');
+    });
+  });
+
+  it('should expand undefined as empty string', () => {
+    return expand('v=VALUE', false, {
+      'VALUE': undefined
+    }).then(res => {
+      expect(res).to.equal('v=');
+    });
+  });
+
+  it('should expand empty string as empty string', () => {
+    return expand('v=VALUE', false, {
+      'VALUE': ''
+    }).then(res => {
+      expect(res).to.equal('v=');
+    });
+  });
+
+  it('should expand zero as zero', () => {
+    return expand('v=VALUE', false, {
+      'VALUE': 0
+    }).then(res => {
+      expect(res).to.equal('v=0');
+    });
+  });
+
+  it('should expand false as false', () => {
+    return expand('v=VALUE', false, {
+      'VALUE': false
+    }).then(res => {
+      expect(res).to.equal('v=false');
+    });
+  });
 });