feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
15.06k stars 752 forks source link

Changing secret in v4 #1440

Closed ranjithkumar8352 closed 5 years ago

ranjithkumar8352 commented 5 years ago

Is it mandatory to change secret in v4 so that all users get logged out?

daffl commented 5 years ago

Yes, because the JWT format changed. The entity id is now in the subject (where it really should be), not in the payload. Supporting backwards compatible JWTs is possible but requires additional manual configuration.

ranjithkumar8352 commented 5 years ago

Can you give an example of manual configuration?

amaanm commented 5 years ago

In src/authentication.js, instead of using the default AuthenticationService, you would need to extend it with something like this:

class MyAuthenticationService extends AuthenticationService {
  async getPayload(authResult, params) {
    // Call original `getPayload` first
    const payload = await super.getPayload(authResult, params);
    const { user } = authResult;

    return {
      ...payload,
      userId: user.id
    };
  }
}

I've tested this and with the same JWT secret in the config, the same accessToken can access both v3 and v4 services.

Another difference is that the new v4 client also appends the JWT in the Authorization header prepended with Bearer (which is more common than how v3 did it). If you use the v4 client to try and access any v3 servers you will have invalid token errors as it is not expecting the Bearer prepended.

I have also worked around that by setting the scheme in the feathers client like so:

// app here is the frontend app
app.configure(auth({
  scheme: '',
}));

The v4 server seems tolerant to receiving the Authorization header without Bearer prepending the token.

I'm unsure if this is something that is planning to be supported going-forward with v4? If tokens compatible with v3 and v4 are going to be an accepted/supported case I can throw together an example section for the documentation. I know in my usage that being able to flexibly support a mix of v3 and v4 is definitely desired.

ranjithkumar8352 commented 5 years ago

Thanks a lot @amaanm I will try this out. It would be great if it is supported officially.

ranjithkumar8352 commented 5 years ago

@amaanm I've tried the above custom class. Few of my observations :

  1. It should be user._id instead of user.id in the payload return value

  2. Above custom class will only append the userId to the authentication service response payload so It still fails while verifying the old JWTs because v3 server JWTs have subject as anonymous (pulled from default config) instead of having entity id.

V4 server expects entity id to be in the subject so it pulls entity Id from the subject which would be anonymous and tries to validate the userId from user service and throws an error Cast to ObjectId failed for value "anonymous" at path "_id" for model "users"

  1. Above custom class would still help in one use case while a user logs in and userId or user object needs to be stored in the local storage.

    Api.authenticate({
      strategy: "local",
      email: "email",
      password: "password"
    })
      .then(response => {
        return Api.passport.verifyJWT(response.accessToken);
      })
      .then(payload => {
        return Api.service("users").get(payload.userId);
      })
      .then(user => {
        Api.set("user", user);
        setUser(user); //localStorage
      })

    If above custom class is not used then payload.userId would be empty so all the old clients would have some issue in getting the user.

  2. One more issue is that when v4 server fails to verify v3 JWTs with the error Cast to ObjectId failed for value "anonymous" at path "_id" for model "users", It returns error code 400 but the client expects 401 code to assume that token is expired or invalid and shows login page. So I'm not able to take the old users to login page also. So, There should be some manual configuration on verifying JWTs so that v3 JWTs are also supported. @daffl Any help on this?

amaanm commented 5 years ago

@ranjithkumar8352 the example I provided was not intended to enable usage of v3 JWTs in v4. My use case is such that I needed v4 issued JWTs to work in v3 APIs.

  1. That may be the case for you, I am not using Mongo so for me it's id not _id
  2. That's true. I'm not sure how you'd go about modifying the v4 API to allow v3, but I feel like extending the JWTStrategy somewhere around where it obtains entityId is probably a good start?
  3. That makes sense, since v3 doesn't return a user property as part of the authenticate response.
  4. I believe technically a 400 is accurate, as the server is responding that your request to authenticate is invalid, not that you need to authenticate.

How long-lived is your JWT expiry that this much work is a good trade-off just to not expire them? Could you have two authentication endpoints and conditionally have a v4 authentication API handle re-authentication once a token is expired, then at the end of the v3 JWT expiry period, just turn off the v3 authentication endpoint and have all the authentication requests go to the v4 API? Then you can keep using the new tokens on v3 and v4 APIs without that much work?

ranjithkumar8352 commented 5 years ago

@amaanm Thanks for the clarification. My problem is that we have an Android app which uses feathers API. So, I can't just change the authentication requests on old clients. There are many users still using the old versions of our app without updating. So If I update the server then the users with old versions of our app get a 400 error and they won't be able to use the app. Your solution of having multiple endpoints might work but it complicates things.

All I need now is to change the status code only for this specific error Cast to ObjectId failed for value "anonymous" at path "_id" for model "users" to 401 so that users can log in again or support v3 JWTs in v4 server.

Isn't the 400 error supposed to be an issue for every feathers user with old JWTs?

@daffl This issue started off as "Is Changing secret mandatory" but it's now moving towards "v4 server backward compatibility with v3 JWTs"

daffl commented 5 years ago

Allright, I realized that the Socket backwards compatibility actually still had some issues which have been fixed in #1488 and also that it wasn't that easy to customize where the entity id was coming from in the legacy strategy. In the latest prerelease you can now use the following for full v3 client backwards compatibility (tested with the Feathers chat):

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

class MyAuthenticationService extends AuthenticationService {
  async getPayload(authResult, params) {
    // Call original `getPayload` first
    const payload = await super.getPayload(authResult, params);
    const { user } = authResult;

    return {
      ...payload,
      userId: user.id
    };
  }
}

class LegacyJWTStrategy extends JWTStrategy {
  getEntityId(authResult) {
    const { authentication: { payload } } = authResult;

    return payload.userId || payload.sub;
  }
}

module.exports = app => {
  const authentication = new MyAuthenticationService(app);

  authentication.register('jwt', new LegacyJWTStrategy());
  authentication.register('local', new LocalStrategy());

  app.use('/authentication', authentication);
  app.configure(expressOauth());
};