Skip to content

Commit 1afac41

Browse files
ClearlyClaireGargron
authored andcommitted
Improve streaming server security (mastodon#10818)
* Check OAuth token scopes in the streaming API * Use Sec-WebSocket-Protocol instead of query string to pass WebSocket token Inspired by kubevirt/kubevirt#1242
1 parent 1da5dca commit 1afac41

2 files changed

Lines changed: 44 additions & 12 deletions

File tree

app/javascript/mastodon/stream.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,7 @@ export function connectStream(path, pollingRefresh = null, callbacks = () => ({
7171
export default function getStream(streamingAPIBaseURL, accessToken, stream, { connected, received, disconnected, reconnected }) {
7272
const params = [ `stream=${stream}` ];
7373

74-
if (accessToken !== null) {
75-
params.push(`access_token=${accessToken}`);
76-
}
77-
78-
const ws = new WebSocketClient(`${streamingAPIBaseURL}/api/v1/streaming/?${params.join('&')}`);
74+
const ws = new WebSocketClient(`${streamingAPIBaseURL}/api/v1/streaming/?${params.join('&')}`, accessToken);
7975

8076
ws.onopen = connected;
8177
ws.onmessage = e => received(JSON.parse(e.data));

streaming/index.js

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,14 @@ const startWorker = (workerId) => {
195195
next();
196196
};
197197

198-
const accountFromToken = (token, req, next) => {
198+
const accountFromToken = (token, allowedScopes, req, next) => {
199199
pgPool.connect((err, client, done) => {
200200
if (err) {
201201
next(err);
202202
return;
203203
}
204204

205-
client.query('SELECT oauth_access_tokens.resource_owner_id, users.account_id, users.chosen_languages FROM oauth_access_tokens INNER JOIN users ON oauth_access_tokens.resource_owner_id = users.id WHERE oauth_access_tokens.token = $1 AND oauth_access_tokens.revoked_at IS NULL LIMIT 1', [token], (err, result) => {
205+
client.query('SELECT oauth_access_tokens.resource_owner_id, users.account_id, users.chosen_languages, oauth_access_tokens.scopes FROM oauth_access_tokens INNER JOIN users ON oauth_access_tokens.resource_owner_id = users.id WHERE oauth_access_tokens.token = $1 AND oauth_access_tokens.revoked_at IS NULL LIMIT 1', [token], (err, result) => {
206206
done();
207207

208208
if (err) {
@@ -218,18 +218,29 @@ const startWorker = (workerId) => {
218218
return;
219219
}
220220

221+
const scopes = result.rows[0].scopes.split(' ');
222+
223+
if (allowedScopes.size > 0 && !scopes.some(scope => allowedScopes.includes(scope))) {
224+
err = new Error('Access token does not cover required scopes');
225+
err.statusCode = 401;
226+
227+
next(err);
228+
return;
229+
}
230+
221231
req.accountId = result.rows[0].account_id;
222232
req.chosenLanguages = result.rows[0].chosen_languages;
233+
req.allowNotifications = scopes.some(scope => ['read', 'read:notifications'].includes(scope));
223234

224235
next();
225236
});
226237
});
227238
};
228239

229-
const accountFromRequest = (req, next, required = true) => {
240+
const accountFromRequest = (req, next, required = true, allowedScopes = ['read']) => {
230241
const authorization = req.headers.authorization;
231242
const location = url.parse(req.url, true);
232-
const accessToken = location.query.access_token;
243+
const accessToken = location.query.access_token || req.headers['sec-websocket-protocol'];
233244

234245
if (!authorization && !accessToken) {
235246
if (required) {
@@ -246,7 +257,7 @@ const startWorker = (workerId) => {
246257

247258
const token = authorization ? authorization.replace(/^Bearer /, '') : accessToken;
248259

249-
accountFromToken(token, req, next);
260+
accountFromToken(token, allowedScopes, req, next);
250261
};
251262

252263
const PUBLIC_STREAMS = [
@@ -261,6 +272,16 @@ const startWorker = (workerId) => {
261272
const wsVerifyClient = (info, cb) => {
262273
const location = url.parse(info.req.url, true);
263274
const authRequired = !PUBLIC_STREAMS.some(stream => stream === location.query.stream);
275+
const allowedScopes = [];
276+
277+
if (authRequired) {
278+
allowedScopes.push('read');
279+
if (location.query.stream === 'user:notification') {
280+
allowedScopes.push('read:notifications');
281+
} else {
282+
allowedScopes.push('read:statuses');
283+
}
284+
}
264285

265286
accountFromRequest(info.req, err => {
266287
if (!err) {
@@ -269,7 +290,7 @@ const startWorker = (workerId) => {
269290
log.error(info.req.requestId, err.toString());
270291
cb(false, 401, 'Unauthorized');
271292
}
272-
}, authRequired);
293+
}, authRequired, allowedScopes);
273294
};
274295

275296
const PUBLIC_ENDPOINTS = [
@@ -286,7 +307,18 @@ const startWorker = (workerId) => {
286307
}
287308

288309
const authRequired = !PUBLIC_ENDPOINTS.some(endpoint => endpoint === req.path);
289-
accountFromRequest(req, next, authRequired);
310+
const allowedScopes = [];
311+
312+
if (authRequired) {
313+
allowedScopes.push('read');
314+
if (req.path === '/api/v1/streaming/user/notification') {
315+
allowedScopes.push('read:notifications');
316+
} else {
317+
allowedScopes.push('read:statuses');
318+
}
319+
}
320+
321+
accountFromRequest(req, next, authRequired, allowedScopes);
290322
};
291323

292324
const errorMiddleware = (err, req, res, {}) => {
@@ -339,6 +371,10 @@ const startWorker = (workerId) => {
339371
return;
340372
}
341373

374+
if (event === 'notification' && !req.allowNotifications) {
375+
return;
376+
}
377+
342378
// Only messages that may require filtering are statuses, since notifications
343379
// are already personalized and deletes do not matter
344380
if (!needsFiltering || event !== 'update') {

0 commit comments

Comments
 (0)