From c3c46d6188015cd5f75e7a6f754fd032ab30ac21 Mon Sep 17 00:00:00 2001 From: Prashanth Pai Date: Thu, 2 Jan 2014 12:20:20 +0530 Subject: Fix users not able to change their own password/key Users were not able to update their own password/key with the update operation resulting in 403 (HTTPForbidden). EXAMPLES: Command to update password/key of regular user: gswauth-add-user -U account1:user1 -K old_pass account1 user1 new_pass Command to update password/key of account admin: gswauth-add-user -U account1:admin -K old_pass -a account1 admin new_pass Command to update password/key of reseller_admin: gswauth-add-user -U account1:radmin -K old_pass -r account1 radmin new_pass BUG: https://bugs.launchpad.net/gluster-swift/+bug/1262227 Change-Id: I604da5aee67099b29541eb7e51a040a041f1961b Signed-off-by: Prashanth Pai Reviewed-on: http://review.gluster.org/6650 Reviewed-by: Luis Pabon Tested-by: Luis Pabon Reviewed-on: http://review.gluster.org/6668 Reviewed-by: Chetan Risbud Tested-by: Chetan Risbud --- test/functional_auth/gswauth/test_gswauth.py | 7 +- test/functional_auth/gswauth/test_gswauth_cli.py | 72 ++++++++- .../middleware/gswauth/swauth/test_middleware.py | 171 +++++++++++++++++++-- 3 files changed, 237 insertions(+), 13 deletions(-) (limited to 'test') diff --git a/test/functional_auth/gswauth/test_gswauth.py b/test/functional_auth/gswauth/test_gswauth.py index 3ee3f5d..5219f13 100644 --- a/test/functional_auth/gswauth/test_gswauth.py +++ b/test/functional_auth/gswauth/test_gswauth.py @@ -227,15 +227,16 @@ class TestGSWauth(unittest.TestCase): # attempt to change password path = '%sv2/%s/%s' % (config['auth_prefix'], config['account'], config['username']) - headers = self._get_admin_headers() + headers = {'X-Auth-Admin-User': + config['account'] + ':' + config['username'], + 'X-Auth-Admin-Key': config['password']} headers.update({'X-Auth-User-Key': 'newpassword', 'Content-Length': '0', - 'X-Auth-Admin-Key': config['password'], 'X-Auth-User-Admin': 'true'}) conn = http_connect(config['auth_host'], config['auth_port'], 'PUT', path, headers) resp = conn.getresponse() - self.assertTrue(resp.status == 401) + self.assertTrue(resp.status == 201) finally: try: diff --git a/test/functional_auth/gswauth/test_gswauth_cli.py b/test/functional_auth/gswauth/test_gswauth_cli.py index e128b54..8cac619 100644 --- a/test/functional_auth/gswauth/test_gswauth_cli.py +++ b/test/functional_auth/gswauth/test_gswauth_cli.py @@ -278,4 +278,74 @@ class TestUser(unittest.TestCase): #TODO:more testcases? - + def testChangeKey(self): + # Create account and users + (status, output) = Utils.addAccount('test') + self.assertEqual(status, 0, 'Account creation failed: ' + output) + + (status, output) = Utils.addAdminUser('test', 'admin', 'password') + self.assertEqual(status, 0, 'User addition failed: ' + output) + + (status, output) = Utils.addUser('test', 'user', 'password') + self.assertEqual(status, 0, 'User addition failed: ' + output) + + (status, output) = Utils.addResellerAdminUser('test', 'radmin', 'password') + self.assertEqual(status, 0, 'User addition failed: ' + output) + + # Change acccount admin password/key + (status, output) = Utils.addAdminUser('test', 'admin', 'new_password', user='test:admin', key='password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Change regular user password/key + (status, output) = Utils.addUser('test', 'user', 'new_password', user='test:user', key='password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Change reseller admin password/key + (status, output) = Utils.addResellerAdminUser('test', 'radmin', 'new_password', user='test:radmin', key='password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # To verify that password was changed for real, re-run the above commands, but with the new password + # Change acccount admin password/key using the new password + (status, output) = Utils.addAdminUser('test', 'admin', 'password', user='test:admin', key='new_password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Change regular user password/key using the new password + (status, output) = Utils.addUser('test', 'user', 'password', user='test:user', key='new_password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Change reseller admin password/key using the new password + (status, output) = Utils.addResellerAdminUser('test', 'radmin', 'password', user='test:radmin', key='new_password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Make sure that regular user cannot upgrade to admin + (status, output) = Utils.addAdminUser('test', 'user', 'password', user='test:user', key='password') + self.assertEqual('User creation failed' in output, True, 'Update key failed: ' + output) + + # Make sure that regular user cannot upgrade to reseller_admin + (status, output) = Utils.addResellerAdminUser('test', 'user', 'password', user='test:user', key='password') + self.assertEqual('User creation failed' in output, True, 'Update key failed: ' + output) + + # Make sure admin cannot update himself to reseller_admin + (status, output) = Utils.addResellerAdminUser('test', 'admin', 'password', user='test:admin', key='password') + self.assertEqual('User creation failed' in output, True, 'Update key failed: ' + output) + + # Account admin changing regular user password/key + (status, output) = Utils.addUser('test', 'user', 'new_password', user='test:admin', key='password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + # Verify by running the command with new password + (status, output) = Utils.addUser('test', 'user', 'password', user='test:user', key='new_password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Reseller admin changing regular user password/key + (status, output) = Utils.addUser('test', 'user', 'new_password', user='test:radmin', key='password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + # Verify by running the command with new password + (status, output) = Utils.addUser('test', 'user', 'password', user='test:user', key='new_password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + + # Reseller admin changing account admin password/key + (status, output) = Utils.addAdminUser('test', 'admin', 'new_password', user='test:radmin', key='password') + self.assertEqual(status, 0, 'Update key failed: ' + output) + # Verify by running the command with new password + (status, output) = Utils.addAdminUser('test', 'admin', 'password', user='test:admin', key='new_password') + self.assertEqual(status, 0, 'Update key failed: ' + output) diff --git a/test/unit/common/middleware/gswauth/swauth/test_middleware.py b/test/unit/common/middleware/gswauth/swauth/test_middleware.py index f01c34f..bce734d 100644 --- a/test/unit/common/middleware/gswauth/swauth/test_middleware.py +++ b/test/unit/common/middleware/gswauth/swauth/test_middleware.py @@ -3242,6 +3242,10 @@ class TestAuth(unittest.TestCase): def test_put_user_reseller_admin_fail_bad_creds(self): self.test_auth.app = FakeApp(iter([ + # Checking if user is changing his own key. This is called. + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:rdm"}, + {"name": "test"}, {"name": ".admin"}, + {"name": ".reseller_admin"}], "auth": "plaintext:key"})), # GET of user object (reseller admin) # This shouldn't actually get called, checked # below @@ -3261,9 +3265,13 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Reseller-Admin': 'true'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 401) - self.assertEquals(self.test_auth.app.calls, 0) + self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ + # Checking if user is changing his own key. This is called. + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:adm"}, + {"name": "test"}, {"name": ".admin"}], + "auth": "plaintext:key"})), # GET of user object (account admin, but not reseller admin) # This shouldn't actually get called, checked # below @@ -3283,13 +3291,16 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Reseller-Admin': 'true'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 401) - self.assertEquals(self.test_auth.app.calls, 0) + self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ + # Checking if user is changing his own key. This is called. + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"})), # GET of user object (regular user) # This shouldn't actually get called, checked # below - ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, {"name": "test"}], "auth": "plaintext:key"}))])) resp = Request.blank('/auth/v2/act/usr', environ={ @@ -3304,13 +3315,17 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Reseller-Admin': 'true'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 401) - self.assertEquals(self.test_auth.app.calls, 0) + self.assertEquals(self.test_auth.app.calls, 1) def test_put_user_account_admin_fail_bad_creds(self): self.test_auth.app = FakeApp(iter([ # GET of user object (account admin, but wrong # account) ('200 Ok', {}, json.dumps({"groups": [{"name": "act2:adm"}, + {"name": "test"}, {"name": ".admin"}], + "auth": "plaintext:key"})), + # Checking if user is changing his own key. + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:adm"}, {"name": "test"}, {"name": ".admin"}], "auth": "plaintext:key"}))])) resp = Request.blank('/auth/v2/act/usr', @@ -3326,10 +3341,13 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Admin': 'true'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 403) - self.assertEquals(self.test_auth.app.calls, 1) + self.assertEquals(self.test_auth.app.calls, 2) self.test_auth.app = FakeApp(iter([ # GET of user object (regular user) + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"})), + # Checking if user is changing his own key. ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, {"name": "test"}], "auth": "plaintext:key"}))])) resp = Request.blank('/auth/v2/act/usr', @@ -3345,13 +3363,17 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Admin': 'true'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 403) - self.assertEquals(self.test_auth.app.calls, 1) + self.assertEquals(self.test_auth.app.calls, 2) def test_put_user_regular_fail_bad_creds(self): self.test_auth.app = FakeApp(iter([ # GET of user object (account admin, but wrong # account) ('200 Ok', {}, json.dumps({"groups": [{"name": "act2:adm"}, + {"name": "test"}, {"name": ".admin"}], + "auth": "plaintext:key"})), + # Checking if user is changing his own key. + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:adm"}, {"name": "test"}, {"name": ".admin"}], "auth": "plaintext:key"}))])) resp = Request.blank('/auth/v2/act/usr', @@ -3365,13 +3387,16 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Key': 'key'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 403) - self.assertEquals(self.test_auth.app.calls, 1) + self.assertEquals(self.test_auth.app.calls, 2) self.test_auth.app = FakeApp(iter([ # GET of user object (regular user) + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"})), + # Checking if user is changing his own key. ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, {"name": "test"}], "auth": "plaintext:key"}))])) - resp = Request.blank('/auth/v2/act/usr', + resp = Request.blank('/auth/v2/act2/usr', environ={ 'REQUEST_METHOD': 'PUT'}, headers={ @@ -3382,7 +3407,7 @@ class TestAuth(unittest.TestCase): 'X-Auth-User-Key': 'key'} ).get_response(self.test_auth) self.assertEquals(resp.status_int, 403) - self.assertEquals(self.test_auth.app.calls, 1) + self.assertEquals(self.test_auth.app.calls, 2) def test_put_user_regular_success(self): self.test_auth.app = FakeApp(iter([ @@ -3941,6 +3966,134 @@ class TestAuth(unittest.TestCase): self.assert_(not self.test_auth.credentials_match( {'auth': 'plaintext:key'}, 'notkey')) + def test_is_user_changing_own_key_err(self): + # User does not exist + self.test_auth.app = FakeApp( + iter([('404 Not Found', {}, '')])) + req = Request.blank('/auth/v2/act/usr', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:usr', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'key'}) + self.assert_( + not self.test_auth.is_user_changing_own_key(req, 'act:usr')) + self.assertEquals(self.test_auth.app.calls, 1) + + # user attempting to escalate himself as admin + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/usr', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:usr', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'key', + 'X-Auth-User-Admin': 'true'}) + self.assert_( + not self.test_auth.is_user_changing_own_key(req, 'act:usr')) + self.assertEquals(self.test_auth.app.calls, 1) + + # admin attempting to escalate himself as reseller_admin + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:adm"}, + {"name": "test"}, {"name": ".admin"}], + "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/adm', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:adm', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'key', + 'X-Auth-User-Reseller-Admin': 'true'}) + self.assert_( + not self.test_auth.is_user_changing_own_key(req, 'act:adm')) + self.assertEquals(self.test_auth.app.calls, 1) + + # different user + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/usr2', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:usr', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'key'}) + self.assert_( + not self.test_auth.is_user_changing_own_key(req, 'act:usr2')) + self.assertEquals(self.test_auth.app.calls, 1) + + # wrong key + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/usr', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:usr', + 'X-Auth-Admin-Key': 'wrongkey', + 'X-Auth-User-Key': 'newkey'}) + self.assert_( + not self.test_auth.is_user_changing_own_key(req, 'act:usr')) + self.assertEquals(self.test_auth.app.calls, 1) + + def test_is_user_changing_own_key_sucess(self): + # regular user + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:usr"}, + {"name": "test"}], "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/usr', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:usr', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'newkey'}) + self.assert_( + self.test_auth.is_user_changing_own_key(req, 'act:usr')) + self.assertEquals(self.test_auth.app.calls, 1) + + # account admin + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:adm"}, + {"name": "test"}, {"name": ".admin"}], + "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/adm', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:adm', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'newkey', + 'X-Auth-User-Admin': 'true'}) + self.assert_( + self.test_auth.is_user_changing_own_key(req, 'act:adm')) + self.assertEquals(self.test_auth.app.calls, 1) + + # reseller admin + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, json.dumps({"groups": [{"name": "act:adm"}, + {"name": "test"}, {"name": ".admin"}, + {"name": ".reseller_admin"}], "auth": "plaintext:key"}))])) + req = Request.blank('/auth/v2/act/adm', + environ={ + 'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Auth-Admin-User': 'act:adm', + 'X-Auth-Admin-Key': 'key', + 'X-Auth-User-Key': 'newkey', + 'X-Auth-User-Reseller-Admin': 'true'}) + self.assert_( + self.test_auth.is_user_changing_own_key(req, 'act:adm')) + self.assertEquals(self.test_auth.app.calls, 1) + def test_is_super_admin_success(self): self.assert_( self.test_auth.is_super_admin( -- cgit