Fix logout to handle no or missing accessToken
Return 401 when the request does not provide any accessToken argument or the token was not found. Also simplify the implementation of the `logout` method to make only a single database call (`deleteById`) instead of `findById` + `delete`.
This commit is contained in:
parent
82d6ede086
commit
d45c1ae7bb
|
@ -292,13 +292,23 @@ module.exports = function(User) {
|
||||||
|
|
||||||
User.logout = function(tokenId, fn) {
|
User.logout = function(tokenId, fn) {
|
||||||
fn = fn || utils.createPromiseCallback();
|
fn = fn || utils.createPromiseCallback();
|
||||||
this.relations.accessTokens.modelTo.findById(tokenId, function(err, accessToken) {
|
|
||||||
|
if (!tokenId) {
|
||||||
|
var err = new Error(g.f('{{accessToken}} is required to logout'));
|
||||||
|
err.status = 401;
|
||||||
|
process.nextTick(fn, err);
|
||||||
|
return fn.promise;
|
||||||
|
}
|
||||||
|
|
||||||
|
this.relations.accessTokens.modelTo.destroyById(tokenId, function(err, info) {
|
||||||
if (err) {
|
if (err) {
|
||||||
fn(err);
|
fn(err);
|
||||||
} else if (accessToken) {
|
} else if ('count' in info && info.count === 0) {
|
||||||
accessToken.destroy(fn);
|
var err = new Error(g.f('Could not find {{accessToken}}'));
|
||||||
|
err.status = 401;
|
||||||
|
fn(err);
|
||||||
} else {
|
} else {
|
||||||
fn(new Error(g.f('could not find {{accessToken}}')));
|
fn();
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
return fn.promise;
|
return fn.promise;
|
||||||
|
@ -753,10 +763,10 @@ module.exports = function(User) {
|
||||||
{
|
{
|
||||||
description: 'Logout a user with access token.',
|
description: 'Logout a user with access token.',
|
||||||
accepts: [
|
accepts: [
|
||||||
{arg: 'access_token', type: 'string', required: true, http: function(ctx) {
|
{arg: 'access_token', type: 'string', http: function(ctx) {
|
||||||
var req = ctx && ctx.req;
|
var req = ctx && ctx.req;
|
||||||
var accessToken = req && req.accessToken;
|
var accessToken = req && req.accessToken;
|
||||||
var tokenID = accessToken && accessToken.id;
|
var tokenID = accessToken ? accessToken.id : undefined;
|
||||||
|
|
||||||
return tokenID;
|
return tokenID;
|
||||||
}, description: 'Do not supply this argument, it is automatically extracted ' +
|
}, description: 'Do not supply this argument, it is automatically extracted ' +
|
||||||
|
|
|
@ -106,6 +106,17 @@ describe('users - integration', function() {
|
||||||
.expect(200, done);
|
.expect(200, done);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('returns 401 on logout with no access token', function(done) {
|
||||||
|
this.post('/api/users/logout')
|
||||||
|
.expect(401, done);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns 401 on logout with invalid access token', function(done) {
|
||||||
|
this.post('/api/users/logout')
|
||||||
|
.set('Authorization', 'unknown-token')
|
||||||
|
.expect(401, done);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('sub-user', function() {
|
describe('sub-user', function() {
|
||||||
|
|
|
@ -1193,6 +1193,22 @@ describe('User', function() {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('fails when accessToken is not provided', function(done) {
|
||||||
|
User.logout(undefined, function(err) {
|
||||||
|
expect(err).to.have.property('message');
|
||||||
|
expect(err).to.have.property('status', 401);
|
||||||
|
done();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fails when accessToken is not found', function(done) {
|
||||||
|
User.logout('expired-access-token', function(err) {
|
||||||
|
expect(err).to.have.property('message');
|
||||||
|
expect(err).to.have.property('status', 401);
|
||||||
|
done();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
function verify(token, done) {
|
function verify(token, done) {
|
||||||
assert(token);
|
assert(token);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue