Skip to content

Commit

Permalink
feat(#6530): add rate limiting for authentication requests
Browse files Browse the repository at this point in the history
  • Loading branch information
garethbowen authored Nov 21, 2023
1 parent f4a5f11 commit 1332879
Show file tree
Hide file tree
Showing 19 changed files with 539 additions and 201 deletions.
11 changes: 11 additions & 0 deletions api/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"pouchdb-mapreduce": "^7.3.1",
"prometheus-api-metrics": "^3.2.2",
"properties": "^1.2.1",
"rate-limiter-flexible": "^3.0.2",
"request": "^2.88.2",
"request-promise-native": "^1.0.9",
"sanitize-html": "^2.11.0",
Expand Down
12 changes: 2 additions & 10 deletions api/src/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,12 @@ module.exports = {
if (!authHeader || !authHeader.startsWith('Basic ')) {
return false;
}

let username;
let password;

try {
[username, password] = Buffer.from(authHeader.split(' ')[1], 'base64').toString().split(':');
const [username, password] = Buffer.from(authHeader.split(' ')[1], 'base64').toString().split(':');
return { username, password };
} catch (err) {
throw Error('Corrupted Auth header');
}

return {
username: username,
password: password
};
},

/**
Expand Down
72 changes: 40 additions & 32 deletions api/src/controllers/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const cookie = require('../services/cookie');
const brandingService = require('../services/branding');
const translations = require('../translations');
const template = require('../services/template');
const rateLimitService = require('../services/rate-limit');
const serverUtils = require('../server-utils');

const templates = {
login: {
Expand Down Expand Up @@ -269,6 +271,24 @@ const renderLogin = (req) => {
return render('login', req);
};

const login = async (req, res) => {
try {
const sessionRes = await createSession(req);
if (sessionRes.statusCode !== 200) {
res.status(sessionRes.statusCode).json({ error: 'Not logged in' });
} else {
const redirectUrl = await setCookies(req, res, sessionRes);
res.status(302).send(redirectUrl);
}
} catch (e) {
if (e.status === 401) {
return res.status(401).json({ error: e.error });
}
logger.error('Error logging in: %o', e);
res.status(500).json({ error: 'Unexpected error logging in' });
}
};

module.exports = {
renderLogin,

Expand All @@ -285,26 +305,12 @@ module.exports = {
})
.catch(next);
},
post: (req, res) => {
return createSession(req)
.then(sessionRes => {
if (sessionRes.statusCode !== 200) {
res.status(sessionRes.statusCode).json({ error: 'Not logged in' });
return;
}
return setCookies(req, res, sessionRes)
.then(redirectUrl => res.status(302).send(redirectUrl))
.catch(err => {
if (err.status === 401) {
return res.status(err.status).json({ error: err.error });
}
throw err;
});
})
.catch(err => {
logger.error('Error logging in: %o', err);
res.status(500).json({ error: 'Unexpected error logging in' });
});
post: async (req, res) => {
const limited = await rateLimitService.isLimited(req);
if (limited) {
return serverUtils.rateLimited(req, res);
}
await login(req, res);
},
getIdentity: (req, res) => {
res.type('application/json');
Expand All @@ -321,17 +327,19 @@ module.exports = {
},

tokenGet: (req, res, next) => renderTokenLogin(req, res).catch(next),
tokenPost: (req, res, next) => {
return auth
.getUserCtx(req)
.then(userCtx => {
return res.status(302).send(getRedirectUrl(userCtx));
})
.catch(err => {
if (err.code === 401) {
return loginByToken(req, res);
}
next(err);
});
tokenPost: async (req, res, next) => {
const limited = await rateLimitService.isLimited(req);
if (limited) {
return serverUtils.rateLimited(req, res);
}
try {
const userCtx = await auth.getUserCtx(req);
return res.status(302).send(getRedirectUrl(userCtx));
} catch (e) {
if (e.code === 401) {
return loginByToken(req, res);
}
next(e);
}
},
};
34 changes: 34 additions & 0 deletions api/src/middleware/rate-limiter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const auth = require('../auth');
const rateLimitService = require('../services/rate-limit');
const serverUtils = require('../server-utils');

const registerReqFinishHandler = (req, res) => {
res.on('finish', () => {
if (res.statusCode === 401 || res.statusCode === 429) {
// log in failed - punish user
rateLimitService.consume(req);
}
});
};

const isLoggingIn = (req) => {
const basicAuth = auth.basicAuthCredentials(req);
return req.body?.user || basicAuth?.username;
};

const shouldLimit = async (req) => {
if (isLoggingIn(req)) {
return await rateLimitService.isLimited(req);
}
return false;
};

const rateLimiterMiddleware = async (req, res, next) => {
if (await shouldLimit(req)) {
return serverUtils.rateLimited(req, res);
}
registerReqFinishHandler(req, res);
next();
};

module.exports = rateLimiterMiddleware;
1 change: 1 addition & 0 deletions api/src/public/login/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const request = function(method, url, payload, callback) {
};
xmlhttp.open(method, url, true);
xmlhttp.setRequestHeader('Content-Type', 'application/json');
xmlhttp.setRequestHeader('Accept', 'application/json');
xmlhttp.send(payload);
};

Expand Down
2 changes: 2 additions & 0 deletions api/src/routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const db = require('./db');
const path = require('path');
const auth = require('./auth');
const prometheusMiddleware = require('prometheus-api-metrics');
const rateLimiterMiddleware = require('./middleware/rate-limiter');
const logger = require('./logger');
const isClientHuman = require('./is-client-human');
const target = 'http://' + environment.host + ':' + environment.port;
Expand Down Expand Up @@ -161,6 +162,7 @@ app.use(
':res[content-length] :response-time ms'
)
);
app.use(rateLimiterMiddleware);

app.use(
helmet({
Expand Down
4 changes: 4 additions & 0 deletions api/src/server-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ module.exports = {
}
},

rateLimited: (req, res) => {
respond(req, res, 429, 'Too Many Requests');
},

/**
* Only to be used when handling unexpected errors.
*/
Expand Down
58 changes: 58 additions & 0 deletions api/src/services/rate-limit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
const { RateLimiterMemory } = require('rate-limiter-flexible');
const auth = require('../auth');

const failedLoginLimit = new RateLimiterMemory({
keyPrefix: 'failed-login',
points: 10, // 10 requests
duration: 10, // per 10 seconds
});

const isLimitedKey = async (key) => {
const limit = await failedLoginLimit.get(key);
return limit && limit.remainingPoints <= 0;
};

const consumeKey = async (key) => {
try {
await failedLoginLimit.consume(key);
} catch (e) {
// ignore - the limit has already been reached
}
};

const getKeys = (req) => {
const keys = [ req.ip ];

if (req.body?.user) {
keys.push(req.body.user);
}
if (req.body?.password) {
keys.push(req.body.password);
}
const basicAuth = auth.basicAuthCredentials(req);
if (basicAuth?.username) {
keys.push(basicAuth.username);
}
if (basicAuth?.password) {
keys.push(basicAuth.password);
}
return keys;
};

module.exports = {
isLimited: async req => {
const keys = getKeys(req);
for (const key of keys) {
if (await isLimitedKey(key)) {
return true;
}
}
return false;
},
consume: async req => {
const keys = getKeys(req);
for (const key of keys) {
await consumeKey(key);
}
}
};
34 changes: 31 additions & 3 deletions api/tests/mocha/controllers/login.spec.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
const rewire = require('rewire');
const _ = require('lodash');
const fs = require('fs');
const chai = require('chai');
const sinon = require('sinon');
const request = require('request-promise-native');

const environment = require('../../../src/environment');
const auth = require('../../../src/auth');
const cookie = require('../../../src/services/cookie');
const branding = require('../../../src/services/branding');
const rateLimit = require('../../../src/services/rate-limit');
const db = require('../../../src/db').medic;
const translations = require('../../../src/translations');
const privacyPolicy = require('../../../src/services/privacy-policy');
const sinon = require('sinon');
const config = require('../../../src/config');
const { tokenLogin, roles, users } = require('@medic/user-management')(config, db);
const request = require('request-promise-native');
const template = require('../../../src/services/template');
const fs = require('fs');
const serverUtils = require('../../../src/server-utils');

let controller;

Expand Down Expand Up @@ -53,6 +56,9 @@ describe('login controller', () => {

sinon.stub(roles, 'isOnlineOnly').returns(false);
sinon.stub(privacyPolicy, 'exists').resolves(false);

sinon.stub(rateLimit, 'isLimited').returns(false);
sinon.stub(serverUtils, 'rateLimited').resolves();
});

afterEach(() => {
Expand Down Expand Up @@ -337,6 +343,17 @@ describe('login controller', () => {
});
});

it('should send 429 when rate limited', () => {
rateLimit.isLimited.returns(true);
sinon.stub(auth, 'getUserCtx');
return controller.tokenPost(req, res).then(() => {
chai.expect(rateLimit.isLimited.callCount).to.equal(1);
chai.expect(rateLimit.isLimited.args[0][0]).to.equal(req);
chai.expect(serverUtils.rateLimited.callCount).to.equal(1);
chai.expect(auth.getUserCtx.callCount).to.equal(0);
});
});

it('should send error when error thrown while validating token', () => {
sinon.stub(auth, 'getUserCtx').rejects({ code: 401 });
sinon.stub(tokenLogin, 'isTokenLoginEnabled').returns(true);
Expand Down Expand Up @@ -483,6 +500,17 @@ describe('login controller', () => {
});
});

it('returns 429 when rate limited', () => {
const post = sinon.stub(request, 'post');
rateLimit.isLimited.returns(true);
return controller.post(req, res).then(() => {
chai.expect(rateLimit.isLimited.callCount).to.equal(1);
chai.expect(rateLimit.isLimited.args[0][0]).to.equal(req);
chai.expect(serverUtils.rateLimited.callCount).to.equal(1);
chai.expect(post.callCount).to.equal(0);
});
});

it('should retry getting userCtx 10 times', async () => {
req.body = { user: 'sharon', password: 'p4ss', locale: 'fr' };
const postResponse = {
Expand Down
Loading

0 comments on commit 1332879

Please sign in to comment.