Repository: rucio/rucio
Issue: Bad exception when failing to set global account limit

Motivation
----------
Running as an unprivileged user, it is expected that this fails but the exception should not be an internal server error:

```
>>> c.set_global_account_limit('dcameron', 'type=DATADISK', 1000000000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dcameron/dev/ddm/rucio/lib/rucio/client/accountlimitclient.py", line 107, in set_global_account_limit
    raise exc_cls(exc_msg)
rucio.common.exception.RucioException: An unknown exception occurred.
Details: no error information passed (http status code: 500 ('internal_server_error', 'server_error', '/o\\\\', '\\xe2\\x9c\\x97'))
```


Modification
------------

The exception should be the same as I see when setting local account limits:

```
>>> c.set_local_account_limit('dcameron', 'NDGF-T1_DATADISK', 1000000000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dcameron/dev/ddm/rucio/lib/rucio/client/accountlimitclient.py", line 63, in set_local_account_limit
    raise exc_cls(exc_msg)
rucio.common.exception.AccessDenied: Access to the requested resource denied.
Details: Account dcameron can not set account limits.
```


Changed files:
lib/rucio/core/permission/cms.py, lib/rucio/client/accountlimitclient.py, lib/rucio/core/permission/atlas.py, lib/rucio/core/permission/generic.py

diff --git a/lib/rucio/client/accountlimitclient.py b/lib/rucio/client/accountlimitclient.py
index 4a188e09f..417b0a8eb 100644
--- a/lib/rucio/client/accountlimitclient.py
+++ b/lib/rucio/client/accountlimitclient.py
@@ -13,7 +13,7 @@
 # limitations under the License.
 #
 # Authors:
-# - Martin Barisits <martin.barisits@cern.ch>, 2014-2018
+# - Martin Barisits <martin.barisits@cern.ch>, 2014-2020
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2014
 # - Vincent Garonne <vgaronne@gmail.com>, 2014-2018
 # - Ralph Vigne <ralph.vigne@cern.ch>, 2015
@@ -21,6 +21,11 @@
 #
 # PY3K COMPATIBLE
 
+try:
+    from urllib import quote_plus
+except ImportError:
+    from urllib.parse import quote_plus
+
 from json import dumps
 from requests.status_codes import codes
 
@@ -132,7 +137,7 @@ class AccountLimitClient(BaseClient):
         """
 
         data = dumps({'bytes': bytes})
-        path = '/'.join([self.ACCOUNTLIMIT_BASEURL, 'global', account, rse_expression])
+        path = '/'.join([self.ACCOUNTLIMIT_BASEURL, 'global', account, quote_plus(rse_expression)])
         url = build_url(choice(self.list_hosts), path=path)
 
         r = self._send_request(url, type='POST', data=data)
@@ -154,7 +159,7 @@ class AccountLimitClient(BaseClient):
         :raises AccountNotFound: if account doesn't exist.
         """
 
-        path = '/'.join([self.ACCOUNTLIMIT_BASEURL, 'global', account, rse_expression])
+        path = '/'.join([self.ACCOUNTLIMIT_BASEURL, 'global', account, quote_plus(rse_expression)])
         url = build_url(choice(self.list_hosts), path=path)
 
         r = self._send_request(url, type='DEL')
diff --git a/lib/rucio/core/permission/atlas.py b/lib/rucio/core/permission/atlas.py
index 11875b09b..9d1fcd3d0 100644
--- a/lib/rucio/core/permission/atlas.py
+++ b/lib/rucio/core/permission/atlas.py
@@ -14,7 +14,7 @@
 #
 # Authors:
 # - Vincent Garonne <vgaronne@gmail.com>, 2016
-# - Martin Barisits <martin.barisits@cern.ch>, 2016-2019
+# - Martin Barisits <martin.barisits@cern.ch>, 2016-2020
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2016-2019
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
 # - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018-2019
@@ -953,7 +953,7 @@ def perm_set_global_account_limit(issuer, kwargs):
     for kv in list_account_attributes(account=issuer):
         if kv['key'].startswith('country-') and kv['value'] == 'admin':
             admin_in_country.add(kv['key'].partition('-')[2])
