Remove stateful app.settings.accessToken

[green3g]

Summary

I'd like to open a discussion on eliminating the stored accessToken on the app object. Setting accessToken on the app introduces a bug in ssr where there is a stateful accessToken being set in the feathers object. DoneJS, for instance doesn't reload all of its client files between requests and instead uses a zone-safe storage object for cookies, etc. When another user accesses the same ssr server, accessToken is already set and that access token is used to access rest api's from the ssr server, resulting in a user impersonating someone else.

This pull request replaces app.get('accessToken') with app.get('storage').getItem('accessToken'); app is a stateful object that shouldn't be used to store information about a user cookie. The storage property on the app is already used in a few places, so there's nothing added there, I am however getting rid accessToken, a property that might be used in places so I'm not sure of the potential pitfalls of this.

• Tell us about the problem your pull request is solving.
• Are there any open issues that are related to this?
• Is this PR dependent on PRs in other repos?

Other Information

• Won't authenticate in DoneSSR canjs/can-connect-feathers#111
• Explain why 3.x is incompatible with SSR? canjs/can-connect-feathers#77

[green3g]

Okay, lots of errors in the tests. I can go about fixing these in additional commits, but before I get too far, I'd like to get some feedback if this is the right solution or is there a different approach I should be taking.

[daffl]

I think this totally makes sense, but maybe I can get @marshallswain to double check.

[marshallswain]

This is something that caught me up when integrating DoneSSR many times, so I'd love to see this land. @roemhildtg, let me know if you need any assistance.

[foxhound87]

I got this issue too, it is very harmful and should be addressed asap!
Please provide a completely stateless authentication mechanism!

Thanks.

[green3g]

Hi guys. I haven't forgotten about this! I'm just in the middle of moving and have been pretty busy lately. I hope to have time in the next month or so to finish this.

[daffl]

The tests should be passing as well.

[daffl]

This needs to work asynchronously (for React Natives AsyncStorage):

return Promise.resolve(app.get('storage').getItem(app.passport.options.storageKey)).then(accessToken => {
  Object.assign(hook.params, { accessToken });
  return hook;
});

[green3g]

These are (mostly) passing for me (on windows).

There are 4 that won't run though...they are just stuck in pending for some reason.

@marshallswain just wondering if you have any suggestions.

[bparise]

Is there a current workaround for this? It took me days to figure out why state was persisting when running this inside of Nuxtjs with SSR. I moved my calls to just using this.$axios.$post('/authentication') but was curious if there was another workaround so I can continue to use this module.

[green3g]

AFAIK the workaround for now is to check for ssr in your Auth implementation and if using ssr, return not authenticated on the server. If on the client browser authenticate then update view to represent authenticated state.

[green3g]

Hey @marshallswain I'd like to see the solution to this merged for a donejs project. Any tips you can offer on getting these tests to pass?

[daffl]

This has been included in Feathers v4 authentication. Please see the migration guide for more information. Closing this PR in order to archive this repository. Related issues can be opened at the new code location in the Feathers main repository.