diff --git a/src/python/WMCore/REST/Auth.py b/src/python/WMCore/REST/Auth.py index 3a6c5fe7b7..9d70df6596 100644 --- a/src/python/WMCore/REST/Auth.py +++ b/src/python/WMCore/REST/Auth.py @@ -41,11 +41,18 @@ def user_info_from_headers(key, verbose=False): # for HMAC validation while processing headers. prefix = suffix = "" hkeys = sorted(headers.keys()) + suffixDict = {} for hk in hkeys: + # hk here represents CMS HTTP header, e.g. cms-authz-user hk = hk.lower() if hk[0:9] in ("cms-authn", "cms-authz") and hk != "cms-authn-hmac": prefix += "h%xv%x" % (len(hk), len(headers[hk])) + # old implementation of suffix does not account for proper ordering suffix += "%s%s" % (hk, headers[hk]) + # keep all header key-vaules pair in dict and later sort values + suffixDict[hk] = headers[hk] + # hkname represents last element of header, e.g. + # if HTTP header is cms-authz-user then hkname would be user hkname = hk.split('-', 2)[-1] if hk.startswith("cms-authn"): val = headers[hk] @@ -53,11 +60,28 @@ def user_info_from_headers(key, verbose=False): val = str(val) if PY3 else str(val, "utf-8") user[hkname] = val if hk.startswith("cms-authz"): - user['roles'][hkname] = {'site': set(), 'group': set()} + user['roles'][hkname] = {} for r in headers[hk].split(): - site_or_group, name = r.split(':') - user['roles'][hkname][site_or_group].add(name) - + # r hear represent HTTP header value, e.g. group:admin + cricRoleKey, cricRoleValue = r.split(':') + # check if cric_role_key exists in user's roles, if not create a set + if cricRoleKey not in user['roles'][hkname].keys(): + user['roles'][hkname][cricRoleKey] = set() + # add new value to a key set + setValues = set(user['roles'][hkname][cricRoleKey]) + setValues.add(cricRoleValue) + user['roles'][hkname][cricRoleKey] = set(sorted(setValues)) + + # take into account that groups can come in different order since they + # are supplied as dictionary, e.g. + # {... {'user': {'group': {'users', 'admin'}}}, ...} + # we should always persists the order of groups to properly calculate new checksum + skeys = sorted([h for h in suffixDict.keys()]) + newSuffix = "" + for key in skeys: + vals = sorted(suffixDict[key].split(' ')) + newSuffix += "%s%s" % (key, ' '.join(vals)) + suffix = newSuffix # Check HMAC over authn/z headers with server key. If differs, reject. msg = prefix + "#" + suffix if PY3: @@ -180,7 +204,6 @@ def __init__(self): def _setup(self): """Hook this tool into cherrypy request.""" - log = cherrypy.log hooks = cherrypy.request.hooks conf = self._merged_args() diff --git a/src/python/WMCore/REST/Test.py b/src/python/WMCore/REST/Test.py index 30a6f30a87..fc1bcec10a 100644 --- a/src/python/WMCore/REST/Test.py +++ b/src/python/WMCore/REST/Test.py @@ -47,11 +47,25 @@ def fake_authz_headers(hmac_key, method = 'HNLogin', prefix = suffix = "" hkeys = list(headers) + suffixDict = {} for hk in sorted(hkeys): if hk != 'cms-auth-status': prefix += "h%xv%x" % (len(hk), len(headers[hk])) + # old implementation of suffix does not account for proper ordering suffix += "%s%s" % (hk, headers[hk]) - + # keep all header key-vaules pair in dict and later sort values + suffixDict[hk] = headers[hk] + + # take into account that groups can come in different order since they + # are supplied as dictionary, e.g. + # {... {'user': {'group': {'users', 'admin'}}}, ...} + # we should always persists the order of groups to properly calculate new checksum + skeys = sorted([h for h in suffixDict.keys()]) + newSuffix = "" + for key in skeys: + vals = sorted(suffixDict[key].split(' ')) + newSuffix += "%s%s" % (key, ' '.join(vals)) + suffix = newSuffix msg = prefix + "#" + suffix if PY3: hmac_key = encodeUnicodeToBytes(hmac_key) diff --git a/test/python/WMCore_t/MicroService_t/MSCore_t/MSAuth_t.py b/test/python/WMCore_t/MicroService_t/MSCore_t/MSAuth_t.py index bc18793713..cdda54957d 100644 --- a/test/python/WMCore_t/MicroService_t/MSCore_t/MSAuth_t.py +++ b/test/python/WMCore_t/MicroService_t/MSCore_t/MSAuth_t.py @@ -29,9 +29,9 @@ def setUp(self): # REST/Auth.py module calculates checksum based on provided hmac # for instnace for self.hmac we will expect self.chkSum # with all CMS headers we setup below - # d357b299194ec4bed4e4fc73fc9ceab10139c16f vs. 4311773080ea12f4ea31a096da49de105083bb9e - self.hmac = 'd357b299194ec4bed4e4fc73fc9ceab10139c16f' - self.chkSum = '4311773080ea12f4ea31a096da49de105083bb9e' + # ERROR: authz hmac mismatch, 169d02b96265caf05894b526f99a22549dcd38ed vs. d357b299194ec4bed4e4fc73fc9ceab10139c16f + self.hmac = '169d02b96265caf05894b526f99a22549dcd38ed' + self.chkSum = 'd357b299194ec4bed4e4fc73fc9ceab10139c16f' self.fname = '/tmp/ms-authz.json' # for testing purposes we need to setup cherrypy headers diff --git a/test/python/WMCore_t/REST_t/RestAuth_t.py b/test/python/WMCore_t/REST_t/RestAuth_t.py new file mode 100644 index 0000000000..34f408610e --- /dev/null +++ b/test/python/WMCore_t/REST_t/RestAuth_t.py @@ -0,0 +1,83 @@ +""" +Unit tests for REST/Auth.py module +Author: Valentin Kuznetsov +""" + +# system modules +import logging +import unittest + +# third party modules +import cherrypy + +# WMCore modules +from WMCore.REST.Auth import user_info_from_headers + + +class RESTAuthTest(unittest.TestCase): + "Unit test for RESTAuth module" + + def setUp(self): + """ + setup necessary data for unit test usage + """ + self.logger = logging.getLogger('rest_auth') + + # REST/Auth.py module calculates checksum based on provided hmac + # for instnace for self.hmac we will expect self.chkSum + # with all CMS headers + + # let's create set of different conditions + self.user_groups = [] + self.hmacs = [] + self.chkSums = [] + + # for group:users group:admin + user_groups = 'group:users group:admin' + hmac = '169d02b96265caf05894b526f99a22549dcd38ed' + chkSum = 'd357b299194ec4bed4e4fc73fc9ceab10139c16f' + self.user_groups.append(user_groups) + self.hmacs.append(hmac) + self.chkSums.append(chkSum) + + # let's reverse order of user groups + user_groups = 'group:admin group:users' + self.user_groups.append(user_groups) + self.hmacs.append(hmac) + self.chkSums.append(chkSum) + + # for group:admin group:users iam_group:test site:T1_XX_YYYY + user_groups = 'group:admin group:users iam_group:test site:T1_XX_YYYY' + hmac = '57ea0f58134aa079972da30a8fc2bf81853c949b' + chkSum = 'd357b299194ec4bed4e4fc73fc9ceab1013' + self.user_groups.append(user_groups) + self.hmacs.append(hmac) + self.chkSums.append(chkSum) + + def testRESTAuth(self): + "test RESTAuth methods" + for idx in range(len(self.user_groups)): + user_groups = self.user_groups[idx] + hmac = self.hmacs[idx] + chkSum = self.chkSums[idx] + + # for testing purposes we need to setup cherrypy headers + # cms-auth-status, cms-authn, cms-authz, cms-authn-hmac, cms-authn-name, cms-authn-dn + cherrypy.request.headers = { + 'cms-auth-status': 'OK', + 'cms-authn': 'fake', + 'cms-authz-user': user_groups, + 'cms-authn-hmac': hmac, + 'cms-authn-name': 'test', + 'cms-authn-dn': 'dn'} + + authzKey = bytes(chkSum, 'UTF-8') + user = user_info_from_headers(key=authzKey) + self.logger.info(f"user_groups {user_groups}") + self.logger.info(f"hmac {hmac}") + self.logger.info(f"chkSum {chkSum}") + self.logger.info(f"user {user}") + + +if __name__ == '__main__': + unittest.main()