From 0dd563cbfc20c1278d362eae7a4cc65b7cf94006 Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Feb 25 2020 09:44:07 +0000
Subject: Do not fallback to 'groups' in allowed_clients if user is found in 'users'.


---

diff --git a/server/odcs/server/api_utils.py b/server/odcs/server/api_utils.py
index abed329..6d93f36 100644
--- a/server/odcs/server/api_utils.py
+++ b/server/odcs/server/api_utils.py
@@ -46,29 +46,27 @@ def _set_default_client_allowed_attrs(ret_attrs, attrs):
 
 def _load_allowed_clients_attr(attrs):
     """
-    Loads attributes from the
-    conf.allowed_clients[key][user_name/group_name] dict. If the requested
-    attribute is not found in the loaded dict, the conf.allowed_$attr_name
-    is used as a default.
+    Loads attributes from the conf.allowed_clients dict based on the logged
+    in user and its groups. If the requested attribute is not found in
+    the loaded dict, the conf.allowed_$attr_name is used as a default.
 
     :param list attrs: List of attribute names to load from the dict.
     :return: Generator of Dicts with loaded attributes.
     """
-    for key in ["users", "groups"]:
-        clients = conf.allowed_clients.get(key, {})
-        if key == "users":
-            # Check if the user is defined in clients, if not, return None.
-            if flask.g.user.username in clients:
-                ret_attrs = dict(copy.deepcopy(clients[flask.g.user.username]))
+    # Check if logged user exists in "users" definition of allowed_clients.
+    clients = conf.allowed_clients.get("users", {})
+    if flask.g.user.username in clients:
+        ret_attrs = dict(copy.deepcopy(clients[flask.g.user.username]))
+        ret_attrs = _set_default_client_allowed_attrs(ret_attrs, attrs)
+        yield ret_attrs
+    else:
+        # Check if group is defined in allowed_clients "groups".
+        clients = conf.allowed_clients.get("groups", {})
+        for group in flask.g.groups:
+            if group in clients:
+                ret_attrs = dict(copy.deepcopy(clients[group]))
                 ret_attrs = _set_default_client_allowed_attrs(ret_attrs, attrs)
                 yield ret_attrs
-        else:
-            # Check if the group is defined in clients, if not, return None
-            for group in flask.g.groups:
-                if group in clients:
-                    ret_attrs = dict(copy.deepcopy(clients[group]))
-                    ret_attrs = _set_default_client_allowed_attrs(ret_attrs, attrs)
-                    yield ret_attrs
 
 
 def _enum_int_to_str_list(enum_dict, val):
@@ -116,7 +114,7 @@ def raise_if_input_not_allowed(**kwargs):
                     % (flask.g.user.username, name, values))
                 continue
 
-            # Conver integers from db format to string list.
+            # Convert integers from db format to string list.
             if name == "source_types":
                 values = INVERSE_PUNGI_SOURCE_TYPE_NAMES[values]
             elif name == "flags":
diff --git a/server/tests/test_views.py b/server/tests/test_views.py
index 573957f..0441bd0 100644
--- a/server/tests/test_views.py
+++ b/server/tests/test_views.py
@@ -117,6 +117,9 @@ class ViewBaseTest(ModelsBaseTest):
                 'dev2': {
                     'source_types': ['module', 'raw_config'],
                     'compose_types': ["test", "nightly"]
+                },
+                'dev3': {
+                    'source_types': ['tag']
                 }
             }
         }
@@ -1045,6 +1048,19 @@ class TestViews(ViewBaseTest):
         six.assertRegex(self, data['message'],
                         r"The delete request for compose \(id=%s\) has been accepted and will be processed by backend later." % c3.id)
 
+    def test_can_create_compose_with_permission_overriden_by_username(self):
+        with self.test_request_context(user='dev3', groups=['dev2']):
+            flask.g.oidc_scopes = [
+                '{0}{1}'.format(conf.oidc_base_namespace, 'new-compose')
+            ]
+
+            resp = self.client.post('/api/1/composes/', data=json.dumps(
+                {'source': {'type': 'module', 'source': 'testmodule:rawhide'}}))
+        db.session.expire_all()
+
+        self.assertEqual(resp.status, '403 FORBIDDEN')
+        self.assertEqual(resp.status_code, 403)
+
     @patch.object(odcs.server.config.Config, 'max_seconds_to_live', new_callable=PropertyMock)
     @patch.object(odcs.server.config.Config, 'seconds_to_live', new_callable=PropertyMock)
     def test_use_seconds_to_live_in_request(self, mock_seconds_to_live, mock_max_seconds_to_live):