From fc9124caf45949dfcc0732536c6825c12d74582a Mon Sep 17 00:00:00 2001 From: Prashanth Pai Date: Fri, 22 Nov 2013 12:13:09 +0530 Subject: gswauth: Fix 403 being returned instead of 401 - 401(Unauthorized) is to be returned when user credentials are wrong where as 403(Forbidden) is to be returned when user credentials are correct but the user doesn't have the priveleges to carry out the operation. - Also error messages displayed when using swauth-* command line utilities have been updated. Change-Id: I485786896ad14d3263f4325d1857cacc93adab96 Signed-off-by: Prashanth Pai Reviewed-on: http://review.gluster.org/6336 Reviewed-by: Luis Pabon Tested-by: Luis Pabon --- test/functional_auth/gswauth/test_gswauth.py | 4 +-- test/functional_auth/gswauth/test_gswauth_cli.py | 42 ++++++++++++++++------ .../middleware/gswauth/swauth/test_middleware.py | 28 +++++++-------- 3 files changed, 48 insertions(+), 26 deletions(-) (limited to 'test') diff --git a/test/functional_auth/gswauth/test_gswauth.py b/test/functional_auth/gswauth/test_gswauth.py index 30ecfeb..3ee3f5d 100644 --- a/test/functional_auth/gswauth/test_gswauth.py +++ b/test/functional_auth/gswauth/test_gswauth.py @@ -159,7 +159,7 @@ class TestGSWauth(unittest.TestCase): conn = http_connect(config['auth_host'], config['auth_port'], 'PUT', path, headers) resp = conn.getresponse() - self.assertTrue(resp.status == 403) + self.assertTrue(resp.status == 401) def test_change_user_password(self): # check and register account @@ -235,7 +235,7 @@ class TestGSWauth(unittest.TestCase): conn = http_connect(config['auth_host'], config['auth_port'], 'PUT', path, headers) resp = conn.getresponse() - self.assertTrue(resp.status == 403) + self.assertTrue(resp.status == 401) finally: try: diff --git a/test/functional_auth/gswauth/test_gswauth_cli.py b/test/functional_auth/gswauth/test_gswauth_cli.py index f6c08df..f81f35e 100644 --- a/test/functional_auth/gswauth/test_gswauth_cli.py +++ b/test/functional_auth/gswauth/test_gswauth_cli.py @@ -81,8 +81,8 @@ class TestSwauthPrep(unittest.TestCase): (status,output)=Utils.swauthPrep(key='notavalidkey') self.assertNotEqual(status, 0, 'Invalid swauth-prep request accepted(wrong key provided):'+output) - #TODO:In place of this error message 'Auth subsystem prep failed: 403 Forbidden, Invalid user/key' would be good to have - self.assertEqual('Auth subsystem prep failed: 403 Forbidden' in output,True, 'Invalid swauth-prep request accepted: '+output) + self.assertEqual('gswauth preparation failed: 401 Unauthorized: Invalid user/key provided' in output,True, 'Invalid\ + swauth-prep request accepted: '+output) #TODO:More cases for invalid url and admin user @@ -128,7 +128,11 @@ class TestAccount(unittest.TestCase): (status,output)=Utils.addAccount('testinvalidkey',key='invalidkey') #self.assertEqual(status, 0, 'account creation failed std err was: '+output) #assert for better error message 403 Forbidden, Invalid user/key would be good to have - self.assertEqual('403 Forbidden' in output,True, 'Invalid account creation request accepted: '+output) + self.assertEqual('Account creation failed: 401 Unauthorized: Invalid user/key provided' in output,True, 'Invalid account creation request accepted: '+output) + + (status,output) = Utils.addUser('test','tester','testing') + (status,output)=Utils.addAccount('test2',user='test:tester',key='testing') + self.assertEqual('Account creation failed: 403 Forbidden: Insufficient priveleges' in output,True, 'Invalid account creation request accepted: '+output) #TODO:more cases? def testDeleteAccount(self): @@ -141,8 +145,8 @@ class TestAccount(unittest.TestCase): #Invalid request to delete an account with users (status,output)=Utils.deleteAccount('test2') self.assertNotEqual(status, 0, 'account deletion failed for test2 account'+output) - #TODO:decide on expected behavior 'there are active users,users needs to be deleted first'? - self.assertEqual('Conflict' in output,True, 'account deletion failed for test account'+output) + self.assertEqual('Delete account failed: 409 Conflict: Account test2 contains active users. Delete all users first.' in output,True, + 'account deletion failed for test account'+output) #delete all users in above account and then try again (status,output) = Utils.deleteUser('test2','tester') @@ -154,12 +158,20 @@ class TestAccount(unittest.TestCase): (status,output) = Utils.deleteUser('test2','tester3') self.assertEqual(status, 0, 'setTestDeleteAccountEnv'+output) + (status,output) = Utils.addUser('test','tester','testing') + (status,output) = Utils.deleteAccount('test2',user='test:tester',key='testing') + self.assertEqual('Delete account failed: 403 Forbidden: Insufficient priveleges' in output,True, 'account deletion failed for test2 account'+output) + + (status,output) = Utils.deleteAccount('test2',key='invalidkey') + self.assertEqual('Delete account failed: 401 Unauthorized: Invalid user/key provided' in output,True, 'account deletion failed for test2 account'+output) + (status,output)=Utils.deleteAccount('test2') self.assertEqual(status, 0, 'account deletion failed for test2 account'+output) (status,output)=Utils.deleteAccount('accountdoesnotexist') - #TODO:decide on expected behavior self.assertNotEqual(status, 0, 'account deletion failed for accountdoesnotexist'+output) + self.assertEqual('Delete account failed: 404 Not Found: Account accountdoesnotexist does not exist' in output,True, 'account deletion failed for test\ + account'+output) #TODO:more cases def testListAcounts(self): @@ -225,12 +237,24 @@ class TestUser(unittest.TestCase): (status,output) = Utils.addAdminUser('accountdoesnotexist', 'testcli', 'testcli') #TODO: decide on behavior,below is just place holder, right now it accepts this request and create both user and account self.assertEqual(status, 0, 'Invalid user creation request accepted,accountdoesnotexist: '+output) + + (status,output) = Utils.addUser('test','testuser2','testuser2',user='test:testuser',key='testuser') + self.assertEqual('User creation failed: 403 Forbidden: Insufficient priveleges' in output, True, 'user addition failed'+output) + + (status,output) = Utils.addUser('test','testuser2','testuser2',user='test:testadminuser',key='invalidkey') + self.assertEqual('User creation failed: 401 Unauthorized: Invalid user/key provided' in output, True, 'user addition failed'+output) #TODO: more test cases? def testDeleteUser(self): #set the env for test self.setTestDeleteUserEnv() + (status,output) = Utils.deleteUser('test','testadminuser',user='test:testuser',key='testuser') + self.assertEqual('Delete user failed: 403 Forbidden: Insufficient priveleges' in output, True, 'user deletion failed'+output) + + (status,output) = Utils.deleteUser('test','testuser',key='invalidkey') + self.assertEqual('Delete user failed: 401 Unauthorized: Invalid user/key provided' in output, True, 'user deletion failed'+output) + (status,output) = Utils.deleteUser('test','testadminuser') self.assertEqual(status, 0, 'valid user deletion failed:'+output) @@ -247,12 +271,10 @@ class TestUser(unittest.TestCase): self.assertEqual('Usage:' in output, True, 'Invalid user deletion request accepted : '+output) (status,output) = Utils.deleteUser('test', 'userdoesnotexist') - self.assertNotEqual(status, 0, 'Invalid user deletion request accepted,userdoesnotexist:'+output) - #TODO:decide on expected behavior,current is '404 Not Found' + self.assertEqual('Delete user failed: 404 Not Found: User userdoesnotexist does not exist' in output, True, 'user deletion failed'+output) (status,output) = Utils.deleteUser('accountisnothere', 'testcli') - self.assertNotEqual(status, 0, 'Invalid user deletion request accepted, accountdoesnotexist:'+output) - #TODO:decide on expected behavior,current is '404 Not Found' + self.assertEqual('Delete user failed: 404 Not Found: User testcli does not exist' in output, True, 'user deletion failed'+output) #TODO:more testcases? diff --git a/test/unit/common/middleware/gswauth/swauth/test_middleware.py b/test/unit/common/middleware/gswauth/swauth/test_middleware.py index 46d634f..00e6453 100644 --- a/test/unit/common/middleware/gswauth/swauth/test_middleware.py +++ b/test/unit/common/middleware/gswauth/swauth/test_middleware.py @@ -1283,7 +1283,7 @@ class TestAuth(unittest.TestCase): 'super_admin', 'X-Auth-Admin-Key': 'supertest'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) resp = Request.blank('/auth/v2/.prep', environ={ 'REQUEST_METHOD': 'POST'}, @@ -1292,25 +1292,25 @@ class TestAuth(unittest.TestCase): '.super_admin', 'X-Auth-Admin-Key': 'upertest'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) resp = Request.blank('/auth/v2/.prep', environ={ 'REQUEST_METHOD': 'POST'}, headers={ 'X-Auth-Admin-User': '.super_admin'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) resp = Request.blank('/auth/v2/.prep', environ={ 'REQUEST_METHOD': 'POST'}, headers={ 'X-Auth-Admin-Key': 'supertest'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) resp = Request.blank( '/auth/v2/.prep', environ={'REQUEST_METHOD': 'POST'}).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) def test_prep_fail_account_create(self): self.test_auth.app = FakeApp(iter([ @@ -1420,7 +1420,7 @@ class TestAuth(unittest.TestCase): 'super:admin', 'X-Auth-Admin-Key': 'supertest'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ @@ -1594,7 +1594,7 @@ class TestAuth(unittest.TestCase): 'super:admin', 'X-Auth-Admin-Key': 'supertest'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ @@ -1809,7 +1809,7 @@ class TestAuth(unittest.TestCase): body=json.dumps( {'storage': {'local': 'new_value'}}) ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ @@ -2042,7 +2042,7 @@ class TestAuth(unittest.TestCase): headers={'X-Auth-Admin-User': 'super:admin', 'X-Auth-Admin-Key': 'supertest'},).get_response( self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ @@ -2387,7 +2387,7 @@ class TestAuth(unittest.TestCase): 'super:admin', 'X-Auth-Admin-Key': 'supertest'}, ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ @@ -2986,7 +2986,7 @@ class TestAuth(unittest.TestCase): 'super:admin', 'X-Auth-Admin-Key': 'supertest'}, ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 1) self.test_auth.app = FakeApp(iter([ @@ -3256,7 +3256,7 @@ class TestAuth(unittest.TestCase): 'key', 'X-Auth-User-Reseller-Admin': 'true'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 0) self.test_auth.app = FakeApp(iter([ @@ -3278,7 +3278,7 @@ class TestAuth(unittest.TestCase): 'key', 'X-Auth-User-Reseller-Admin': 'true'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 0) self.test_auth.app = FakeApp(iter([ @@ -3299,7 +3299,7 @@ class TestAuth(unittest.TestCase): 'key', 'X-Auth-User-Reseller-Admin': 'true'} ).get_response(self.test_auth) - self.assertEquals(resp.status_int, 403) + self.assertEquals(resp.status_int, 401) self.assertEquals(self.test_auth.app.calls, 0) def test_put_user_account_admin_fail_bad_creds(self): -- cgit