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 ++++++++++++++++++------ 2 files changed, 34 insertions(+), 12 deletions(-) (limited to 'test/functional_auth') 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? -- cgit