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 <ppai@redhat.com>
Reviewed-on: http://review.gluster.org/6336
Reviewed-by: Luis Pabon <lpabon@redhat.com>
Tested-by: Luis Pabon <lpabon@redhat.com>
Signed-off-by: Prashanth Pai <ppai@redhat.com>
Reviewed-on: http://review.gluster.org/6447
Reviewed-by: Thiago Da Silva <thiago@redhat.com>
Tested-by: Thiago Da Silva <thiago@redhat.com>

3 files changed, 48 insertions, 26 deletions

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 e63eeb2..e128b54 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 privileges' 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 privileges' 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 privileges' 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 privileges' 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 7bf44fe..f01c34f 100644
--- a/test/unit/common/middleware/gswauth/swauth/test_middleware.py
+++ b/test/unit/common/middleware/gswauth/swauth/test_middleware.py
@@ -1287,7 +1287,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'},
@@ -1296,25 +1296,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([
@@ -1424,7 +1424,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([
@@ -1598,7 +1598,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([
@@ -1813,7 +1813,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([
@@ -2046,7 +2046,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([
@@ -2391,7 +2391,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([
@@ -2990,7 +2990,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([
@@ -3260,7 +3260,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([
@@ -3282,7 +3282,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([
@@ -3303,7 +3303,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):