Repository: rucio/rucio
Issue: Update permissions for identity API

Motivation
----------
Some of the permission checks are not done correctly and need to be updated in all permission files.


Changed files:
lib/rucio/core/permission/atlas.py, lib/rucio/core/permission/generic.py, lib/rucio/web/rest/flaskapi/v1/account.py, lib/rucio/web/rest/webpy/v1/account.py, lib/rucio/api/identity.py, lib/rucio/core/permission/cms.py, lib/rucio/tests/test_account.py

diff --git a/lib/rucio/api/identity.py b/lib/rucio/api/identity.py
index bed06f91b..b5c05b3f8 100644
--- a/lib/rucio/api/identity.py
+++ b/lib/rucio/api/identity.py
@@ -9,6 +9,7 @@
 # - Mario Lassnig, <mario.lassnig@cern.ch>, 2012, 2017
 # - Tomas Kouba, <tomas.kouba@cern.ch>, 2014
 # - Thomas Beermann, <thomas.beermann@cern.ch>, 2014
+# - Hannes Hansen, <hannes.jakob.hansen@cern.ch>, 2019
 #
 # PY3K COMPATIBLE
 
@@ -35,14 +36,19 @@ def add_identity(identity_key, id_type, email, password=None):
     return identity.add_identity(identity_key, IdentityType.from_sym(id_type), email, password=password)
 
 
-def del_identity(identity_key, id_type):
+def del_identity(identity_key, id_type, issuer):
     """
     Deletes a user identity.
-
     :param identity_key: The identity key name. For example x509 DN, or a username.
     :param id_type: The type of the authentication (x509, gss, userpass, ssh).
+    :param issuer: The issuer account.
     """
-    return identity.del_identity(identity_key, IdentityType.from_sym(id_type))
+    id_type = IdentityType.from_sym(id_type)
+    kwargs = {'accounts': identity.list_accounts_for_identity(identity_key, id_type)}
+    if not permission.has_permission(issuer=issuer, action='del_identity', kwargs=kwargs):
+        raise exception.AccessDenied('Account %s can not delete identity' % (issuer))
+
+    return identity.del_identity(identity_key, id_type)
 
 
 def add_account_identity(identity_key, id_type, account, email, issuer, default=False):
@@ -63,14 +69,19 @@ def add_account_identity(identity_key, id_type, account, email, issuer, default=
     return identity.add_account_identity(identity=identity_key, type=IdentityType.from_sym(id_type), default=default, email=email, account=account)
 
 
-def del_account_identity(identity_key, id_type, account):
+def del_account_identity(identity_key, id_type, account, issuer):
     """
     Removes a membership association between identity and account.
 
     :param identity_key: The identity key name. For example x509 DN, or a username.
     :param id_type: The type of the authentication (x509, gss, userpass, ssh).
     :param account: The account name.
+    :param issuer: The issuer account.
     """
+    kwargs = {'account': account}
+    if not permission.has_permission(issuer=issuer, action='del_account_identity', kwargs=kwargs):
+        raise exception.AccessDenied('Account %s can not delete account identity' % (issuer))
+
     return identity.del_account_identity(identity_key, IdentityType.from_sym(id_type), account)
 
 
diff --git a/lib/rucio/core/permission/atlas.py b/lib/rucio/core/permission/atlas.py
index 23b5925f3..da811e768 100644
--- a/lib/rucio/core/permission/atlas.py
+++ b/lib/rucio/core/permission/atlas.py
@@ -17,7 +17,7 @@
 # - Martin Barisits <martin.barisits@cern.ch>, 2016-2018
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2016-2018
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
-# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018-2019
 #
 # PY3K COMPATIBLE
 
@@ -106,7 +106,9 @@ def has_permission(issuer, action, kwargs):
             'update_lifetime_exceptions': perm_update_lifetime_exceptions,
             'get_ssh_challenge_token': perm_get_ssh_challenge_token,
             'get_signed_url': perm_get_signed_url,
-            'add_bad_pfns': perm_add_bad_pfns}
+            'add_bad_pfns': perm_add_bad_pfns,
+            'del_account_identity': perm_del_account_identity,
+            'del_identity': perm_del_identity}
 
     return perm.get(action, perm_default)(issuer=issuer, kwargs=kwargs)
 
@@ -324,6 +326,30 @@ def perm_add_account_identity(issuer, kwargs):
     return issuer == 'root' or issuer == kwargs.get('account')
 
 
