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 AUTH var substitutions to pingback/login URLs. #1456

Merged
merged 1 commit into from Jan 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 33 additions & 7 deletions extensions/amp-access/0.1/amp-access.js
Expand Up @@ -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} */
Expand Down Expand Up @@ -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);
});
}

Expand All @@ -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, () => {
Expand All @@ -319,6 +334,15 @@ export class AccessService {
});
}

/**
* @param {!JSONObject} authResponse
* @private
*/
setAuthResponse_(authResponse) {
this.authResponse_ = authResponse;
this.firstAuthorizationResolver_();
}

/**
* @param {!JSONObjectDef} response
* @return {!Promise}
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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;
Expand Down
35 changes: 27 additions & 8 deletions extensions/amp-access/0.1/test/test-amp-access.js
Expand Up @@ -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;
});
});

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(() => {
Expand Down
13 changes: 9 additions & 4 deletions extensions/amp-access/amp-access.md
Expand Up @@ -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}",
Expand All @@ -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.
Expand All @@ -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.

Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/url-replacements.js
Expand Up @@ -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);
Expand Down
40 changes: 40 additions & 0 deletions test/functional/test-url-replacements.js
Expand Up @@ -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');
});
});
});