Skip to content

Commit 03391f7

Browse files
authored
Merge pull request #3158 from strongloop/backport/fix-logout-without-token
Fix logout to handle no or missing accessToken
2 parents 2ade55e + 1dac9ad commit 03391f7

File tree

3 files changed

+43
-6
lines changed

3 files changed

+43
-6
lines changed

common/models/user.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -293,13 +293,23 @@ module.exports = function(User) {
293293

294294
User.logout = function(tokenId, fn) {
295295
fn = fn || utils.createPromiseCallback();
296-
this.relations.accessTokens.modelTo.findById(tokenId, function(err, accessToken) {
296+
297+
if (!tokenId) {
298+
var err = new Error(g.f('{{accessToken}} is required to logout'));
299+
err.status = 401;
300+
process.nextTick(function() { fn(err); });
301+
return fn.promise;
302+
}
303+
304+
this.relations.accessTokens.modelTo.destroyById(tokenId, function(err, info) {
297305
if (err) {
298306
fn(err);
299-
} else if (accessToken) {
300-
accessToken.destroy(fn);
307+
} else if ('count' in info && info.count === 0) {
308+
err = new Error(g.f('Could not find {{accessToken}}'));
309+
err.status = 401;
310+
fn(err);
301311
} else {
302-
fn(new Error(g.f('could not find {{accessToken}}')));
312+
fn();
303313
}
304314
});
305315
return fn.promise;
@@ -743,10 +753,10 @@ module.exports = function(User) {
743753
{
744754
description: 'Logout a user with access token.',
745755
accepts: [
746-
{arg: 'access_token', type: 'string', required: true, http: function(ctx) {
756+
{arg: 'access_token', type: 'string', http: function(ctx) {
747757
var req = ctx && ctx.req;
748758
var accessToken = req && req.accessToken;
749-
var tokenID = accessToken && accessToken.id;
759+
var tokenID = accessToken ? accessToken.id : undefined;
750760
return tokenID;
751761
}, description: 'Do not supply this argument, it is automatically extracted ' +
752762
'from request headers.',

test/user.integration.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,17 @@ describe('users - integration', function() {
148148
});
149149
});
150150
});
151+
152+
it('returns 401 on logout with no access token', function(done) {
153+
this.post('/api/users/logout')
154+
.expect(401, done);
155+
});
156+
157+
it('returns 401 on logout with invalid access token', function(done) {
158+
this.post('/api/users/logout')
159+
.set('Authorization', 'unknown-token')
160+
.expect(401, done);
161+
});
151162
});
152163

153164
describe('sub-user', function() {

test/user.test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,22 @@ describe('User', function() {
11861186
}
11871187
});
11881188

1189+
it('fails when accessToken is not provided', function(done) {
1190+
User.logout(undefined, function(err) {
1191+
expect(err).to.have.property('message');
1192+
expect(err).to.have.property('status', 401);
1193+
done();
1194+
});
1195+
});
1196+
1197+
it('fails when accessToken is not found', function(done) {
1198+
User.logout('expired-access-token', function(err) {
1199+
expect(err).to.have.property('message');
1200+
expect(err).to.have.property('status', 401);
1201+
done();
1202+
});
1203+
});
1204+
11891205
function verify(token, done) {
11901206
assert(token);
11911207

0 commit comments

Comments
 (0)