Update CORS, add AMP-Same-Origin when Origin is missing on same origin requests

build-system/server.js

@@ -98,21 +98,7 @@ const SOURCE_ORIGIN_REGEX = new RegExp('^http://localhost:8000|' +
     '^https?://.+\.herokuapp\.com:8000/');
 
 app.use('/form/html/post', function(req, res) {
-  if (!ORIGIN_REGEX.test(req.headers.origin)) {
-    res.statusCode = 500;
-    res.end(JSON.stringify({
-      message: 'Origin header is invalid.'
-    }));
-    return;
-  }
-
-  if (!SOURCE_ORIGIN_REGEX.test(req.query.__amp_source_origin)) {
-    res.statusCode = 500;
-    res.end(JSON.stringify({
-      message: '__amp_source_origin parameter is invalid.'
-    }));
-    return;
-  }
+  assertCors(req, res, ['POST']);
 
   var form = new formidable.IncomingForm();
   form.parse(req, function(err, fields) {
@@ -132,39 +118,83 @@ app.use('/form/html/post', function(req, res) {
   });
 });
 
-app.use('/form/echo-json/post', function(req, res) {
-  if (!ORIGIN_REGEX.test(req.headers.origin)) {
-    res.statusCode = 500;
-    res.end(JSON.stringify({
-      message: 'Origin header is invalid.'
-    }));
-    return;
+function assertCors(req, res, opt_validMethods) {
+  const validMethods = opt_validMethods || ['GET', 'POST', 'OPTIONS'];
+  const invalidMethod = req.method + ' method is not allowed. Use POST.';
+  const invalidOrigin = 'Origin header is invalid.';
+  const invalidSourceOrigin = '__amp_source_origin parameter is invalid.';
+  const unauthorized = 'Unauthorized Request';
+  var origin;
+
+  if (validMethods.indexOf(req.method) == -1) {
+    res.statusCode = 405;
+    res.end(JSON.stringify({message: invalidMethod}));
+    throw invalidMethod;
   }
 
-  if (!SOURCE_ORIGIN_REGEX.test(req.query.__amp_source_origin)) {
-    res.statusCode = 500;
-    res.end(JSON.stringify({
-      message: '__amp_source_origin parameter is invalid.'
-    }));
-    return;
+  if (req.headers.origin) {
+    origin = req.headers.origin;
+    if (!ORIGIN_REGEX.test(req.headers.origin)) {
+      res.statusCode = 500;
+      res.end(JSON.stringify({message: invalidOrigin}));
+      throw invalidOrigin;
+    }
+
+    if (!SOURCE_ORIGIN_REGEX.test(req.query.__amp_source_origin)) {
+      res.statusCode = 500;
+      res.end(JSON.stringify({message: invalidSourceOrigin}));
+      throw invalidSourceOrigin;
+    }
+  } else if (req.headers['amp-same-origin'] == 'true') {
+    origin = getUrlPrefix(req);
+  } else {
+    res.statusCode = 401;
+    res.end(JSON.stringify({message: unauthorized}));
+    throw unauthorized;
   }
 
+  res.setHeader('Access-Control-Allow-Credentials', 'true');
+  res.setHeader('Access-Control-Allow-Origin', origin);
+  res.setHeader('Access-Control-Expose-Headers',
+      'AMP-Access-Control-Allow-Source-Origin')
+  res.setHeader('AMP-Access-Control-Allow-Source-Origin',
+      req.query.__amp_source_origin);
+}
+
+app.use('/form/echo-json/post', function(req, res) {
+  assertCors(req, res, ['POST']);
   var form = new formidable.IncomingForm();
   form.parse(req, function(err, fields) {
     res.setHeader('Content-Type', 'application/json');
     if (fields['email'] == '[email protected]') {
       res.statusCode = 500;
     }
-    res.setHeader('Access-Control-Allow-Origin',
-        req.headers.origin);
-    res.setHeader('Access-Control-Expose-Headers',
-        'AMP-Access-Control-Allow-Source-Origin')
-    res.setHeader('AMP-Access-Control-Allow-Source-Origin',
-        req.query.__amp_source_origin);
     res.end(JSON.stringify(fields));
   });
 });
 
+
+app.use('/form/search-html/get', function(req, res) {
+  res.setHeader('Content-Type', 'text/html');
+  res.end(`
+     <h1>Here's results for your search<h1>
+     <ul>
+      <li>Result 1</li>
+      <li>Result 2</li>
+      <li>Result 3</li>
+     </ul>
+  `);
+});
+
+
+app.use('/form/search-json/get', function(req, res) {
+  assertCors(req, res, ['GET']);
+  res.json({
+    results: [{title: 'Result 1'}, {title: 'Result 2'}, {title: 'Result 3'}]
+  });
+});
+
+
 app.use('/share-tracking/get-outgoing-fragment', function(req, res) {
   res.setHeader('AMP-Access-Control-Allow-Source-Origin',
       req.protocol + '://' + req.headers.host);

examples/forms.amp.html

@@ -71,10 +71,71 @@
 </head>
 <body>
 
-<h2>Subscribe to our weekly Newsletter (non-xhr)</h2>
+<h4>Search (GET non-xhr same origin)</h4>
+<form method="get" action="/form/search-html/get" target="_blank">
+    <fieldset>
+        <label>
+            <span>Search for</span>
+            <input type="search" name="term" required>
+        </label>
+        <input type="submit" value="Search">
+    </fieldset>
+</form>
 
+<h4>Search (GET non-xhr cross origin - current host > httpbin.org)</h4>
+<form method="get" action="https://httpbin.org/html" target="_blank">
+    <fieldset>
+        <label>
+            <span>Search for</span>
+            <input type="search" name="term" required>
+        </label>
+        <input type="submit" value="Search">
+    </fieldset>
+</form>
+
+<h4>Search (GET xhr same origin)</h4>
+<form method="get" action="/form/search-html/get" action-xhr="/form/search-json/get" target="_blank">
+    <fieldset>
+        <label>
+            <span>Search for</span>
+            <input type="search" name="term" required>
+        </label>
+        <input type="submit" value="Search">
+    </fieldset>
+
+    <div submit-success>
+        <template type="amp-mustache">
+            <h1>Here are the results for the search:</h1>
+            <ul>
+                {{#results}}<li>{{title}}</li>{{/results}}
+            </ul>
+        </template>
+    </div>
+</form>
+
+<h4>Search (GET xhr cross origin current host > httpbin.org)</h4>
+<form method="get"
+      action="https://httpbin.org/response-headers?AMP-Access-Control-Allow-Source-Origin=http%3A%2F%2Flocalhost%3A8000&access-control-expose-headers=AMP-Access-Control-Allow-Source-Origin"
+      action-xhr="https://httpbin.org/response-headers?AMP-Access-Control-Allow-Source-Origin=http%3A%2F%2Flocalhost%3A8000&access-control-expose-headers=AMP-Access-Control-Allow-Source-Origin"
+      target="_blank">
+    <fieldset>
+        <label>
+            <span>Search for</span>
+            <input type="search" name="term" required>
+        </label>
+        <input type="submit" value="Search">
+    </fieldset>
+
+    <div submit-success>
+        <template type="amp-mustache">
+            <h1>You searched for: {{term}}</h1>
+        </template>
+    </div>
+</form>
+
+<h4>Subscribe to our weekly Newsletter (POST xhr same origin)</h4>
 <form method="post"
-      action="https://example.com/form/html/post"
+      action-xhr="/form/echo-json/post"
       target="_blank">
     <fieldset>
         <label>
@@ -88,19 +149,23 @@ <h2>Subscribe to our weekly Newsletter (non-xhr)</h2>
         <input type="submit" value="Subscribe">
     </fieldset>
 
-    <div class="form-message invalid-message">
-        There are some input that needs fixing. Please fix them.
+    <div submit-success>
+        <template type="amp-mustache">
+            Success! Thanks {{name}} for subscribing! Please make sure to check your email {{email}}
+        to confirm!
+        </template>
     </div>
-    <div class="form-message valid-message">
-        Everything looks good, you can submit now!
+    <div submit-error>
+        <template type="amp-mustache">
+            Oops! {{name}}, We apologies something went wrong. Please try again later.
+        </template>
     </div>
 </form>
 
-<h2>Subscribe to our weekly Newsletter (xhr)</h2>
 
+<h4>Subscribe to our weekly Newsletter (xhr POST cross origin - current host > amp.localhost:8000)</h4>
 <form method="post"
-      action="https://example.com/form/html/post"
-      action-xhr="https://example.com/form/echo-json/post"
+      action-xhr="http://amp.localhost:8000/form/echo-json/post"
       target="_blank">
     <fieldset>
         <label>
@@ -117,7 +182,7 @@ <h2>Subscribe to our weekly Newsletter (xhr)</h2>
     <div submit-success>
         <template type="amp-mustache">
             Success! Thanks {{name}} for subscribing! Please make sure to check your email {{email}}
-        to confirm!
+            to confirm!
         </template>
     </div>
     <div submit-error>
@@ -127,11 +192,10 @@ <h2>Subscribe to our weekly Newsletter (xhr)</h2>
     </div>
 </form>
 
-<h2>Subscribe to our weekly Newsletter (xhr + submit events)</h2>
+<h4>Subscribe to our weekly Newsletter (xhr + submit events)</h4>
 
 <form method="post"
-      action="https://example.com/form/html/post"
-      action-xhr="https://example.com/form/echo-json/post"
+      action-xhr="/form/echo-json/post"
       target="_blank"
       on="submit-success:success-lightbox;submit-error:error-lightbox">
     <fieldset>
@@ -189,5 +253,16 @@ <h1>Oops! Kittens!</h1>
     </div>
 </amp-lightbox>
 
+<h4>NOT ALLOWED (POST non-xhr - submission prevented)</h4>
+<form method="post" action="/regardless" target="_blank">
+    <fieldset>
+        <label>
+            <span>Subscribe</span>
+            <input type="email" name="email" required>
+        </label>
+        <input type="submit" value="Subscribe">
+    </fieldset>
+</form>
+
 </body>
 </html>

extensions/amp-form/amp-form.md

@@ -75,13 +75,16 @@ __required__
 
 Action must be provided, `https` and is non-cdn link (does **NOT** link to https://cdn.ampproject.org).
 
+__Note__: `target` and `action` will only be used for non-xhr GET requests. AMP runtime will use `action-xhr` to make the request and will ignore `action` and `target`. When `action-xhr` is not provided AMP would make a GET request to `action` endpoint and use `target` to open a new window (if `_blank`). AMP runtime might also fallback to using action and target in cases where `amp-form` extension fails to load.
+
 **action-xhr**
-__(optional)__
+__(optional)__ for `GET` __required__ for `POST` requests 
 You can also provide an action-xhr attribute, if provided, the form will be submitted in an XHR fashion.
 
 This attribute can be the same or a different endpoint than `action` and has the same action requirements above.
 
-**Important**: Your XHR endpoints need to follow and implement [CORS Requests in AMP spec](https://github.com/ampproject/amphtml/blob/master/spec/amp-cors-requests.md).
+
+**Important**: See [Security Considerations](#security-considerations) for notes on how to secure your forms endpoints.
 
 All other [form attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form) are optional.
 
@@ -181,3 +184,18 @@ One of the main differences between `:invalid` and `:user-invalid` is when are t
 `.user-valid` and `.user-invalid` classes are a polyfill for the pseudo classes as described above. Publishers can use these to style their inputs and fieldsets to be responsive to user actions (e.g. highlighting an invalid input with a red border after user blurs from it).
 
 See the [full example here](https://github.com/ampproject/amphtml/blob/master/examples/forms.amp.html) on using these.
+
+
+## Security Considerations
+Your XHR endpoints need to follow and implement [CORS Requests in AMP spec](https://github.com/ampproject/amphtml/blob/master/spec/amp-cors-requests.md). 
+
+### Protecting against XSRF
+In addition to following AMP CORS spec, please pay extra attention to [state changing requests note](https://github.com/ampproject/amphtml/blob/master/spec/amp-cors-requests.md#note-on-state-changing-requests).
+
+In general, keep in mind the following points when accepting input from the user:
+
+* Only use POST for state changing requests.
+* Use non-XHR GET for navigational purposes only, e.g. Search.
+    * non-XHR GET requests are not going to receive accurate origin/headers and backends won't be able to protect against XSRF with the above mechanism.
+    * In general use XHR/non-XHR GET requests for navigational or information retrieval only. 
+* non-XHR POST requests are not allowed in AMP documents. This is due to inconsistencies of setting `Origin` header on these requests across browsers. And the complications supporting it would introduce in protecting against XSRF. This might be reconsidered and introduced later, please file an issue if you think this is needed. 

spec/amp-cors-requests.md

@@ -37,23 +37,28 @@ by default.
 
 ## CORS Security in AMP
 
-This security protocol consists of two components: CORS origin and source origin restrictions.
+This security protocol consists of three components: `AMP-Same-Origin`, CORS origin and source origin restrictions.
 
 CORS endpoints receive requesting origin via "Origin" HTTP header. This header has to be restricted to only allow the following origins:
  - *.ampproject.org
  - The Publisher’s own origins
 
+If `Origin` header is missing, AMP will set `AMP-Same-Origin: true` custom header. If this header is set it indicates the request is coming from same origin. And should, as rule, be allowed.
+
 Source origin restrictions has to be implemented by requiring "__amp_source_origin" URL parameter to be within a set of the Publisher's own origins. The "__amp_source_origin" parameter is passed from AMP Runtime in all fetch requests and contains the source origin, e.g. "https://publisher1.com".
 
 The resulting HTTP response has to also contain the following headers:
  - `Access-Control-Allow-Origin: <origin>`. Here "origin" refers to the requesting origin that was allowed via "Origin" request header above. Ex: "https://cdn.ampproject.org". This is a CORS spec requirement. Notice that while CORS spec allows the value of '\*' to be returned in this header, AMP strongly discourages use of '\*'. Instead the value of the "Origin" header should be validated and echoed for improved security.
  - `AMP-Access-Control-Allow-Source-Origin: <source-origin>`. Here "source-origin" indicates the source origin that is allowed to read the authorization response as was verified via "__amp_source_origin" URL parameter. Ex: "https://publisher1.com".
  - `Access-Control-Expose-Headers: AMP-Access-Control-Allow-Source-Origin`. This header simply allows CORS response to contain the "AMP-Access-Control-Allow-Source-Origin" header.
 
-#### Note on non-idempotent Requests
+#### Note on State Changing Requests
 When making CORS requests that would change the state of your system (e.g. user subscribes to or unsubscribes from a mailing list) the first two steps you need to make sure to do:
 
-1. Check the `Origin` header. If the origin was not `*.ampproject.org` or the publisher's origin, stop and return an error response.
+1. Check the `Origin` header, if it doesn't exist move to step 3. If the origin was not `*.ampproject.org` or the publisher's origin, stop and return an error response.
 2. Check the `__amp_source_origin` query parameter. If it's not the publisher's origin stop and return an error response.
+3. Check if the request has `AMP-Same-Origin: true` header. If yes, proceed to process the request safely (skip next steps).
+  * This custom request header is sent by AMP runtime when making an XHR request on sameorigin (document served from non-cache URL).
+4. Otherwise reject the request and return an error response.
 
 It's very important that these are done first before processing the request, this provides protection against CSRF attacks and avoids processing untrusted sources requests.

src/document-submit.js

@@ -15,8 +15,8 @@
  */
 
 import {startsWith} from './string';
-import {dev, user} from './log';
-import {assertHttpsUrl, getCorsUrl, SOURCE_ORIGIN_PARAM} from './url';
+import {user} from './log';
+import {assertHttpsUrl, checkCorsUrl, SOURCE_ORIGIN_PARAM} from './url';
 import {urls} from './config';
 
 
@@ -59,21 +59,28 @@ export function onDocumentFormSubmit_(e) {
         'Illegal input name, %s found: %s', SOURCE_ORIGIN_PARAM, inputs[i]);
   }
 
-  const win = form.ownerDocument.defaultView;
-  let action = form.getAttribute('action');
-  if (!form.__AMP_INIT_ACTION__) {
-    form.__AMP_INIT_ACTION__ = action;
-  } else {
-    action = form.__AMP_INIT_ACTION__;
+  const action = form.getAttribute('action');
+  const actionXhr = form.getAttribute('action-xhr');
+  const method = (form.getAttribute('method') || 'GET').toUpperCase();
+  if (method == 'GET') {
+    user().assert(action,
+        'form action attribute is required for method=GET: %s', form);
+    assertHttpsUrl(action, /** @type {!Element} */ (form), 'action');
+    user().assert(!startsWith(action, urls.cdn),
+        'form action should not be on AMP CDN: %s', form);
+    checkCorsUrl(action);
+  } else if (action) {
+    e.preventDefault();
+    user().assert(false,
+        'form action attribute is invalid for method=POST: %s', form);
+  } else if (!actionXhr) {
+    e.preventDefault();
+    user().assert(false,
+        'Only XHR based (via action-xhr attribute) submissions are support ' +
+        'for POST requests. %s',
+        form);
   }
-  user().assert(action, 'form action attribute is required: %s', form);
-  assertHttpsUrl(action, dev().assertElement(form), 'action');
-  user().assert(!startsWith(action, urls.cdn),
-      'form action should not be on AMP CDN: %s', form);
-
-  // Update the form non-xhr action to add `__amp_source_origin` parameter.
-  // This allows publishers to understand where the request is coming from.
-  form.setAttribute('action', getCorsUrl(win, action));
+  checkCorsUrl(actionXhr);
 
   const target = form.getAttribute('target');
   user().assert(target, 'form target attribute is required: %s', form);