+def perm_del_account_identity(issuer, kwargs):
+    """
+    Checks if an account can delete an identity to an account.
+
+    :param issuer: Account identifier which issues the command.
+    :param kwargs: List of arguments for the action.
+    :returns: True if account is allowed, otherwise False
+    """
+
+    return issuer == 'root' or issuer == kwargs.get('account')
+
+
+def perm_del_identity(issuer, kwargs):
+    """
+    Checks if an account can delete an identity.
+
+    :param issuer: Account identifier which issues the command.
+    :param kwargs: List of arguments for the action.
+    :returns: True if account is allowed, otherwise False
+    """
+
+    return issuer == 'root' or issuer in kwargs.get('accounts')
+
+
 def perm_add_did(issuer, kwargs):
     """
     Checks if an account can add an data identifier to a scope.
diff --git a/lib/rucio/core/permission/cms.py b/lib/rucio/core/permission/cms.py
index 4affa5d52..76a0e0f44 100644
--- a/lib/rucio/core/permission/cms.py
+++ b/lib/rucio/core/permission/cms.py
@@ -11,7 +11,7 @@
 # - Mario Lassnig, <mario.lassnig@cern.ch>, 2017-2018
 # - Martin Barisits, <martin.barisits@cern.ch>, 2017
 # - Eric Vaandering, <ewv@fnal.gov>, 2018
-# - Hannes Hansen, <hannes.jakob.hansen@cern.ch>, 2018
+# - Hannes Hansen, <hannes.jakob.hansen@cern.ch>, 2018-2019
 #
 # PY3K COMPATIBLE
 
@@ -98,7 +98,9 @@ def has_permission(issuer, action, kwargs):
             'update_lifetime_exceptions': perm_update_lifetime_exceptions,
             'get_ssh_challenge_token': perm_get_ssh_challenge_token,
             'get_signed_url': perm_get_signed_url,
-            'add_bad_pfns': perm_add_bad_pfns}
+            'add_bad_pfns': perm_add_bad_pfns,
+            'del_account_identity': perm_del_account_identity,
+            'del_identity': perm_del_identity}
 
     return perm.get(action, perm_default)(issuer=issuer, kwargs=kwargs)
 
@@ -299,6 +301,30 @@ def perm_add_account_identity(issuer, kwargs):
     return issuer == 'root' or issuer == kwargs.get('account')
 
 
+def perm_del_account_identity(issuer, kwargs):
+    """
+    Checks if an account can delete an identity to an account.
+
+    :param issuer: Account identifier which issues the command.
+    :param kwargs: List of arguments for the action.
+    :returns: True if account is allowed, otherwise False
+    """
+
+    return issuer == 'root' or issuer == kwargs.get('account')
+
+
+def perm_del_identity(issuer, kwargs):
+    """
+    Checks if an account can delete an identity.
+
+    :param issuer: Account identifier which issues the command.
+    :param kwargs: List of arguments for the action.
+    :returns: True if account is allowed, otherwise False
+    """
+
+    return issuer == 'root' or issuer in kwargs.get('accounts')
+
+
 def perm_add_did(issuer, kwargs):
     """
     Checks if an account can add an data identifier to a scope.
diff --git a/lib/rucio/core/permission/generic.py b/lib/rucio/core/permission/generic.py
index 63d34de50..8dbf53fab 100644
--- a/lib/rucio/core/permission/generic.py
+++ b/lib/rucio/core/permission/generic.py
@@ -17,7 +17,7 @@
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2016-2018
 # - Martin Barisits <martin.barisits@cern.ch>, 2017
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
-# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018-2019
 #
 # PY3K COMPATIBLE
 
@@ -103,7 +103,9 @@ def has_permission(issuer, action, kwargs):
             'update_lifetime_exceptions': perm_update_lifetime_exceptions,
             'get_ssh_challenge_token': perm_get_ssh_challenge_token,
             'get_signed_url': perm_get_signed_url,
-            'add_bad_pfns': perm_add_bad_pfns}
+            'add_bad_pfns': perm_add_bad_pfns,
+            'del_account_identity': perm_del_account_identity,
+            'del_identity': perm_del_identity}
 
     return perm.get(action, perm_default)(issuer=issuer, kwargs=kwargs)
 
@@ -301,6 +303,30 @@ def perm_add_account_identity(issuer, kwargs):
     return issuer == 'root' or issuer == kwargs.get('account')
 
 
