From 9896f67358fa1055b427849787b9006894eedb11 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Tue, 1 Dec 2020 17:15:48 +0100 Subject: [PATCH] new: [security] New setting Security.username_in_response_header --- app/Controller/AppController.php | 13 +++++++- app/Controller/UsersController.php | 8 ----- app/Model/AuthKey.php | 4 +++ app/Model/Server.php | 12 ++++++- tests/testlive_security.py | 52 ++++++++++++++++++++++++------ 5 files changed, 70 insertions(+), 19 deletions(-) diff --git a/app/Controller/AppController.php b/app/Controller/AppController.php index d8eacf745..075013c3d 100755 --- a/app/Controller/AppController.php +++ b/app/Controller/AppController.php @@ -241,11 +241,21 @@ class AppController extends Controller Configure::write('CurrentUserId', $user['id']); $this->__logAccess($user); - // update script + // If user is site admin, try to run updates if ($user['Role']['perm_site_admin'] || (Configure::read('MISP.live') && !$this->_isRest())) { $this->User->runUpdates(); } + // Put username to response header for webserver or proxy logging + if (Configure::read('Security.username_in_response_header')) { + $headerValue = $user['email']; + if (isset($user['logged_by_authkey']) && $user['logged_by_authkey']) { + $headerValue .= isset($user['authkey_id']) ? "/API/{$user['authkey_id']}" : '/API/default'; + } + $this->response->header('X-Username', $headerValue); + $this->RestResponse->setHeader('X-Username', $headerValue); + } + if (!$this->__verifyUser($user)) { $this->_stop(); // just for sure } @@ -930,6 +940,7 @@ class AppController extends Controller if (!$user['Role']['perm_auth']) { return false; } + $user['logged_by_authkey'] = true; return $user; } diff --git a/app/Controller/UsersController.php b/app/Controller/UsersController.php index a558282bc..7090df547 100644 --- a/app/Controller/UsersController.php +++ b/app/Controller/UsersController.php @@ -51,14 +51,6 @@ class UsersController extends AppController if (!$this->_isSiteAdmin() && $this->Auth->user('id') != $id) { throw new NotFoundException(__('Invalid user or not authorised.')); } - if (!is_numeric($id) && !empty($id)) { - $userId = $this->User->find('first', array( - 'conditions' => array('email' => $id), - 'fields' => array('id') - )); - $id = $userid['User']['id']; - } - $user = $this->User->read(null, $id); $user = $this->User->find('first', array( 'recursive' => -1, 'conditions' => array('User.id' => $id), diff --git a/app/Model/AuthKey.php b/app/Model/AuthKey.php index 9ab1e40f9..60a9d6c9f 100644 --- a/app/Model/AuthKey.php +++ b/app/Model/AuthKey.php @@ -51,6 +51,10 @@ class AuthKey extends AppModel return true; } + /** + * @param string $authkey + * @return array|false + */ public function getAuthUserByAuthKey($authkey) { $start = substr($authkey, 0, 4); diff --git a/app/Model/Server.php b/app/Model/Server.php index a11e0e1ae..59b15ba11 100644 --- a/app/Model/Server.php +++ b/app/Model/Server.php @@ -1603,7 +1603,16 @@ class Server extends AppModel 'test' => 'testBool', 'type' => 'boolean', 'null' => true - ) + ), + 'username_in_response_header' => [ + 'level' => self::SETTING_OPTIONAL, + 'description' => __('When enabled, logged in username will be included in X-Username HTTP response header. This is useful for request logging on webserver/proxy side.'), + 'value' => false, + 'errorMessage' => '', + 'test' => 'testBool', + 'type' => 'boolean', + 'null' => true + ] ), 'SecureAuth' => array( 'branch' => 1, @@ -4934,6 +4943,7 @@ class Server extends AppModel public function dbSchemaDiagnostic() { + $this->AdminSetting = ClassRegistry::init('AdminSetting'); $actualDbVersion = $this->AdminSetting->find('first', array( 'conditions' => array('setting' => 'db_version') ))['AdminSetting']['value']; diff --git a/tests/testlive_security.py b/tests/testlive_security.py index a886c9090..463ee51aa 100644 --- a/tests/testlive_security.py +++ b/tests/testlive_security.py @@ -45,7 +45,7 @@ def check_response(response): raise Exception(response["errors"]) -def login(url: str, email: str, password: str) -> bool: +def login(url: str, email: str, password: str) -> requests.Session: session = requests.Session() r = session.get(url) @@ -77,7 +77,7 @@ def login(url: str, email: str, password: str) -> bool: r = r.json() if email != r["User"]["email"]: raise Exception(r) # logged in as different user - return True + return session class MISPSetting: @@ -166,7 +166,7 @@ class TestSecurity(unittest.TestCase): # Try to connect as user to check if everything works PyMISP(url, cls.test_usr.authkey) # Check if user can login with given password - assert login(url, cls.test_usr.email, cls.test_usr_password) + assert login(url, cls.test_usr.email, cls.test_usr_password) is not False @classmethod def tearDownClass(cls): @@ -302,7 +302,7 @@ class TestSecurity(unittest.TestCase): self.assertFalse(updated_user.disabled) # Try to login - self.assertTrue(login(url, self.test_usr.email, self.test_usr_password)) + self.assertIsInstance(login(url, self.test_usr.email, self.test_usr_password), requests.Session) def test_disabled_user_api_access(self): # Disable user @@ -327,7 +327,7 @@ class TestSecurity(unittest.TestCase): self.assertFalse(login(url, self.test_usr.email, self.test_usr_password)) # Check if user can login with given password - self.assertTrue(login(url, self.test_usr.email, self.test_usr_password)) + self.assertIsInstance(login(url, self.test_usr.email, self.test_usr_password), requests.Session) def test_disabled_misp_api_access(self): with self.__setting("MISP.live", False): @@ -545,7 +545,7 @@ class TestSecurity(unittest.TestCase): logged_in.change_user_password(str(uuid.uuid4())) # Password should be still the same - self.assertTrue(login(url, self.test_usr.email, self.test_usr_password)) + self.assertIsInstance(login(url, self.test_usr.email, self.test_usr_password), requests.Session) def test_change_pw_disabled_different_way(self): with self.__setting("MISP.disable_user_password_change", True): @@ -554,14 +554,14 @@ class TestSecurity(unittest.TestCase): logged_in.update_user({"password": str(uuid.uuid4())}, self.test_usr.id) # Password should be still the same - self.assertTrue(login(url, self.test_usr.email, self.test_usr_password)) + self.assertIsInstance(login(url, self.test_usr.email, self.test_usr_password), requests.Session) def test_change_pw_disabled_by_org_admin(self): with self.__setting("MISP.disable_user_password_change", True): self.org_admin_misp_connector.update_user({"password": str(uuid.uuid4())}, self.test_usr.id) # Password should be still the same - self.assertTrue(login(url, self.test_usr.email, self.test_usr_password)) + self.assertIsInstance(login(url, self.test_usr.email, self.test_usr_password), requests.Session) def test_add_user_by_org_admin(self): user = MISPUser() @@ -790,7 +790,7 @@ class TestSecurity(unittest.TestCase): def test_shibb_form_login(self): with self.__setting(self.__default_shibb_config()): # Form login should still works when no header provided - self.assertTrue(login(url, self.test_usr.email, self.test_usr_password)) + self.assertIsInstance(login(url, self.test_usr.email, self.test_usr_password), requests.Session) def test_shibb_api_login(self): with self.__setting(self.__default_shibb_config()): @@ -924,6 +924,40 @@ class TestSecurity(unittest.TestCase): logged_in = PyMISP(url, self.test_usr.authkey) check_response(logged_in.get_user()) + def test_username_in_response_header(self): + with MISPSetting(self.admin_misp_connector, "Security.username_in_response_header", True): + logged_in = login(url, self.test_usr.email, self.test_usr_password) + self.assertIsInstance(logged_in, requests.Session) + + response = logged_in.get(url + "/users/view/me.json") + self.assertIn("X-Username", response.headers) + self.assertEqual(self.test_usr.email, response.headers["X-Username"]) + + def test_username_in_response_header_api_access(self): + with MISPSetting(self.admin_misp_connector, "Security.username_in_response_header", True): + logged_in = PyMISP(url, self.test_usr.authkey) + + response = logged_in._prepare_request('GET', 'users/view/me') + self.assertIn("X-Username", response.headers) + self.assertEqual(self.test_usr.email + "/API/default", response.headers["X-Username"]) + + def test_username_in_response_header_advanced_api_access(self): + with MISPComplexSetting({ + "Security": { + "advanced_authkeys": True, + "username_in_response_header": True, + } + }): + auth_key = self.__create_advanced_authkey(self.test_usr.id) + + logged_in = PyMISP(url, auth_key["authkey_raw"]) + response = logged_in._prepare_request('GET', 'users/view/me') + + self.__delete_advanced_authkey(auth_key["id"]) + + self.assertIn("X-Username", response.headers) + self.assertEqual(f"{self.test_usr.email}/API/{auth_key['id']}", response.headers["X-Username"]) + def test_sg_index_user_cannot_see(self): org = self.__create_org() hidden_sg = self.__create_sharing_group()