Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finalizing client side authentication module #101

Merged
merged 4 commits into from Mar 14, 2016

Conversation

Projects
None yet
2 participants
@daffl
Copy link
Member

daffl commented Mar 13, 2016

This is a refactoring for the client side authentication to finalize the API and make it easier to use:

const app = feathers()
  .configure(socketio(socket))
  .configure(hooks())
  .configure(authentication())

// Store the token in local- or async storage
const app = feathers()
  .configure(socketio(socket))
  .configure(hooks())
  .configure(authentication({
    storage: window.localStorage // AsyncStorage
  }));

// Authenticate with local username and password
app.authenticate({
  type: 'local',
  email: 'hello@feathersjs.com',
  password: 'supersecret'
}).then(response => {
  console.log('Authenticated', response);

  app.get('token') // -> the JWT
  app.get('user') // -> the user
});

// Authenticate with cookie JWT or JWT from storage
app.authenticate().then(response => {
  console.log('Authenticated', response);

  app.get('token') // -> the JWT
  app.get('user') // -> the user
});

Closes #95

@daffl daffl force-pushed the client-auth-polish branch from df4b699 to 05ef5db Mar 13, 2016

@daffl

This comment has been minimized.

Copy link
Member Author

daffl commented Mar 13, 2016

This should be done for now. Tested for REST, Socket.io and Primus. One tests is skipped because I couldn't find out how to disconnect an existing Primus socket for the life of me.

@daffl daffl force-pushed the client-auth-polish branch from 05ef5db to b63a56b Mar 14, 2016

return storage;
}

return {

This comment has been minimized.

@ekryski

ekryski Mar 14, 2016

Member

What's this part for? Just a shim for localstorage so that it will work with node without having to include another module?

This comment has been minimized.

@daffl

daffl Mar 14, 2016

Author Member

Yep. Just an in-memory shim for the localStorage API. I used to have window.localStorage in there as well but I think it's better to do it explicitly.

@@ -1,73 +1,220 @@
import assert from 'assert';

This comment has been minimized.

@ekryski

ekryski Mar 14, 2016

Member

👍 👍 on this whole file. Nice tests.

}).catch(done);
});

it.skip('.logout works, lets you not access to protected service', done => {

This comment has been minimized.

@ekryski

ekryski Mar 14, 2016

Member

Was this one meant to be skipped? It's not working?

This comment has been minimized.

@ekryski

ekryski Mar 14, 2016

Member

Ah I saw the note above. Ok you disconnect a socket by calling .end() I believe.

This comment has been minimized.

@daffl

daffl Mar 14, 2016

Author Member

Ah. Yeah, the problem was that this test fails with sockets because app.logout really has to disconnect the socket, too. I'll give it a shot with .end().

// Set up hook that adds authorization header for REST provider
if (app.rest) {
app.mixins.push(function(service) {
if (!service.before || !service.after) {
throw new Error(`It looks like feathers-hooks isn't configured. It is required before you configure feathers-authentication.`);
if (typeof service.before !== 'function' || typeof service.after !== 'function') {

This comment has been minimized.

@ekryski

ekryski Mar 14, 2016

Member

I realized that we probably don't need this second duplicate warning because it should be caught by the check above.

This comment has been minimized.

@daffl

daffl Mar 14, 2016

Author Member

True that. I'll remove it.

ekryski added some commits Mar 14, 2016

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 14, 2016

:shipit:

ekryski added a commit that referenced this pull request Mar 14, 2016

Merge pull request #101 from feathersjs/client-auth-polish
Finalizing client side authentication module

@ekryski ekryski merged commit bb0a881 into master Mar 14, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ekryski ekryski deleted the client-auth-polish branch Mar 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.