+def perm_del_account_identity(issuer, kwargs):
+    """
+    Checks if an account can delete an identity to an account.
+
+    :param issuer: Account identifier which issues the command.
+    :param kwargs: List of arguments for the action.
+    :returns: True if account is allowed, otherwise False
+    """
+
+    return issuer == 'root' or issuer == kwargs.get('account')
+
+
+def perm_del_identity(issuer, kwargs):
+    """
+    Checks if an account can delete an identity.
+
+    :param issuer: Account identifier which issues the command.
+    :param kwargs: List of arguments for the action.
+    :returns: True if account is allowed, otherwise False
+    """
+
+    return issuer == 'root' or issuer in kwargs.get('accounts')
+
+
 def perm_add_did(issuer, kwargs):
     """
     Checks if an account can add an data identifier to a scope.
diff --git a/lib/rucio/tests/test_account.py b/lib/rucio/tests/test_account.py
index 218c82e0e..2b38575be 100644
--- a/lib/rucio/tests/test_account.py
+++ b/lib/rucio/tests/test_account.py
@@ -26,7 +26,8 @@ from rucio.client.accountclient import AccountClient
 from rucio.common.config import config_get
 from rucio.common.exception import AccountNotFound, Duplicate, InvalidObject
 from rucio.common.utils import generate_uuid as uuid
-from rucio.db.sqla.constants import AccountStatus
+from rucio.core.identity import add_account_identity, add_identity
+from rucio.db.sqla.constants import AccountStatus, IdentityType
 from rucio.tests.common import account_name_generator
 from rucio.web.rest.account import APP as account_app
 from rucio.web.rest.authentication import APP as auth_app
@@ -282,6 +283,32 @@ class TestAccountRestApi():
         assert_equal(body['email'], 'test')
         assert_equal(res4.status, 200)
 
+    def test_delete_identity_of_account(self):
+        """ ACCOUNT (REST): send a DELETE to remove an identity of an account."""
+        mw = []
+        account = account_name_generator()
+        identity = uuid()
+        password = 'secret'
+        add_account(account, 'USER', 'rucio@email.com', 'root')
+        add_identity(identity, IdentityType.USERPASS, 'email@email.com', password)
+        add_account_identity(identity, IdentityType.USERPASS, account, 'email@email.com')
+        headers1 = {'X-Rucio-Account': account, 'X-Rucio-Username': identity, 'X-Rucio-Password': password}
+        res1 = TestApp(auth_app.wsgifunc(*mw)).get('/userpass', headers=headers1, expect_errors=True)
+        token = str(res1.header('X-Rucio-Auth-Token'))
+
+        # normal deletion
+        headers2 = {'X-Rucio-Auth-Token': str(token)}
+        data = dumps({'authtype': 'USERPASS', 'identity': identity})
+        res2 = TestApp(account_app.wsgifunc(*mw)).delete('/' + account + '/identities', headers=headers2, params=data, expect_errors=True)
+        assert_equal(res2.status, 200)
+
+        # unauthorized deletion
+        other_account = account_name_generator()
+        headers2 = {'X-Rucio-Auth-Token': str(token)}
+        data = dumps({'authtype': 'USERPASS', 'identity': identity})
+        res2 = TestApp(account_app.wsgifunc(*mw)).delete('/' + other_account + '/identities', headers=headers2, params=data, expect_errors=True)
+        assert_equal(res2.status, 401)
+
 
 class TestAccountClient():
 
diff --git a/lib/rucio/web/rest/flaskapi/v1/account.py b/lib/rucio/web/rest/flaskapi/v1/account.py
index e252ca337..614206b66 100755
--- a/lib/rucio/web/rest/flaskapi/v1/account.py
+++ b/lib/rucio/web/rest/flaskapi/v1/account.py
@@ -21,7 +21,7 @@
 # - Cheng-Hsi Chao <cheng-hsi.chao@cern.ch>, 2014
 # - Joaquin Bogado <joaquin.bogado@cern.ch>, 2015
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
-# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018-2019
 #
 # PY3K COMPATIBLE
 
@@ -504,7 +504,7 @@ class Identities(MethodView):
         except TypeError:
             return generate_http_error_flask(400, 'TypeError', 'body must be a json dictionary')
         try:
-            del_account_identity(identity, authtype, account)
+            del_account_identity(identity, authtype, account, request.environ.get('issuer'))
         except AccessDenied as error:
             return generate_http_error_flask(401, 'AccessDenied', error.args[0])
         except AccountNotFound as error:
diff --git a/lib/rucio/web/rest/webpy/v1/account.py b/lib/rucio/web/rest/webpy/v1/account.py
index 10dd13e74..3266eea13 100755
--- a/lib/rucio/web/rest/webpy/v1/account.py
+++ b/lib/rucio/web/rest/webpy/v1/account.py
@@ -21,7 +21,7 @@
 # - Cheng-Hsi Chao <cheng-hsi.chao@cern.ch>, 2014
 # - Joaquin Bogado <joaquin.bogado@cern.ch>, 2015
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
-# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018-2019
 #
 # PY3K COMPATIBLE
 
@@ -544,7 +544,7 @@ class Identities(RucioController):
         except TypeError:
             raise generate_http_error(400, 'TypeError', 'body must be a json dictionary')
         try:
-            del_account_identity(identity, authtype, account)
+            del_account_identity(identity, authtype, account, ctx.env.get('issuer'))
         except AccessDenied as error:
             raise generate_http_error(401, 'AccessDenied', error.args[0])
         except AccountNotFound as error: