Repository: conan-io/conan
Pull request #17132 (2024-10-08 14:08:17)

fix server auth based on username
Changelog: Bugfix: Drop the username permission validation bypass in ``conan_server``, it could be a potential security issue.
Docs: Omit

Close https://github.com/conan-io/conan/issues/11185


Patch:
diff --git a/conans/client/downloaders/file_downloader.py b/conans/client/downloaders/file_downloader.py
index 19ac00b40..c71f14553 100644
--- a/conans/client/downloaders/file_downloader.py
+++ b/conans/client/downloaders/file_downloader.py
@@ -84,7 +84,7 @@ class FileDownloader:
             if response.status_code == 404:
                 raise NotFoundException("Not found: %s" % url)
             elif response.status_code == 403:
-                if auth is None or (hasattr(auth, "token") and auth.token is None):
+                if auth is None or (hasattr(auth, "bearer") and auth.bearer is None):
                     # TODO: This is a bit weird, why this conversion? Need to investigate
                     raise AuthenticationException(response_to_str(response))
                 raise ForbiddenException(response_to_str(response))
diff --git a/conans/client/graph/install_graph.py b/conans/client/graph/install_graph.py
index d51f9082c..cc7db6587 100644
--- a/conans/client/graph/install_graph.py
+++ b/conans/client/graph/install_graph.py
@@ -1,5 +1,6 @@
 import json
 import os
+import shlex
 import textwrap
 
 from conan.api.output import ConanOutput
@@ -72,7 +73,8 @@ class _InstallPackageReference:
         compatible = "compatible:" if self.info and self.info.get("compatibility_delta") else ""
         cmd += f" --build={compatible}{self.ref}"
         if self.options:
-            cmd += " " + " ".join(f"-o {o}" for o in self.options)
+            scope = "" if self.context == "host" else ":b"
+            cmd += " " + " ".join(f'-o{scope}="{o}"' for o in self.options)
         if self.overrides:
             cmd += f' --lockfile-overrides="{self.overrides}"'
         return cmd
@@ -283,7 +285,8 @@ class _InstallConfiguration:
         compatible = "compatible:" if self.info and self.info.get("compatibility_delta") else ""
         cmd += f" --build={compatible}{self.ref}"
         if self.options:
-            cmd += " " + " ".join(f"-o {o}" for o in self.options)
+            scope = "" if self.context == "host" else ":b"
+            cmd += " " + " ".join(f'-o{scope}="{o}"' for o in self.options)
         if self.overrides:
             cmd += f' --lockfile-overrides="{self.overrides}"'
         return cmd
diff --git a/conans/client/rest/auth_manager.py b/conans/client/rest/auth_manager.py
index e67745ba8..525de701a 100644
--- a/conans/client/rest/auth_manager.py
+++ b/conans/client/rest/auth_manager.py
@@ -7,8 +7,7 @@ methods if receives AuthenticationException from RestApiClient.
 Flow:
     Directly invoke a REST method in RestApiClient, example: get_conan.
     if receives AuthenticationException (not open method) will ask user for login and password
