Cookie store rewrite (#1056)

* Persist to cookie when relevant attributes change * Refactor cookie properties on adaptive store * Fix cookie store getters and setters * Fix adaptive store test * Remove unused code * Fix cookie properties on adaptive store * Change cookie properties to adaptive store * Fix cookie setters * Fix jshint errors * Remove unused code * Wrap `rewriteCookie` in Ember.run.scheduleOnce * Remove extra run.scheduleOnce * Remove unnecessary jshint ignores * Minor cleanup * Fix cookie domain test * Add tests for rewrite behavior * Save WIP on shared cookie behavior * fix cookie store rewrite behavior tests * Small fixes (#1047) * Fix jshint error * Allow cookie to write blank values on clear * Restore previous cookieDomain test * Persist value in test so cookie will write * Fix cookie domain test * simplify AdaptiveStore code * simplify cooke store code * use CPs consistently internally * code cleanup * handle changes of cookie name correctly * no need to clear before rewrite * clear old expiration cookie as well * Include path when deleting cookie (#1067) * Refactor cookie string (#1074) * Include path when deleting cookie * Add a cookie string method
mainmatter · Sep 21, 2016 · c001cea · c001cea
1 parent 726e59b
commit c001cea
Show file tree

Hide file tree

Showing 5 changed files with 184 additions and 25 deletions.
diff --git a/addon/session-stores/adaptive.js b/addon/session-stores/adaptive.js
@@ -8,6 +8,22 @@ const { computed } = Ember;
 
 const LOCAL_STORAGE_TEST_KEY = '_ember_simple_auth_test_key';
 
+const proxyToInternalStore = function() {
+  return computed({
+    get(key) {
+      return this.get(`_${key}`);
+    },
+    set(key, value) {
+      this.set(`_${key}`, value);
+      let _store = this.get('_store');
+      if (_store) {
+        _store.set(key, value);
+      }
+      return value;
+    }
+  });
+};
+
 /**
   Session store that persists data in the browser's `localStorage` (see
   {{#crossLink "LocalStorageStore"}}{{/crossLink}}) if that is available or in
@@ -44,7 +60,8 @@ export default Base.extend({
     @default null
     @public
   */
-  cookieDomain: null,
+  _cookieDomain: null,
+  cookieDomain: proxyToInternalStore(),
 
   /**
     The name of the cookie to use if `localStorage` is not available.
@@ -54,7 +71,8 @@ export default Base.extend({
     @default ember_simple_auth-session
     @public
   */
-  cookieName: 'ember_simple_auth-session',
+  _cookieName: 'ember_simple_auth-session',
+  cookieName: proxyToInternalStore(),
 
   /**
     The expiration time for the cookie in seconds if `localStorage` is not
@@ -66,7 +84,8 @@ export default Base.extend({
     @type Integer
     @public
   */
-  cookieExpirationTime: null,
+  _cookieExpirationTime: null,
+  cookieExpirationTime: proxyToInternalStore(),
 
   _isLocalStorageAvailable: computed(function() {
     try {

diff --git a/addon/session-stores/cookie.js b/addon/session-stores/cookie.js
@@ -2,7 +2,21 @@ import Ember from 'ember';
 import BaseStore from './base';
 import objectsAreEqual from '../utils/objects-are-equal';
 
-const { RSVP, computed, run: { next, cancel, later }, isEmpty, typeOf, testing } = Ember;
+const { RSVP, computed, run: { next, cancel, later, scheduleOnce }, isEmpty, typeOf, testing, isBlank, isPresent, K, A } = Ember;
+
+const persistingProperty = function(beforeSet = K) {
+  return computed({
+    get(key) {
+      return this.get(`_${key}`);
+    },
+    set(key, value) {
+      beforeSet.apply(this);
+      this.set(`_${key}`, value);
+      scheduleOnce('actions', this, this.rewriteCookie);
+      return value;
+    }
+  });
+};
 
 /**
   Session store that persists data in a cookie.
@@ -52,7 +66,8 @@ export default BaseStore.extend({
     @default null
     @public
   */
-  cookieDomain: null,
+  _cookieDomain: null,
+  cookieDomain: persistingProperty(),
 
   /**
     The name of the cookie.
@@ -62,7 +77,10 @@ export default BaseStore.extend({
     @default ember_simple_auth-session
     @public
   */
-  cookieName: 'ember_simple_auth-session',
+  _cookieName: 'ember_simple_auth-session',
+  cookieName: persistingProperty(function() {
+    this._oldCookieName = this._cookieName;
+  }),
 
   /**
     The expiration time for the cookie in seconds. A value of `null` will make
@@ -74,7 +92,8 @@ export default BaseStore.extend({
     @type Integer
     @public
   */
-  cookieExpirationTime: null,
+  _cookieExpirationTime: null,
+  cookieExpirationTime: persistingProperty(),
 
   _secureCookies: window.location.protocol === 'https:',
 
@@ -121,7 +140,7 @@ export default BaseStore.extend({
     @public
   */
   restore() {
-    let data = this._read(this.cookieName);
+    let data = this._read(this.get('cookieName'));
     if (isEmpty(data)) {
       return RSVP.resolve({});
     } else {
@@ -137,7 +156,7 @@ export default BaseStore.extend({
     @public
   */
   clear() {
-    this._write(null, 0);
+    this._write('', 0);
     this._lastData = {};
     return RSVP.resolve();
   },
@@ -148,20 +167,38 @@ export default BaseStore.extend({
   },
 
   _calculateExpirationTime() {
-    let cachedExpirationTime = this._read(`${this.cookieName}-expiration_time`);
+    let cachedExpirationTime = this._read(`${this.get('cookieName')}-expiration_time`);
     cachedExpirationTime     = !!cachedExpirationTime ? new Date().getTime() + cachedExpirationTime * 1000 : null;
-    return !!this.cookieExpirationTime ? new Date().getTime() + this.cookieExpirationTime * 1000 : cachedExpirationTime;
+    return this.get('cookieExpirationTime') ? new Date().getTime() + this.get('cookieExpirationTime') * 1000 : cachedExpirationTime;
   },
 
   _write(value, expiration) {
-    let path        = '; path=/';
-    let domain      = isEmpty(this.cookieDomain) ? '' : `; domain=${this.cookieDomain}`;
+    let _value = encodeURIComponent(value);
+    if (this._oldCookieName) {
+      A([this._oldCookieName, `${this._oldCookieName}-expiration_time`]).forEach((oldCookie) => {
+        let expires     = `; expires=${new Date(0).toUTCString()}`;
+        document.cookie = this._createCookieString(oldCookie, _value, expires);
+      });
+      delete this._oldCookieName;
+    }
+
+    if (isBlank(value) && expiration !== 0) {
+      return;
+    }
+
+    let {
+      cookieName,
+      cookieExpirationTime
+    } = this.getProperties('cookieName', 'cookieDomain', 'cookieExpirationTime');
+
     let expires     = isEmpty(expiration) ? '' : `; expires=${new Date(expiration).toUTCString()}`;
-    let secure      = !!this._secureCookies ? ';secure' : '';
-    document.cookie = `${this.cookieName}=${encodeURIComponent(value)}${domain}${path}${expires}${secure}`;
-    if (expiration !== null) {
-      let cachedExpirationTime = this._read(`${this.cookieName}-expiration_time`);
-      document.cookie = `${this.cookieName}-expiration_time=${encodeURIComponent(this.cookieExpirationTime || cachedExpirationTime)}${domain}${path}${expires}${secure}`;
+    document.cookie = this._createCookieString(cookieName, _value, expires);
+
+    if (expiration !== null && cookieExpirationTime !== null) {
+      let cachedExpirationTime = this._read(`${cookieName}-expiration_time`);
+      let expiry = encodeURIComponent(cookieExpirationTime) || cachedExpirationTime;
+      let name = `${cookieName}-expiration_time`;
+      document.cookie = this._createCookieString(name, expiry, expires);
     }
   },
 
@@ -198,5 +235,21 @@ export default BaseStore.extend({
     } else {
       return RSVP.resolve();
     }
+  },
+
+  _createCookieString(name, value, expires) {
+    let cookieDomain = this.get('cookieDomain');
+    let domain = isEmpty(cookieDomain) ? '' : `;domain=${cookieDomain}`;
+    let path = '; path=/';
+    let secure = this._secureCookies ? ';secure' : '';
+    return `${name}=${value}${domain}${path}${expires}${secure}`;
+  },
+
+  rewriteCookie() {
+    const data = this._read(this._oldCookieName);
+    if (isPresent(data)) {
+      const expiration = this._calculateExpirationTime();
+      this._write(data, expiration);
+    }
   }
 });
diff --git a/tests/unit/session-stores/adaptive-test.js b/tests/unit/session-stores/adaptive-test.js
@@ -1,8 +1,14 @@
 import { describe, beforeEach, afterEach } from 'mocha';
+import { it } from 'ember-mocha';
+import { expect } from 'chai';
+import Ember from 'ember';
+import sinon from 'sinon';
 import Adaptive from 'ember-simple-auth/session-stores/adaptive';
 import itBehavesLikeAStore from './shared/store-behavior';
 import itBehavesLikeACookieStore from './shared/cookie-store-behavior';
 
+const { run } = Ember;
+
 describe('AdaptiveStore', () => {
   let store;
 
@@ -43,7 +49,26 @@ describe('AdaptiveStore', () => {
       },
       sync(store) {
         store.get('_store')._syncData();
+      },
+      spyRewriteCookieMethod(store) {
+        sinon.spy(store.get('_store'), 'rewriteCookie');
+        return store.get('_store').rewriteCookie;
       }
     });
+
+    it('persists to cookie when cookie attributes change', () => {
+      run(() => {
+        let store = Adaptive.create({
+          _isLocalStorageAvailable: false
+        });
+        store.persist({ key: 'value' });
+        store.setProperties({
+          cookieName:           'test:session',
+          cookieExpirationTime: 60
+        });
+      });
+
+      expect(document.cookie).to.contain('test:session-expiration_time=60');
+    });
   });
 });
diff --git a/tests/unit/session-stores/cookie-test.js b/tests/unit/session-stores/cookie-test.js
@@ -1,5 +1,6 @@
 /* jshint expr:true */
 import { describe, beforeEach, afterEach } from 'mocha';
+import sinon from 'sinon';
 import Cookie from 'ember-simple-auth/session-stores/cookie';
 import itBehavesLikeAStore from './shared/store-behavior';
 import itBehavesLikeACookieStore from './shared/cookie-store-behavior';
@@ -33,6 +34,10 @@ describe('CookieStore', () => {
     },
     sync(store) {
       store._syncData();
+    },
+    spyRewriteCookieMethod(store) {
+      sinon.spy(store, 'rewriteCookie');
+      return store.rewriteCookie;
     }
   });
 });
diff --git a/tests/unit/session-stores/shared/cookie-store-behavior.js b/tests/unit/session-stores/shared/cookie-store-behavior.js
@@ -4,18 +4,22 @@ import { it } from 'ember-mocha';
 import { describe, beforeEach, afterEach } from 'mocha';
 import { expect } from 'chai';
 
-const { run: { next } } = Ember;
+const { run, run: { next } } = Ember;
 
 export default function(options) {
   let store;
   let createStore;
   let renew;
   let sync;
+  let spyRewriteCookieMethod;
+  let unspyRewriteCookieMethod;
 
   beforeEach(() => {
     createStore = options.createStore;
     renew = options.renew;
     sync = options.sync;
+    spyRewriteCookieMethod = options.spyRewriteCookieMethod;
+    unspyRewriteCookieMethod = options.unspyRewriteCookieMethod;
     store = createStore();
   });
 
@@ -25,17 +29,26 @@ export default function(options) {
 
   describe('#persist', () => {
     it('respects the configured cookieName', () => {
-      store = createStore({ cookieName: 'test-session' });
+      let store;
+      run(() => {
+        store = createStore({ cookieName: 'test-session' });
+      });
       store.persist({ key: 'value' });
 
       expect(document.cookie).to.contain('test-session=%7B%22key%22%3A%22value%22%7D');
     });
 
     it('respects the configured cookieDomain', () => {
-      store = createStore({ cookieDomain: 'example.com' });
-      store.persist({ key: 'value' });
+      let store;
+      run(() => {
+        store = createStore({
+          cookieName: 'session-cookie-domain',
+          cookieDomain: 'example.com'
+        });
+        store.persist({ key: 'value' });
+      });
 
-      expect(document.cookie).to.not.contain('test-session=%7B%22key%22%3A%22value%22%7D');
+      expect(document.cookie).to.not.contain('session-cookie-domain=%7B%22key%22%3A%22value%22%7D');
     });
   });
 
@@ -44,14 +57,14 @@ export default function(options) {
       store = createStore({
         cookieName:           'test-session',
         cookieExpirationTime: 60,
-        expires:              new Date().getTime() + store.cookieExpirationTime * 1000
+        expires:              new Date().getTime() + store.get('cookieExpirationTime') * 1000
       });
       store.persist({ key: 'value' });
       renew(store);
     });
 
     it('stores the expiration time in a cookie named "test-session-expiration_time"', () => {
-      expect(document.cookie).to.contain(`${store.cookieName}-expiration_time=60`);
+      expect(document.cookie).to.contain('test-session-expiration_time=60');
     });
   });
 
@@ -98,4 +111,48 @@ export default function(options) {
       });
     });
   });
