Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow cookie to be restricted to secure connections #261

Closed
wants to merge 1 commit into from

3 participants

@jwb

The Cookie.js module only sets the cookie without the Secure option, so the
user's session information could be sent over an open connection. This is
specifically the case if the user first accesses a canvas app using a secure
connection and then visits the application's web site using an URL specifying
http.

This change allows FB.init to accept cookie: 'secure' in addition to
cookie: true. It also assumes that if the FB session ever includes a 'secure'
property, its presence implies that the cookie should be restricted
to secure connections.

It also includes a (weak) test for the new code in cookie.js. More robust
testing depends on an approach for testing against (or mocking) secure (https:)
connections.

The tests have also been modified to allow index.html to provide an
apikey/appid that is used in the initialize.js tests. It's displayed on the
page and will allow the user to change it.

The readme.md is changed to point here for issues since the attempt to report the issue at the location referenced resulted in its immediate closure with instructions to report issues here.

@jwb jwb readme.md
 Changed because when an issue was reported on the SDKs, it was closed with a note to report the issue on GitHub.
cookie.js, init.js
 Changed to process parameters to specify secure cookies.
index.html, initialize.js
 Changed to allow API_KEY used in tests to be set in index.html.
cookie.js
 Added test to verify secure cookie option.
c6499dc
@msingleton
  • a million

hopefully this will get accepted soon

@oyvindkinsey

This repository has been discontinued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 10, 2011
  1. @jwb

    readme.md

    jwb authored
     Changed because when an issue was reported on the SDKs, it was closed with a note to report the issue on GitHub.
    cookie.js, init.js
     Changed to process parameters to specify secure cookies.
    index.html, initialize.js
     Changed to allow API_KEY used in tests to be set in index.html.
    cookie.js
     Added test to verify secure cookie option.
