Automatically register the authenticate hook with 'local'

[marshallswain]

Instead of having to setup your auth config then manually register the authenticate hook, like this:

  app.configure(auth(config))
    .configure(jwt())
    .configure(local());

  app.service('authentication').hooks({
    before: {
      create: [
        // You can chain multiple strategies
        auth.hooks.authenticate(['local'])
      ]
    }
  });

Can we move the process of adding the hook into the local module so the step of registering a hook on the authentication service isn't required?

var config = app.get('auth');
app.service( config.path ).hooks({
  before: {
    create: auth.hooks.authenticate('local')
  }
});

And we could do similar changes to the other strategies, even oAuth2, since we require a strategy name. Is it possible to make multiple calls to auth.hooks.authenticate()?

/cc @ekryski @daffl

[ekryski]

Nope we cannot do that because it's not always auth.hooks.authenticate('local') it can be a chain of strategies and the strategies can be completely custom.

For example we have:

app.configure(authentication(app.get('auth')))
    .configure(local())
    .configure(local({ name: 'local-device', Verifier }))
    .configure(jwt())
    .configure(jwt({ name: 'jwt-device' }));

  app.service('authentication').hooks({
    before: {
      create: [
        authentication.hooks.authenticate(['local-device', 'local', 'jwt', 'jwt-device'])
      ],
      remove: authentication.hooks.authenticate('jwt')
    }
  });

to support both device authentication using clientId and clientSecret and user auth via the regular email + password.

[marshallswain]

Ok. I made this to help others with my same mindset in the future: https://github.com/feathersjs/feathers-authentication/pull/368

I think it would be nice to remove this step, if we can figure out a way to do so. I'm thinking we could add a top-level auth option of configureStrategies: true or something to indicate that by default we would actually register strategies on authenticationService.create internally, automatically. Calls to auth.hooks.authenticate('local') would work by keeping an internal array of strategies (since most apps won't be concerned with the order of the strategies). Strategy strings that are individually passed to the authenticate hook can be pushed into the array. Something along those lines.

Anyway, this won't matter much with clearer docs. I got the rare opportunity to see this through the eyes of a noob, even though we reviewed this stuff a few weeks ago. :)

[daffl]

I still don't get it. Why can't feathers-authentication-local just register theauthentication.hooks.authenticate([ options.name ]) hook on the authentication service automatically? It's really strange if I set up

app.configure(authentication(app.get('auth')))
    .configure(local())
    .configure(jwt())

And I kind of can authenticate but not really.

[ekryski]

And I kind of can authenticate but not really.

@daffl no you can not authenticate. Not without calling authenticate somewhere (either through hooks or middleware or sockets). You could get a JWT anonymously. Which might be what you are referring to. That is also one of the reasons this is designed this way. It is intentional to be able to support anonymous auth.

You also want to be able to set custom options (like on a per hook or per route basis).

Trust me dudes, it's not a good call to auto set it up. That's exactly one of the reasons we ended up having to change auth so drastically in the first place, because options were buried and there was too much magic happening or things that were set up automatically got in the way of trying to do something custom.

I don't see what the big deal is here. It's a couple lines and gives you much more flexibility. If it was confusing to migrate and you read through the migration guide then let's talk about how the guide and docs can be improved to make this easier before we make even more code changes. 😄

[daffl]

As per our discussion, I think we can register those hooks automatically if we make the strategy mandatory. The reason I'd like to see this is not just because I ran into it 😉 but also because I still think it is the 95% case and it would be nice to not having to know about what the authentication service exactly does and which hook to register to get basic auth working.

[ekryski]

I gave this more thought and I think this is simply something we should be doing in the generator. We should not bury it and automatically set it up.

There are many times where you may not want that hook added by default and there are also custom options that can be passed to that auth.hooks.authenticate call (for example, successRedirect & failureRedirect, among others). If we bury them then we now need to proxy those (and any custom options a user wants) down through the initial auth setup. Although this will make 95% of the use cases a bit easier it will severely inhibit the other 5%.

Kind of putting my foot down on this one. I think it's a docs and generator issue and not something we should change.

[daffl]

Not too happy with it but the generator could take care of it. Maybe we can add a comment and make the strategies configurable like

  app.service('authentication').hooks({
    before: {
      create: [
        authentication.hooks.authenticate(app.get('auth').strategies)
      ],
      remove: authentication.hooks.authenticate('jwt')
    }
  });

So that you can add them dynamically (and hopefully make it more clear).