Skip to content
New issue

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

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

Already on GitHub? # to your account

v4 socket.io? auth code is assuming 'jwt' as a strategy #1430

Closed
jnardone opened this issue Jul 2, 2019 · 7 comments · Fixed by #1510
Closed

v4 socket.io? auth code is assuming 'jwt' as a strategy #1430

jnardone opened this issue Jul 2, 2019 · 7 comments · Fixed by #1510

Comments

@jnardone
Copy link
Contributor

jnardone commented Jul 2, 2019

Steps to reproduce

UPDATE WITH REPRO PROJECT - https://github.com/jnardone/alt-strategy-name
See details in this repo, but note that socket.io requests are getting the wrong strategy name passed during server-side authenticate calls and thus fail.


Our app explicitly does NOT use auth strategies named "local" or "jwt". In v3 this is working with one or two small hacks, but there appear to be a few issues with this.

In my client I've defined my auth config:

const auth = authentication({
  storage: window.localStorage,
  storageKey: 'wirewheel-jwt',
  path: '/authentication',
  jwtStrategy: 'app-jwt'
});

and my socket client

const socket = io(site);
const socketApp = feathers()
  .configure(socketio(socket))
  .configure(auth);

server-side, my config is:

{
  "secret": "...",
  "service": "users",
  "entity": "user",
  "session": "false",
  "header": "Authorization",
  "authStrategies": [ "app-jwt", "app-local" ],
  "jwtOptions": {
    "header": {
      "typ": "access"
    },
    "audience": "https://zzz.com",
    "issuer": "zzz",
    "algorithm": "HS256",
    "expiresIn": "1d"
  },
  "app-local": {
    "usernameField": "email",
    "passwordField": "password"
  }
}

and the service:

  const config = require('./authentication.json');  // the file above
  app.set('myauth', config);
  const service = new AuthenticationService(app, 'myauth');
  service.register('app-jwt', new JWTStrategy());
  service.register('app-local', new LocalStrategy());
  app.use('/myauth', service);

but when I go to make a socket.io call e.g.

const service = socketApp.service('/foo');
const result = await service.find({});

I get 401s. I can see, by inspecting the authenticate call in core.js (core.ts) that the passed-in strategy is jwt, not app-jwt.

I don't know why it's not using the configured jwtStrategy. I think it is related to the connection.js hook never getting passed the real strategy name but ???

Do the test harnesses attempt to try this (e.g. use all non-default names for things like strategies?) This has been an area of bugs in previous versions as well as v4.

Expected behavior

Should be using correct jwtStrategy based on config

Actual behavior

'jwt' is getting passed in incorrectly

System configuration

v4-pre.3 for all components
node 10.16
macos

@jnardone
Copy link
Contributor Author

jnardone commented Jul 8, 2019

this will block us from going to auth v4

@daffl
Copy link
Member

daffl commented Jul 8, 2019

The problem with the previous version was more that options could come from multiple places and only at certain times in the application lifecycle. This one should definitely be an option, but it can also be wired up with a custom strategy name like this:

const { AuthenticationService, JWTStrategy, hooks } = require('@feathersjs/authentication');
const { LocalStrategy } = require('@feathersjs/authentication-local');
const { expressOauth } = require('@feathersjs/authentication-oauth');

module.exports = app => {
  const authService = new AuthenticationService(app);

  service.register('jwt', new JWTStrategy());
  service.register('local', new LocalStrategy());

  app.use('/authentication', authService);
  app.service('authentication').hooks({
    after: [ hooks.connection('app-jwt') ]
  });
  app.configure(expressOauth());
}

@jnardone
Copy link
Contributor Author

jnardone commented Jul 8, 2019

We need to be able to use non-standard names here. And then be able to call authenticate('customname') from the back-end and have it check the appropriate config. Otherwise there's no point of allowing these to be named at all. I need to be able to have more than one thing that uses a JWTStrategy. It should never assume a strategy name since I'm explicitly invoking it in my hook with the name I want.

@jnardone
Copy link
Contributor Author

jnardone commented Jul 8, 2019

If the system requires a strategy with a name of jwt (or whatever) i'd consider that broken, or else do not allow custom names but only the default names (which would then require a separate AuthenticationService for each variant and client-side knowledge of which service to call)

@daffl
Copy link
Member

daffl commented Jul 8, 2019

No. This is the strategy authentication information that needs to be attached to the connection so that it can be handled the same way as any other request. Just like for REST subsequent requests should use the access token and its strategy, not the original authentication information.

@jnardone
Copy link
Contributor Author

jnardone commented Jul 8, 2019

I guess I don't see why it's not coming from the .register call like it does for REST. I'm providing the name at register time, and then providing the same name at enforcement time, and the connection should be aware of it by then. But I'll definitely state I don't know all the low-level stuff that's happening with feathers and socket.io connection management.

@daffl
Copy link
Member

daffl commented Jul 8, 2019

I think you're right and there may be a way to make this easier. I'm thinking of possibly having a parseConnection which returns the authentication information for a connection just like it does for HTTP requests.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants