Skip to content

Commit

Permalink
adding entity id to payload. Ensuring custom named strategies pull fr…
Browse files Browse the repository at this point in the history
…om auth config. Closes #2.
  • Loading branch information
ekryski authored and daffl committed Aug 29, 2018
1 parent ea5a2aa commit 094306c
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 10 deletions.
6 changes: 5 additions & 1 deletion packages/authentication-jwt/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ export default function init (options = {}) {
throw new Error(`Can not find app.passport. Did you initialize feathers-authentication before feathers-authentication-jwt?`);
}

let authOptions = app.get('auth') || {};
let jwtOptions = authOptions[options.name] || {};

// NOTE (EK): Pull from global auth config to support legacy auth for an easier transition.
let jwtSettings = merge({}, defaults, pick(app.get('auth') || {}, KEYS), omit(options, ['Verifier']));
let jwtSettings = merge({}, defaults, pick(authOptions, KEYS), jwtOptions, omit(options, ['Verifier']));

if (typeof jwtSettings.header !== 'string') {
throw new Error(`You must provide a 'header' in your authentication configuration or pass one explicitly`);
Expand Down Expand Up @@ -72,6 +75,7 @@ export default function init (options = {}) {
// Register 'jwt' strategy with passport
debug('Registering jwt authentication strategy with options:', strategyOptions);
app.passport.use(jwtSettings.name, new JWTStrategy(strategyOptions, verifier.verify.bind(verifier)));
app.passport.options(jwtSettings.name, jwtSettings);

return result;
};
Expand Down
6 changes: 4 additions & 2 deletions packages/authentication-jwt/src/verifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@ class JWTVerifier {
verify (req, payload, done) {
debug('Received JWT payload', payload);

const id = payload[this.service.id];
const id = payload[`${this.options.entity}Id`];

if (id === undefined) {
debug(`JWT payload does not contain ${this.options.entity}Id`);
return done(null, { payload });
}

debug(`Looking up ${this.options.entity} by id`, id);

this.service.get(id).then(entity => {
return done(null, Object.assign({ payload }, entity));
const newPayload = { [`${this.options.entity}Id`]: id };
return done(null, Object.assign({ payload }, entity), newPayload);
})
.catch(error => {
debug(`Error populating ${this.options.entity} with id ${id}`, error);
Expand Down
27 changes: 26 additions & 1 deletion packages/authentication-jwt/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ describe('feathers-authentication-jwt', () => {
passportJWT.Strategy.restore();
});

it('registers the strategy options', () => {
sinon.spy(app.passport, 'options');
app.configure(jwt());
app.setup();

expect(app.passport.options).to.have.been.calledOnce;

app.passport.options.restore();
});

describe('passport strategy options', () => {
let authOptions;
let args;
Expand Down Expand Up @@ -164,6 +174,21 @@ describe('feathers-authentication-jwt', () => {
passportJWT.Strategy.restore();
});

it('pulls options from global config with custom name', () => {
sinon.spy(passportJWT, 'Strategy');
let authOptions = app.get('auth');
authOptions.custom = { entity: 'device' };
app.set('auth', authOptions);

app.configure(jwt({ name: 'custom' }));
app.setup();

expect(passportJWT.Strategy.getCall(0).args[0].entity).to.equal('device');
expect(passportJWT.Strategy.getCall(0).args[0].bodyKey).to.equal('accessToken');

passportJWT.Strategy.restore();
});

describe('custom Verifier', () => {
it('throws an error if a verify function is missing', () => {
expect(() => {
Expand All @@ -190,7 +215,7 @@ describe('feathers-authentication-jwt', () => {
class CustomVerifier extends Verifier {
verify (req, payload, done) {
expect(payload.id).to.equal(Payload.id);
done(null, Payload);
done(null, payload, Payload);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/authentication-jwt/test/integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('integration', () => {
return hook => {
const app = hook.app;
const id = hook.result.id;
return app.passport.createJWT({ id }, app.get('auth')).then(accessToken => {
return app.passport.createJWT({ userId: id }, app.get('auth')).then(accessToken => {
hook.result.accessToken = accessToken;
return Promise.resolve(hook);
});
Expand Down
10 changes: 5 additions & 5 deletions packages/authentication-jwt/test/verifier.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ describe('Verifier', () => {
});

describe('verify', () => {
describe('when id is present in payload', () => {
describe('when userId is present in payload', () => {
it('calls get on the provided service', done => {
verifier.verify({}, { id: 1 }, () => {
verifier.verify({}, { userId: 1 }, () => {
expect(service.get).to.have.been.calledOnce;
expect(service.get).to.have.been.calledWith(1);
done();
});
});

it('returns the payload', done => {
const payload = { id: 1 };
const payload = { userId: 1 };
verifier.verify({}, payload, (error, result) => {
expect(error).to.equal(null);
expect(result.payload).to.deep.equal(payload);
Expand All @@ -84,7 +84,7 @@ describe('Verifier', () => {
});

it('returns the entity', done => {
verifier.verify({}, { id: 1 }, (error, result) => {
verifier.verify({}, { userId: 1 }, (error, result) => {
expect(error).to.equal(null);
expect(result.email).to.deep.equal(user.email);
done();
Expand All @@ -100,7 +100,7 @@ describe('Verifier', () => {

options.service = service;
const erroringVerifier = new Verifier(app, options);
const payload = { id: 1 };
const payload = { userId: 1 };
erroringVerifier.verify({}, payload, (error, result) => {
expect(error).to.equal(null);
expect(result.payload).to.deep.equal(payload);
Expand Down

0 comments on commit 094306c

Please sign in to comment.