+
+  describe('rewrite behavior', () => {
+    let store;
+    let cookieSpy;
+
+    beforeEach(() => {
+      store = createStore({
+        cookieName: 'session-foo',
+        cookieExpirationTime: 1000
+      });
+      cookieSpy = spyRewriteCookieMethod(store);
+    });
+
+    afterEach(() => {
+      cookieSpy.restore();
+    });
+
+    it('deletes the old cookie and writes a new one when name property changes', (done) => {
+      run(() => {
+        store.persist({ key: 'value' });
+        store.set('cookieName', 'session-bar');
+      });
+
+      next(() => {
+        expect(document.cookie).to.not.contain('session-foo=');
+        expect(document.cookie).to.not.contain('session-foo-expiration_time=');
+        expect(document.cookie).to.contain('session-bar=%7B%22key%22%3A%22value%22%7D');
+        expect(document.cookie).to.contain('session-bar-expiration_time=');
+        done();
+      });
+    });
+
+    it('only rewrites the cookie once per run loop when multiple properties are changed', (done) => {
+      run(() => {
+        store.set('cookieName', 'session-bar');
+        store.set('cookieExpirationTime', 10000);
+      });
+
+      next(() => {
+        expect(cookieSpy).to.have.been.called.once;
+        done();
+      });
+    });
+  });
 }