-    resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_exp'])}
+    resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_expression'])}
     if resolved_rse_countries.issubset(admin_in_country):
         return True
     return False
@@ -974,7 +974,7 @@ def perm_delete_global_account_limit(issuer, kwargs):
     for kv in list_account_attributes(account=issuer):
         if kv['key'].startswith('country-') and kv['value'] == 'admin':
             admin_in_country.add(kv['key'].partition('-')[2])
-    resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_exp'])}
+    resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_expression'])}
     if resolved_rse_countries.issubset(admin_in_country):
         return True
     return False
diff --git a/lib/rucio/core/permission/cms.py b/lib/rucio/core/permission/cms.py
index a919b7eba..28345ead1 100644
--- a/lib/rucio/core/permission/cms.py
+++ b/lib/rucio/core/permission/cms.py
@@ -9,7 +9,7 @@
 # - Vincent Garonne, <vincent.garonne@cern.ch>, 2016
 # - Cedric Serfon, <cedric.serfon@cern.ch>, 2016-2018
 # - Mario Lassnig, <mario.lassnig@cern.ch>, 2017-2018
-# - Martin Barisits, <martin.barisits@cern.ch>, 2017-2019
+# - Martin Barisits, <martin.barisits@cern.ch>, 2017-2020
 # - Eric Vaandering, <ewv@fnal.gov>, 2018-2020
 # - Hannes Hansen, <hannes.jakob.hansen@cern.ch>, 2018-2019
 # - Ruturaj Gujar, <ruturaj.gujar23@gmail.com>, 2019
@@ -807,7 +807,7 @@ def perm_set_global_account_limit(issuer, kwargs):
         if kv['key'].startswith('country-') and kv['value'] == 'admin':
             admin_in_country.add(kv['key'].partition('-')[2])
     resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country')
-                              for rse in parse_expression(kwargs['rse_exp'])}
+                              for rse in parse_expression(kwargs['rse_expression'])}
     if resolved_rse_countries.issubset(admin_in_country):
         return True
     return False
@@ -830,7 +830,7 @@ def perm_delete_global_account_limit(issuer, kwargs):
             admin_in_country.add(kv['key'].partition('-')[2])
     if admin_in_country:
         resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country')
-                                  for rse in parse_expression(kwargs['rse_exp'])}
+                                  for rse in parse_expression(kwargs['rse_expression'])}
         if resolved_rse_countries.issubset(admin_in_country):
             return True
     return False
diff --git a/lib/rucio/core/permission/generic.py b/lib/rucio/core/permission/generic.py
index 7baaf8c29..d437d6c25 100644
--- a/lib/rucio/core/permission/generic.py
+++ b/lib/rucio/core/permission/generic.py
@@ -15,7 +15,7 @@
 # Authors:
 # - Vincent Garonne <vgaronne@gmail.com>, 2016
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2016-2018
-# - Martin Barisits <martin.barisits@cern.ch>, 2017
+# - Martin Barisits <martin.barisits@cern.ch>, 2017-2020
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
 # - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018-2019
 # - Andrew Lister, <andrew.lister@stfc.ac.uk>, 2019
@@ -772,7 +772,7 @@ def perm_set_global_account_limit(issuer, kwargs):
     for kv in list_account_attributes(account=issuer):
         if kv['key'].startswith('country-') and kv['value'] == 'admin':
             admin_in_country.add(kv['key'].partition('-')[2])
-    resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_exp'])}
+    resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_expression'])}
     if resolved_rse_countries.issubset(admin_in_country):
         return True
     return False
@@ -814,7 +814,7 @@ def perm_delete_global_account_limit(issuer, kwargs):
         if kv['key'].startswith('country-') and kv['value'] == 'admin':
             admin_in_country.add(kv['key'].partition('-')[2])
     if admin_in_country:
-        resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_exp'])}
+        resolved_rse_countries = {list_rse_attributes(rse_id=rse['rse_id']).get('country') for rse in parse_expression(kwargs['rse_expression'])}
         if resolved_rse_countries.issubset(admin_in_country):
             return True
     return False