This page is out of date. Refresh to see the latest.
View
2  readme.md
@@ -57,4 +57,4 @@ Reporting Issues
If you have bugs or other issues, file them [here][issues].
-[issues]: http://bugs.developers.facebook.net/enter_bug.cgi?product=SDKs
+[issues]: https://github.com/facebook/connect-js/issues
View
21 src/core/cookie.js
@@ -62,7 +62,17 @@ FB.provide('Cookie', {
* @returns {Boolean} true if Cookie support is enabled
*/
getEnabled: function() {
- return FB.Cookie._enabled;
+ return (!!FB.Cookie._enabled);
+ },
+
+ /**
+ * Return the security flag applied for the cooke.
+ *
+ * @access private
+ * @returns {Boolean} true if the cookie is to be restricted to secure connections
+ */
+ getSecure: function() {
+ return ('secure' == FB.Cookie._enabled);
},
/**
@@ -97,13 +107,15 @@ FB.provide('Cookie', {
* @param val {String} the string value (should already be encoded)
* @param ts {Number} a unix timestamp denoting expiry
* @param domain {String} optional domain for cookie
+ * @param secure {Boolean} optionally specify whether the cookie is to be restricted to secure connections
*/
- setRaw: function(val, ts, domain) {
+ setRaw: function(val, ts, domain, secure) {
document.cookie =
'fbs_' + FB._apiKey + '="' + val + '"' +
(val && ts == 0 ? '' : '; expires=' + new Date(ts * 1000).toGMTString()) +
'; path=/' +
- (domain ? '; domain=.' + domain : '');
+ (domain ? '; domain=.' + domain : '') +
+ (secure ? '; Secure' : '');
// capture domain for use when we need to clear
FB.Cookie._domain = domain;
@@ -120,7 +132,8 @@ FB.provide('Cookie', {
? FB.Cookie.setRaw(
FB.QS.encode(session),
session.expires,
- session.base_domain)
+ session.base_domain,
+ (session.secure === undefined) ? FB.Cookie.getSecure() : session.secure)
: FB.Cookie.clear();
},
View
16 src/core/init.js
@@ -113,14 +113,14 @@ FB.provide('', {
* @access public
* @param options {Object}
*
- * Property | Type | Description | Argument | Default
- * -------- | ------- | ------------------------------------ | ---------- | -------
- * appId | String | Your application ID. | *Optional* | `null`
- * cookie | Boolean | `true` to enable cookie support. | *Optional* | `false`
- * logging | Boolean | `false` to disable logging. | *Optional* | `true`
- * session | Object | Use specified session object. | *Optional* | `null`
- * status | Boolean | `true` to fetch fresh status. | *Optional* | `false`
- * xfbml | Boolean | `true` to parse [[wiki:XFBML]] tags. | *Optional* | `false`
+ * Property | Type | Description | Argument | Default
+ * -------- | ------- | ------------------------------------------------------------------------------------ | ---------- | -------
+ * appId | String | Your application ID. | *Optional* | `null`
+ * cookie | Boolean | `true` to enable cookie support or `secure` to restrict cookie to secure connection. | *Optional* | `false`
+ * logging | Boolean | `false` to disable logging. | *Optional* | `true`
+ * session | Object | Use specified session object. | *Optional* | `null`
+ * status | Boolean | `true` to fetch fresh status. | *Optional* | `false`
+ * xfbml | Boolean | `true` to parse [[wiki:XFBML]] tags. | *Optional* | `false`
*/
init: function(options) {
// only need to list values here that do not already have a falsy default.
View
7 tests/index.html
@@ -28,6 +28,7 @@
QUnit Requirements
**************************************************************************
-->
+ <input type='text' id='API_KEY' title='API_KEY'/>
<h1 id="qunit-header">
<input style="float: right; margin: 1em;"
type="button"
@@ -128,6 +129,9 @@ <h2 id="qunit-userAgent"></h2>
-->
<script>
(function() {
+ // Set the variable below if you are testing against a Facebook account that
+ // has not installed the App used by the original developers
+ var testApiKey = "";
// if we are running on a facebook.com domain, munge the FB._domain map
// to point to the current domain. this is useful for facebook
// developers working on the unit tests in their sandboxes.
@@ -140,7 +144,8 @@ <h2 id="qunit-userAgent"></h2>
FB._domain.staticfb = FB._domain.www.replace('www', 'static');
}
-
+ // Make the APIKey visible
+ document.getElementById('API_KEY').value = testApiKey;
// scroll to the bottom of the page as the tests run. this ensures the
// latest test is always in view.
QUnit.testDone = function(name, failures, total) {
View
25 tests/js/cookie.js
@@ -91,3 +91,28 @@ test(
FB._apiKey = origApiKey;
}
);
+//TODO: Expand this test when there are unit tests running/mocking secure connections
+//This test is a bit weak, but since the prior tests demonstrate that Cookie.set adds
+//non-secure cookies, it at least demonstrates that the code runs and specify the
+//secure option changes the behavior.
+test(
+ 'set a secure cookie and verify not present on non-secure page',
+
+ function() {
+ var origApiKey = FB._apiKey;
+ var docCookie;
+ expect(2);
+ FB._apiKey = cookieApiKey;
+
+ FB.Cookie.set({
+ expires: (1000000 + (+new Date())) / 1000,
+ base_domain: document.domain,
+ secure: true
+ });
+ docCookie = document.cookie.match('fbs_' + cookieApiKey);
+ ok(!(document.location.protocol == 'https:'), "test running on non-secure page");
+ ok(!(docCookie), 'secure cookie not applied to non-secure document');
+ FB.Cookie.clear();
+ FB._apiKey = origApiKey;
+ }
+);
View
2  tests/js/initialize.js
@@ -17,7 +17,7 @@
* @requires fb.tests.qunit
* fb.init
*/
-var API_KEY = '48f06bc570aaf9ed454699ec4fe416df';
+var API_KEY = document.getElementById('API_KEY').value || '48f06bc570aaf9ed454699ec4fe416df';
var EXPIRED_SESSION = {
session_key : "5070653ddbe6a2efbfb23388-499433185",
uid : 499433185,
Something went wrong with that request. Please try again.