From 0865adaa1eb3f4bc023e42ead001e9c09a9b349a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 18:01:41 +0300 Subject: [PATCH] [fix][core] stop returning credentials from the current user endpoint /o/users/me answered with the caller's member document after deleting only the password, so the response carried the account's api_key and, for members using two factor auth, the stored secret for that factor. The neighbouring reads already treat both as sensitive: getUserById and getAllUsers project api_key away, and the member event payloads delete it with a comment saying it must never be forwarded. This was the one read path that returned it. Answer with a copy of the member that has the password, the api_key and the two factor object removed. Nothing in the product reads the key from here: the dashboard takes it from the server rendered globals, and a member who wants their own key has the /api-key route. Also refuse a token that was restricted to specific applications on this endpoint. It reports the caller's own account and belongs to no application, so a token deliberately limited to some applications has no business reading it. The restriction in verify_token is only compared when the request itself names an application, and this request never does, so without this an app restricted token still reached account level data. Tokens with no application restriction keep working exactly as before, which is what the dashboard and the existing suites use. --- api/parts/mgmt/users.js | 23 +++++++++++++++--- api/utils/requestProcessor.js | 26 +++++++++++++++++++- test/2.api/02.read.user.js | 20 ++++++++++++++++ test/2.api/14.authorize.token.js | 41 ++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/api/parts/mgmt/users.js b/api/parts/mgmt/users.js index 5510c534aa6..7339429076b 100644 --- a/api/parts/mgmt/users.js +++ b/api/parts/mgmt/users.js @@ -24,9 +24,26 @@ var crypto = require('crypto'); * @returns {boolean} true **/ usersApi.getCurrentUser = function(params) { - delete params.member.password; - - common.returnOutput(params, params.member); + //Answer with a copy, so removing fields here cannot affect the member object the rest + //of the request still uses. + var member = Object.assign({}, params.member); + + //The api_key is not scoped: it grants everything its owner can do, on every app they + //can reach. getUserById and getAllUsers already project it away, and the member event + //payloads delete it, so this was the one read path that handed it out. It matters here + //because a request can be authorized by a token rather than by the key itself, and a + //token can be limited to a single app, so returning the key would let a token that is + //limited to one app produce a credential that is limited to nothing. Anyone who needs + //their own key can still read it from the dashboard's /api-key route. + delete member.password; + delete member.api_key; + //Same reasoning for the second factor, whose secret lives on the member document: a + //response carrying both the key and the secret behind the factor protecting it protects + //nothing. The whole object goes, since this endpoint has no consumer that needs it and + //the enabled flag is available from the user listing. + delete member.two_factor_auth; + + common.returnOutput(params, member); return true; }; diff --git a/api/utils/requestProcessor.js b/api/utils/requestProcessor.js index 3287fe1c7dc..db49f3ae61d 100644 --- a/api/utils/requestProcessor.js +++ b/api/utils/requestProcessor.js @@ -1607,7 +1607,31 @@ const processRequest = (params) => { validateUserForGlobalAdmin(params, countlyApi.mgmt.users.getAllUsers); break; case 'me': - validateUserForMgmtReadAPI(countlyApi.mgmt.users.getCurrentUser, params); + validateUserForMgmtReadAPI(function() { + //This endpoint answers with the caller's own account and belongs to no + //application, so a token that was deliberately limited to some + //applications has no business reading it. Without this an app limited + //token still reached account level data, because the app restriction in + //verify_token is only compared when the request itself names an app. + var authToken = params.qstring.auth_token || params.req.headers["countly-token"] || ""; + if (!authToken) { + return countlyApi.mgmt.users.getCurrentUser(params); + } + authorize.read({ + db: common.db, + token: authToken, + callback: function(tokenErr, tokenData) { + //save() stores app as "" when unrestricted and as an array + //otherwise, so a non empty length is what marks a restriction + if (tokenData && tokenData.app && tokenData.app.length) { + common.returnMessage(params, 401, 'Token is restricted to specific applications'); + return false; + } + return countlyApi.mgmt.users.getCurrentUser(params); + } + }); + return true; + }, params); break; case 'id': validateUserForGlobalAdmin(params, countlyApi.mgmt.users.getUserById); diff --git a/test/2.api/02.read.user.js b/test/2.api/02.read.user.js index 7997b673231..6b51ae01077 100644 --- a/test/2.api/02.read.user.js +++ b/test/2.api/02.read.user.js @@ -103,6 +103,26 @@ describe('Initial reading', function() { }); }); }); + describe('Reading users /me does not return credentials', function() { + it('should omit api_key and the second factor secret', function(done) { + request + .get('/o/users/me?api_key=' + API_KEY_ADMIN) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + // the account's own fields are still there + ob.should.have.property('email', testUtils.email); + // but nothing that authenticates as this account + ob.should.not.have.property('api_key'); + ob.should.not.have.property('password'); + ob.should.not.have.property('two_factor_auth'); + done(); + }); + }); + }); describe('Reading users /all', function() { it('should return information', function(done) { request diff --git a/test/2.api/14.authorize.token.js b/test/2.api/14.authorize.token.js index c05bfd2d678..cd06aec1960 100644 --- a/test/2.api/14.authorize.token.js +++ b/test/2.api/14.authorize.token.js @@ -216,6 +216,47 @@ describe('Testing global admin user token', function() { */ }); +describe('Token restricted to an application cannot read account information', function() { + var appScopedToken = ""; + + it('creating a token restricted to one application', function(done) { + authorize.save({ + db: testUtils.db, + multi: true, + owner: testowner, + app: [APP_ID], + callback: function(err, token) { + if (err) { + return done(err); + } + if (!token) { + return done("token not created"); + } + appScopedToken = token; + done(); + } + }); + }); + + it('should refuse /o/users/me, which belongs to no application', function(done) { + request + .get('/o/users/me?auth_token=' + appScopedToken) + .expect(401) + .end(function(err) { + if (err) { + return done(err); + } + done(); + }); + }); + + it('cleaning up the app restricted token', function(done) { + testUtils.db.collection("auth_tokens").remove({_id: appScopedToken}, function() { + done(); + }); + }); +}); + describe('Creating token to allow only paths under /o/users/', function() { it('creating token for user', function(done) { authorize.save({