The getStorage implementation needs to be augmented to supported private browsers on Safari OS X, Safari iOS, and Android browsers.
Users typically supply their own storage implementation:
import authentication from 'feathers-authentication/client'
import feathers from 'feathers/client'
var client = feathers()
.configure(authentication({storage: window.localStorage}));
window.localStorage is defined on these browsers, so it will be returned, rather than the shim. Consequently, any calls to setItem (e.g., https://github.com/feathersjs/feathers-authentication/blob/fca25f2b5b03b773975595ae9ac034199df5127b/src/client/index.js#L40-L42) will throw exceptions in these browsers.
More info in this comment on #132.
The crappy part in Safari is that you actually have to call setItem to know if it is working or not. I guess we have to setItem a random string, catch the exception and return the in-memory implementation.
Yup, that's what meteor does.
It's not elegant, but it's certainly an accepted solution to resolving the corner case.
I'd like to help get this implemented. Instead of using the memory provider, why don't we use cookies? They work in Private Browsing mode, and as long as we're not consuming them on the server, they're basically the same as localStorage. The additional bonus of using them would be that they persist on refreshes, where memory-based storage won't. Maybe we should make this part configurable: fallback:cookie`by default, but you can specifyfallback: 'memory'` if needed.
I actually think in 95% of use cases it already works just fine. That's just me throwing out a random number and pretending to be definitive.... but how many people actually use private browsing mode only on Safari? It sort of defeats the purpose of storing any token anyway in private browsing mode... I think the shim is a totally valid option. I'm also wondering if it is something that should just be left up to the developer to implement??
You have to think about React Native as well, which doesn't support cookies unless you are inside a webview because RN isn't a browser.
The number of users you encounter using Safari in Private Browsing mode increases a lot when you are talking about a financial application, or any app where users naturally want to do more to protect potentially-sensitive data. In this case, though, the app can simply pass a cookie storage engine and not worry about localStorage at all. I can always check if localStorage is available and switch the storageEngine based on that.
function getTokenLocation(){
let localStorageAvailable = true;
// Only run this in the browser. (localStorage isn't available on the SSR server)
if (window.localStorage) {
// Try localStorage
try{
localStorage.setItem('2398sdvlkn4v', 'test');
localStorage.removeItem('2398sdvlkn4v');
}
catch(err){
localStorageAvailable = false;
}
// If localStorage isn't available, try reading from a cookie.
if (localStorageAvailable) {
return 'localStorage';
} else {
// document.cookie = 'feathers-jwt=hi';
return 'cookie';
}
}
}
getTokenLocation();
Looks like somebody already made a web storage compatible syntax around document.cookie:
Why don't we get opinionated about the storage engine used by default? We can make the user experience better by providing a useStorage boolean in the options. If the dev also provides the storage option, then we use what they provide, otherwise we use one of the following:
If we want to keep it more modular and not have the cookie-storage package as part of this one, then maybe we just allow the storage option to take an array, then we only have to check to see if it stored properly. If one fails, we try the next.
You can easily provide your own store by passing an object that implements an interface like:
{
getItem(key) {},
setItem(key, value) {}
}
We can add a defaultStorage fallback that you can set if the check discussed above fails as part of the fix for this issue.
@ekryski what works for React Native?
I believe this issue should be closed and a new issue should be created under feathers-authentication-client.
I think this can be solved by directing people to use localForage. We can focus on solutions that others aren't already solving. Anybody object to closing this?
Here's my current solution for safari (private browsing mode):
/* eslint no-undefined: 0 */
export default function getStorage() {
const storageKey = '_localstorage_test_';
let retrieved;
try {
if (window.localStorage) {
window.localStorage.setItem(storageKey, storageKey);
retrieved = window.localStorage.getItem(storageKey);
window.localStorage.removeItem(storageKey);
}
} catch (e) {
// ... ignore
}
if (storageKey === retrieved) {
return window.localStorage;
}
return {
data: {},
setItem: function (key, val) {
this.data[key] = val;
},
removeItem: function (key) {
delete this.data[key];
},
getItem: function (key) {
const value = this.data[key];
return value === undefined ? null : value;
}
};
}
init like this:
import auth from 'feathers-authentication-client';
import feathers from 'feathers/client';
import hooks from 'feathers-hooks';
import io from 'socket.io-client';
import socketio from 'feathers-socketio/client';
const socket = io('http://api/url');
const app = feathers()
.configure(hooks())
.configure(socketio(socket))
.configure(auth({
storage: getStorage()
}));
Yeah @marshallswain I agree that for web the use case, we should probably just be a documentation update to discuss edge cases when using localStorage (in normal web environments), an inform people to use one of the polyfills in environments where window.localStorage acts weird, like Safari with Private Browsing.
I believe for React Native you'd probably just use RN's AsyncStorage, however on RN you're given a Promise-based API for getItem and setItem (unlike window.localStorage, which is synchronous).
import auth from 'feathers-authentication-client';
import feathers from 'feathers/client';
import hooks from 'feathers-hooks';
import io from 'socket.io-client';
import socketio from 'feathers-socketio/client';
import {AsyncStorage} from 'react-native';
const socket = io('http://api/url');
const app = feathers()
.configure(hooks())
.configure(socketio(socket))
.configure(auth({
storage: AsyncStorage
}));
In light of that, you may want to ensure that the auth client works with a storage API that returns promises instead of synchronous values. I'm not sure if it does this currently. But it wouldn't be hard to add support for it.
This works with AsyncStorage as @ekryski and I are using it in a few RN apps