-    and will invoke RestApiClient.get_token() (with LOGIN_RETRIES retries) and retry to call
-    get_conan with the new token.
+    (with LOGIN_RETRIES retries) and retry to call with the new token.
 """
 
 from conan.api.output import ConanOutput
@@ -60,18 +59,10 @@ class ConanApiAuthManager:
             raise ForbiddenException(f"Permission denied for user: '{user}': {e}")
         except AuthenticationException:
             # User valid but not enough permissions
-            if user is None or token is None:
-                # token is None when you change user with user command
-                # Anonymous is not enough, ask for a user
-                ConanOutput().info('Please log in to "%s" to perform this action. '
-                                   'Execute "conan remote login" command.' % remote.name)
-                if self._get_credentials_and_authenticate(rest_client, user, remote):
-                    return self.call_rest_api_method(remote, method_name, *args, **kwargs)
-            else:
-                # Token expired or not valid, so clean the token and repeat the call
-                # (will be anonymous call but exporting who is calling)
-                # logger.info("Token expired or not valid, cleaning the saved token and retrying")
-                self._clear_user_tokens_in_db(user, remote)
+            # token is None when you change user with user command
+            # Anonymous is not enough, ask for a user
+            ConanOutput().info(f"Remote '{remote.name}' needs authentication, obtaining credentials")
+            if self._get_credentials_and_authenticate(rest_client, user, remote):
                 return self.call_rest_api_method(remote, method_name, *args, **kwargs)
 
     def _get_credentials_and_authenticate(self, rest_client, user, remote):
@@ -95,14 +86,6 @@ class ConanApiAuthManager:
                 return True
         raise AuthenticationException("Too many failed login attempts, bye!")
 
-    def _clear_user_tokens_in_db(self, user, remote):
-        try:
-            self._creds.set(remote, user, token=None)
-        except Exception as e:
-            out = ConanOutput()
-            out.error('Your credentials could not be stored in local cache', error_type="exception")
-            out.debug(str(e) + '\n')
-
     def _authenticate(self, rest_client, remote, user, password):
         try:
             token = rest_client.authenticate(user, password)
diff --git a/conans/client/rest/file_uploader.py b/conans/client/rest/file_uploader.py
index 13dd9255f..429150581 100644
--- a/conans/client/rest/file_uploader.py
+++ b/conans/client/rest/file_uploader.py
@@ -26,7 +26,7 @@ class FileUploader(object):
             raise AuthenticationException(response_to_str(response))
 
         if response.status_code == 403:
-            if auth is None or auth.token is None:
+            if auth is None or auth.bearer is None:
                 raise AuthenticationException(response_to_str(response))
             raise ForbiddenException(response_to_str(response))
 
diff --git a/conans/server/service/authorize.py b/conans/server/service/authorize.py
index a6ce3df90..67150205f 100644
--- a/conans/server/service/authorize.py
+++ b/conans/server/service/authorize.py
@@ -127,9 +127,6 @@ class BasicAuthorizer(Authorizer):
         username: User that request to read the conans
         ref: RecipeReference
         """
-        if ref.user == username:
-            return
-
         self._check_any_rule_ok(username, self.read_permissions, ref)
 
     def check_write_conan(self, username, ref):
@@ -137,9 +134,6 @@ class BasicAuthorizer(Authorizer):
         username: User that request to write the conans
         ref: RecipeReference
         """
-        if ref.user == username:
-            return True
-
         self._check_any_rule_ok(username, self.write_permissions, ref)
 
     def check_delete_conan(self, username, ref):


Test patch:
diff --git a/test/integration/command_v2/test_info_build_order.py b/test/integration/command_v2/test_info_build_order.py
index 21f6b86a4..bb9c688c7 100644
--- a/test/integration/command_v2/test_info_build_order.py
+++ b/test/integration/command_v2/test_info_build_order.py
@@ -237,13 +237,13 @@ def test_info_build_order_options():
                   'context': 'build', 'depends': [], "overrides": {},
                   'binary': 'Build', 'options': ['tool/0.1:myopt=2'], 'filenames': [],
                   'info': {'options': {'myopt': '2'}},
-                  'build_args': '--tool-requires=tool/0.1 --build=tool/0.1 -o tool/0.1:myopt=2'},
+                  'build_args': '--tool-requires=tool/0.1 --build=tool/0.1 -o:b="tool/0.1:myopt=2"'},
                  {'package_id': 'a9035d84c5880b26c4b44acf70078c9a7dd37412', 'prev': None,
                   'context': 'build', 'depends': [], "overrides": {},
                   'info': {'options': {'myopt': '1'}},
                   'binary': 'Build', 'options': ['tool/0.1:myopt=1'],
                   'filenames': [],
-                  'build_args': '--tool-requires=tool/0.1 --build=tool/0.1 -o tool/0.1:myopt=1'}
+                  'build_args': '--tool-requires=tool/0.1 --build=tool/0.1 -o:b="tool/0.1:myopt=1"'}
              ]]}
         ],
         [
@@ -765,3 +765,22 @@ class TestBuildOrderReduce:
         tc.run("graph build-order-merge --file=order.json --file=order.json --format=json", assert_error=True)
         assert "dep/1.0:da39a3ee5e6b4b0d3255bfef95601890afd80709: Invalid configuration" in tc.out
         assert "IndexError: list index out of range" not in tc.out
+
+def test_build_order_space_in_options():
+    tc = TestClient(light=True)
+    tc.save({"dep/conanfile.py": GenConanfile("dep", "1.0")
+                .with_option("flags", ["ANY", None])
+                .with_option("extras", ["ANY", None]),
+             "conanfile.txt": textwrap.dedent("""
+             [requires]
+             dep/1.0
+
+             [options]
+             dep/*:flags=define=FOO define=BAR define=BAZ
+             dep/*:extras=cxx="yes" gnuext='no'
+             """)})
+
+    tc.run("create dep")
+    tc.run("graph build-order . --order-by=configuration --build=dep/1.0 -f=json", redirect_stdout="order.json")
+    order = json.loads(tc.load("order.json"))
+    assert order["order"][0][0]["build_args"] == '''--requires=dep/1.0 --build=dep/1.0 -o="dep/*:extras=cxx="yes" gnuext='no'" -o="dep/*:flags=define=FOO define=BAR define=BAZ"'''
diff --git a/test/integration/remote/auth_test.py b/test/integration/remote/auth_test.py
index 80aacdcb8..da585c94e 100644
--- a/test/integration/remote/auth_test.py
+++ b/test/integration/remote/auth_test.py
@@ -202,9 +202,9 @@ def test_token_expired():
        */*@*/*: admin
        """)
     save(os.path.join(server_folder, ".conan_server", "server.conf"), server_conf)
-    server = TestServer(base_path=server_folder, users={"admin": "password"})
+    server = TestServer(base_path=server_folder, users={"admin": "password", "other": "pass"})
 
-    c = TestClient(servers={"default": server}, inputs=["admin", "password"])
+    c = TestClient(servers={"default": server}, inputs=["admin", "password", "other", "pass"])
     c.save({"conanfile.py": GenConanfile()})
     c.run("create . --name=pkg --version=0.1 --user=user --channel=stable")
     c.run("upload * -r=default -c")
@@ -216,13 +216,12 @@ def test_token_expired():
     import time
     time.sleep(3)
     c.users = {}
-    conan_conf = "core:non_interactive=True"
-    c.save_home({"global.conf": conan_conf})
     c.run("remove * -c")
     c.run("install --requires=pkg/0.1@user/stable")
+    assert "Remote 'default' needs authentication, obtaining credentials" in c.out
     user, token, _ = localdb.get_login(server.fake_url)
-    assert user == "admin"
-    assert token is None
+    assert user == "other"
+    assert token is not None
 
 
 def test_auth_username_space():
diff --git a/test/integration/remote/rest_api_test.py b/test/integration/remote/rest_api_test.py
index cbb87dcd1..014af5b00 100644
--- a/test/integration/remote/rest_api_test.py
+++ b/test/integration/remote/rest_api_test.py
@@ -32,7 +32,11 @@ class RestApiTest(unittest.TestCase):
     def setUpClass(cls):
         if not cls.server:
             with environment_update({"CONAN_SERVER_PORT": str(get_free_port())}):
-                cls.server = TestServerLauncher(server_capabilities=['ImCool', 'TooCool'])
+                read_perms = [("*/*@*/*", "private_user")]
+                write_perms = [("*/*@*/*", "private_user")]
+                cls.server = TestServerLauncher(server_capabilities=['ImCool', 'TooCool'],
+                                                read_permissions=read_perms,
+                                                write_permissions=write_perms)
                 cls.server.start()
 
                 filename = os.path.join(temp_folder(), "conan.conf")
diff --git a/test/integration/remote/retry_test.py b/test/integration/remote/retry_test.py
index dd3e85981..5653f6562 100644
--- a/test/integration/remote/retry_test.py
+++ b/test/integration/remote/retry_test.py
@@ -73,7 +73,7 @@ class RetryDownloadTests(unittest.TestCase):
             uploader = FileUploader(requester=_RequesterMock(403, "content"),
                                     verify=False, config=_ConfigMock())
             with self.assertRaisesRegex(ForbiddenException, "content"):
-                auth = namedtuple("auth", "token")
+                auth = namedtuple("auth", "bearer")
                 uploader.upload(url="fake", abs_path=self.filename, retry=2, auth=auth("token"))
             output_lines = output.getvalue().splitlines()
             counter = Counter(output_lines)
@@ -86,7 +86,7 @@ class RetryDownloadTests(unittest.TestCase):
             uploader = FileUploader(requester=_RequesterMock(403, "content"),
                                     verify=False, config=_ConfigMock())
             with self.assertRaisesRegex(AuthenticationException, "content"):
-                auth = namedtuple("auth", "token")
+                auth = namedtuple("auth", "bearer")
                 uploader.upload(url="fake", abs_path=self.filename, retry=2, auth=auth(None))
             output_lines = output.getvalue().splitlines()
             counter = Counter(output_lines)
diff --git a/test/unittests/client/rest/uploader_test.py b/test/unittests/client/rest/uploader_test.py
index 4777ccd21..e8a6fbb71 100644
--- a/test/unittests/client/rest/uploader_test.py
+++ b/test/unittests/client/rest/uploader_test.py
@@ -37,7 +37,7 @@ class UploaderUnitTest(unittest.TestCase):
             uploader.upload("fake_url", self.f)
 
     def test_403_raises_unauthoirzed_exception_if_no_token(self):
-        auth = namedtuple("auth", "token")(None)
+        auth = namedtuple("auth", "bearer")(None)
         uploader = FileUploader(MockRequester(403), verify=False, config=_ConfigMock())
         with self.assertRaisesRegex(AuthenticationException, "tururu"):
             uploader.upload("fake_url", self.f, auth=auth)
@@ -48,7 +48,7 @@ class UploaderUnitTest(unittest.TestCase):
             uploader.upload("fake_url", self.f)
 
     def test_403_raises_forbidden_exception_if_token(self):
-        auth = namedtuple("auth", "token")("SOMETOKEN")
+        auth = namedtuple("auth", "bearer")("SOMETOKEN")
         uploader = FileUploader(MockRequester(403), verify=False, config=_ConfigMock())
         with self.assertRaisesRegex(ForbiddenException, "tururu"):
             uploader.upload("fake_url", self.f, auth=auth)
diff --git a/test/unittests/server/service/authorizer_test.py b/test/unittests/server/service/authorizer_test.py
index 1c80b8fe7..1424dad61 100644
--- a/test/unittests/server/service/authorizer_test.py
+++ b/test/unittests/server/service/authorizer_test.py
@@ -86,7 +86,7 @@ class AuthorizerTest(unittest.TestCase):
         # Only lasote can read it but other conans can be readed
         read_perms = [(str(self.openssl_ref), "lasote"), ("*/*@*/*", "*")]
         # Only pepe (and lasote because its owner) can write it and no more users can write
-        write_perms = [(str(self.openssl_ref), "pepe")]
+        write_perms = [(str(self.openssl_ref), "pepe, lasote")]
 
         authorizer = BasicAuthorizer(read_perms, write_perms)
 
@@ -185,12 +185,11 @@ class AuthorizerTest(unittest.TestCase):
         # Simple user list
         read_perms = [("openssl/*@lasote/testing", "user1,user2,user3")]
         authorizer = BasicAuthorizer(read_perms, [])
-        for u in ['user1','user2','user3']:
+        for u in ['user1', 'user2', 'user3']:
             authorizer.check_read_conan(u, self.openssl_ref)
 
         # Spaces bewteen user names should be ignored
         read_perms = [("openssl/*@lasote/testing", "user1 , user2,\tuser3")]
         authorizer = BasicAuthorizer(read_perms, [])
-        for u in ['user1','user2','user3']:
+        for u in ['user1', 'user2', 'user3']:
             authorizer.check_read_conan(u, self.openssl_ref)
-
diff --git a/test/unittests/server/service/service_test.py b/test/unittests/server/service/service_test.py
index 97c90f33c..8e3e1f7d4 100644
--- a/test/unittests/server/service/service_test.py
+++ b/test/unittests/server/service/service_test.py
@@ -39,7 +39,7 @@ class ConanServiceTest(unittest.TestCase):
         self.tmp_dir = temp_folder()
 
         read_perms = [("*/*@*/*", "*")]
-        write_perms = []
+        write_perms = [("*/*@*/*", "*")]
         authorizer = BasicAuthorizer(read_perms, write_perms)
 
         self.fake_url = "http://url"