diff --git a/lib/browserid/views.js b/lib/browserid/views.js index 9380f9b5ddc22723e033ecc6445029e17c629487..48fbc69c5c5bdefd73199e53126ac18a0a159915 100644 --- a/lib/browserid/views.js +++ b/lib/browserid/views.js @@ -41,6 +41,20 @@ function renderCachableView(req, res, template, options) { } exports.setup = function(app) { + + // Issue#1353 This is kind of dirty, but this is our last chance + // to fixup headers for an ETag cache hit + // x-frame-options - Allow these to be run within a frame + app.use(function (req, res, next) { + if (req.path === '/communication_iframe') { + res.removeHeader('x-frame-options'); + } else if (req.path === '/relay') { + res.removeHeader('x-frame-options'); + } + next(); + }); + + // Caching for dynamic resources app.use(etagify()); app.set("views", VIEW_PATH); @@ -77,7 +91,7 @@ exports.setup = function(app) { }); app.get('/communication_iframe', function(req, res, next ) { - res.removeHeader('x-frame-options'); + renderCachableView(req, res, 'communication_iframe.ejs', { layout: false, production: config.get('use_minified_resources') @@ -94,8 +108,6 @@ exports.setup = function(app) { // Used for a relay page for communication. app.get("/relay", function(req, res, next) { - // Allow the relay to be run within a frame - res.removeHeader('x-frame-options'); renderCachableView(req, res, 'relay.ejs', { layout: false, production: config.get('use_minified_resources') diff --git a/tests/cache-header-tests.js b/tests/cache-header-tests.js index ab9882b78ab0f0442701c13d08b869367b24ac31..4713b0d6bbe711c5d1bf9ae556cb15a4d1e0a58a 100755 --- a/tests/cache-header-tests.js +++ b/tests/cache-header-tests.js @@ -45,6 +45,14 @@ function doRequest(path, headers, cb) { req.end(); } +function hasProperFramingHeaders(r, path) { + if (['/communication_iframe', '/relay'].indexOf(path) !== -1) { + assert.strictEqual(r.headers['x-frame-options'], undefined); + } else { + assert.strictEqual(r.headers['x-frame-options'],"DENY"); + } +} + function hasProperCacheHeaders(path) { return { topic: function() { @@ -64,6 +72,8 @@ function hasProperCacheHeaders(path) { }, "returns 200 with content": function(err, r) { assert.strictEqual(r.statusCode, 200); + // check X-Frame-Option headers + hasProperFramingHeaders(r, path); // ensure vary headers assert.strictEqual(r.headers['vary'], 'Accept-Encoding,Accept-Language'); // ensure public, max-age=0 @@ -82,12 +92,17 @@ function hasProperCacheHeaders(path) { }, this.callback); }, "returns a 304": function(err, r) { + if (!err) hasProperFramingHeaders(r, path); assert.strictEqual(r.statusCode, 304); } }, "followed by a request with an if-modified-since cache header, and bogus etag": { topic: function(err, r) { - var etag = r.headers['etag'].replace(/"$/, "bogus\""); + var etag = r.headers['etag'] = '"bogus"'; + // No ETag present in iframes, make one + if (['/communication_iframe', '/relay'].indexOf(path) === -1) { + etag = r.headers['etag'].replace(/"$/, "bogus\""); + } doRequest(path, { "If-None-Match": etag }, this.callback);