Skip to content

Commit 4fa47d0

Browse files
unocelliunocelli
authored
Security hardening for JWT lifecycle and Socket.IO admin operations (#2379)
* security: jwt lifecycle GHSA-rg7m-xwqc-mjw6 * security: SSRF hardening for `device-webapi-request` GHSA-wrg6-49wh-46pw * security: Unauthenticated Socket.IO read events GHSA-rh5p-m38p-2w75 * security: clean
1 parent d27abc3 commit 4fa47d0

9 files changed

Lines changed: 460 additions & 41 deletions

File tree

‎client/src/app/_services/auth.service.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,24 @@ export class AuthService {
113113
return false;
114114
}
115115

116-
setNewToken(token: string) {
116+
setNewToken(token: string, userData?: Partial<UserProfile>) {
117117
if (!this.currentUser) {
118118
return;
119119
}
120+
if (userData) {
121+
this.currentUser.username = userData.username ?? this.currentUser.username;
122+
this.currentUser.fullname = userData.fullname ?? this.currentUser.fullname;
123+
this.currentUser.groups = userData.groups ?? this.currentUser.groups;
124+
this.currentUser.info = userData.info ?? this.currentUser.info;
125+
if (this.currentUser.info) {
126+
this.currentUser.infoRoles = JSON.parse(this.currentUser.info)?.roles;
127+
} else {
128+
this.currentUser.infoRoles = null;
129+
}
130+
}
120131
this.currentUser.token = token;
121132
this.saveUserToken(this.currentUser);
133+
this.currentUser$.next(this.currentUser);
122134
}
123135

124136
// to check by page refresh
@@ -180,8 +192,8 @@ export class AuthService {
180192
...(this.currentUser || {}),
181193
username: result.data.username || this.currentUser?.username,
182194
fullname: result.data.fullname || this.currentUser?.fullname,
183-
groups: result.data.groups || this.currentUser?.groups,
184-
info: result.data.info || this.currentUser?.info,
195+
groups: result.data.groups ?? this.currentUser?.groups,
196+
info: result.data.info ?? this.currentUser?.info,
185197
token: result.data.token
186198
};
187199
if (refreshed.info) {

‎client/src/app/_services/heartbeat.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class HeartbeatService {
2929
this.heartbeatSubscription = interval(this.heartbeatInterval).subscribe(() => {
3030
this.server.heartbeat(this.activity).subscribe(res => {
3131
if (res?.message === 'tokenRefresh' && res?.token) {
32-
this.authService.setNewToken(res.token);
32+
this.authService.setNewToken(res.token, res.data);
3333
} else if (res?.message === 'guest' && res?.token) {
3434
this.authService.signOut();
3535
}

‎server/api/auth/index.js‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ function buildAccessToken(user) {
5252
}
5353

5454
function buildRefreshToken(user) {
55-
return jwt.sign({ id: user.username, groups: user.groups, type: 'refresh' }, secretCode, { expiresIn: refreshTokenExpiresIn });
55+
return jwt.sign({ id: user.username, type: 'refresh' }, secretCode, { expiresIn: refreshTokenExpiresIn });
5656
}
5757

5858
function setRefreshCookie(res, token) {
@@ -155,20 +155,18 @@ module.exports = {
155155
return res.status(401).json({ status: 'error', message: 'Invalid refresh token' });
156156
}
157157

158-
let userData = null;
159-
try {
160-
const users = await runtime.users.getUsers({ username: decoded.id });
161-
if (users && users.length) {
162-
userData = users[0];
163-
}
164-
} catch (err) {
165-
runtime.logger.error(`api refresh: user lookup failed ${err}`);
158+
const users = await runtime.users.getUsers({ username: decoded.id });
159+
if (!users || !users.length) {
160+
clearRefreshCookie(res);
161+
return res.status(401).json({ status: 'error', message: 'Invalid refresh token' });
166162
}
167163

164+
const userData = users[0];
165+
168166
const user = {
169167
username: decoded.id,
170168
fullname: userData?.fullname,
171-
groups: userData?.groups || decoded.groups,
169+
groups: userData?.groups,
172170
info: userData?.info
173171
};
174172

‎server/api/index.js‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ function init(_server, _runtime) {
185185
/**
186186
* GET Heartbeat to check token
187187
*/
188-
apiApp.post('/api/heartbeat', authMiddleware, function (req, res) {
188+
apiApp.post('/api/heartbeat', authMiddleware, async function (req, res) {
189189

190190
if (!runtime.settings.secureEnabled) {
191191
return res.end();
@@ -200,10 +200,17 @@ function init(_server, _runtime) {
200200
});
201201
}
202202

203+
const currentUser = await getCurrentTokenUser(req);
204+
if (!currentUser) {
205+
return res.status(401).json({ error: 'unauthorized_error', message: 'Unauthorized!' });
206+
}
207+
208+
req.userGroups = currentUser.groups;
203209
const token = authJwt.getNewTokenFromRequest(req);
204210
return res.status(200).json({
205211
message: 'tokenRefresh',
206-
token
212+
token,
213+
data: currentUser
207214
});
208215
}
209216

@@ -279,6 +286,28 @@ function mergeUserSettings(settings) {
279286
}
280287
}
281288

289+
async function getCurrentTokenUser(req) {
290+
if (!req.isAuthenticated || authJwt.isGuestUser(req.userId, req.userGroups)) {
291+
return null;
292+
}
293+
294+
try {
295+
const users = await runtime.users.getUsers({ username: req.userId });
296+
if (users && users.length && !utils.isNullOrUndefined(users[0].groups)) {
297+
return {
298+
username: users[0].username,
299+
fullname: users[0].fullname,
300+
groups: users[0].groups,
301+
info: users[0].info
302+
};
303+
}
304+
} catch (err) {
305+
runtime.logger.error(`api heartbeat: user lookup failed ${err}`);
306+
}
307+
308+
return null;
309+
}
310+
282311
function verifyGroups(req) {
283312
if (runtime.settings && runtime.settings.secureEnabled) {
284313
if (req.apiKey) {
@@ -288,6 +317,9 @@ function verifyGroups(req) {
288317
return (runtime.settings.userRole) ? null : 0;
289318
}
290319
const userInfo = runtime.users.getUserCache(req.userId);
320+
if (req.isAuthenticated && !authJwt.isGuestUser(req.userId, req.userGroups) && !userInfo) {
321+
return null;
322+
}
291323
return (runtime.settings.userRole && req.userId !== 'admin') ? userInfo : userInfo ? userInfo.groups : req.userGroups;
292324
} else {
293325
return authJwt.adminGroups[0];

‎server/api/jwt-helper.js‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -72,27 +72,6 @@ function verifyToken (req, res, next) {
7272
}
7373

7474
function requireAuth (req, res, next) {
75-
// Allow requests from FUXA interface (iframe embedding)
76-
// Check for common FUXA referer patterns
77-
const referer = req.headers.referer;
78-
if (referer) {
79-
// Allow if referer is from the same host (to support IP access without specific paths)
80-
const requestHost = req.headers.host;
81-
if (referer.startsWith(`http://${requestHost}`) || referer.startsWith(`https://${requestHost}`)) {
82-
return next();
83-
}
84-
// Allow if referer contains common FUXA paths or is from the same server
85-
const fuxaPatterns = [
86-
'/fuxa', '/editor', '/viewer', '/lab', '/home',
87-
'localhost:', '127.0.0.1:', '0.0.0.0:'
88-
];
89-
const hasFuxaReferer = fuxaPatterns.some(pattern => referer.includes(pattern));
90-
if (hasFuxaReferer) {
91-
return next();
92-
}
93-
}
94-
95-
// For direct access, require authentication
9675
let token = req.headers['x-access-token'];
9776

9877
if (!token) {

‎server/api/users/index.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,22 @@ var runtime;
88
var secureFnc;
99
var checkGroupsFnc;
1010

11+
function sanitizeUser(user) {
12+
if (!user || typeof user !== 'object') {
13+
return user;
14+
}
15+
const sanitized = Object.assign({}, user);
16+
delete sanitized.password;
17+
return sanitized;
18+
}
19+
20+
function sanitizeUsers(users) {
21+
if (Array.isArray(users)) {
22+
return users.map(sanitizeUser);
23+
}
24+
return sanitizeUser(users);
25+
}
26+
1127
module.exports = {
1228
init: function (_runtime, _secureFnc, _checkGroupsFnc) {
1329
runtime = _runtime;
@@ -40,7 +56,7 @@ module.exports = {
4056
// res.header("Access-Control-Allow-Origin", "*");
4157
// res.header("Access-Control-Allow-Headers", "Origin, X-Requested-With, Content-Type, Accept");
4258
if (result) {
43-
res.json(result);
59+
res.json(sanitizeUsers(result));
4460
} else {
4561
res.end();
4662
}
@@ -185,4 +201,4 @@ module.exports = {
185201
});
186202
return usersApp;
187203
}
188-
}
204+
}

‎server/runtime/index.js‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ function isSocketWriteAuthorized(socket) {
4040
return !!(socket && socket.isAuthenticated);
4141
}
4242

43+
function isSocketAdminAuthorized(socket) {
44+
if (!settings || !settings.secureEnabled) {
45+
return true;
46+
}
47+
return !!(socket && socket.isAuthenticated && api?.authJwt?.haveAdminPermission(socket.userGroups));
48+
}
49+
4350
function init(_io, _api, _settings, _log, eventsMain) {
4451
io = _io;
4552
settings = _settings;
@@ -197,6 +204,10 @@ function init(_io, _api, _settings, _log, eventsMain) {
197204
// client ask device browse
198205
socket.on(Events.IoEventTypes.DEVICE_BROWSE, (message) => {
199206
try {
207+
if (!isSocketAdminAuthorized(socket)) {
208+
logger.warn(`${Events.IoEventTypes.DEVICE_BROWSE}: unauthorized request from ${socket.userId || 'guest'}`);
209+
return;
210+
}
200211
if (message) {
201212
if (message.device) {
202213
devices.browseDevice(message.device, message.node, function (nodes) {
@@ -218,6 +229,10 @@ function init(_io, _api, _settings, _log, eventsMain) {
218229
// client ask device node attribute
219230
socket.on(Events.IoEventTypes.DEVICE_NODE_ATTRIBUTE, (message) => {
220231
try {
232+
if (!isSocketAdminAuthorized(socket)) {
233+
logger.warn(`${Events.IoEventTypes.DEVICE_NODE_ATTRIBUTE}: unauthorized request from ${socket.userId || 'guest'}`);
234+
return;
235+
}
221236
if (message) {
222237
if (message.device) {
223238
devices.readNodeAttribute(message.device, message.node).then(result => {
@@ -278,6 +293,10 @@ function init(_io, _api, _settings, _log, eventsMain) {
278293
// client ask host interfaces
279294
socket.on(Events.IoEventTypes.HOST_INTERFACES, (message) => {
280295
try {
296+
if (!isSocketAdminAuthorized(socket)) {
297+
logger.warn(`${Events.IoEventTypes.HOST_INTERFACES}: unauthorized request from ${socket.userId || 'guest'}`);
298+
return;
299+
}
281300
if (message === 'get') {
282301
message = {};
283302
utils.getHostInterfaces().then(result => {
@@ -300,7 +319,7 @@ function init(_io, _api, _settings, _log, eventsMain) {
300319
// client ask device webapi request and return result
301320
socket.on(Events.IoEventTypes.DEVICE_WEBAPI_REQUEST, (message) => {
302321
try {
303-
if (!isSocketWriteAuthorized(socket)) {
322+
if (!isSocketAdminAuthorized(socket)) {
304323
logger.warn(`${Events.IoEventTypes.DEVICE_WEBAPI_REQUEST}: unauthorized request from ${socket.userId || 'guest'}`);
305324
return;
306325
}
@@ -326,6 +345,10 @@ function init(_io, _api, _settings, _log, eventsMain) {
326345
// client ask device tags configurtions, used for connections that load tags dinamically (webapi)
327346
socket.on(Events.IoEventTypes.DEVICE_TAGS_REQUEST, (message) => {
328347
try {
348+
if (!isSocketAdminAuthorized(socket)) {
349+
logger.warn(`${Events.IoEventTypes.DEVICE_TAGS_REQUEST}: unauthorized request from ${socket.userId || 'guest'}`);
350+
return;
351+
}
329352
if (message && message.deviceId) {
330353
devices.getDeviceTagsResult(message.deviceId).then(result => {
331354
message.result = result;

‎server/runtime/users/index.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ function removeUsers(username) {
9090
return new Promise(function (resolve, reject) {
9191
if (username) {
9292
usrstorage.removeUser(username).then(() => {
93+
usersMap.delete(username);
9394
resolve();
9495
}).catch(function (err) {
9596
logger.error(`users.usrstorage-remove-users failed! ${err}`);
@@ -207,4 +208,4 @@ module.exports = {
207208
removeRoles: removeRoles,
208209
findOne: findOne,
209210
getUserCache: getUserCache
210-
};
211+
};

0 commit comments

Comments
 (0)