fix(skore-hub-project/login): Ignore token URI when using API key

Author: thomass-dev

skore-hub-project/src/skore_hub_project/authentication/uri.py

@@ -24,10 +24,36 @@ def persist(uri: str) -> None:
 
 
 def URI() -> str:
-    """URI used for ``skore hub`` authentication."""
+    """
+    URI used for ``skore hub`` authentication.
+
+    Notes
+    -----
+    It is discovered by following the rules in order:
+
+    1. If you have setup an API key using the envar ``SKORE_HUB_API_KEY``, returns the
+       value of the envar ``SKORE_HUB_URI``, or ``https://api.skore.probabl.ai``.
+
+    2. If you have an active token, extract the content of the envar ``SKORE_HUB_URI``
+       and the URI associated with the token:
+        2.1. If both the URIs from the environment and the one associated with the token
+             are empty, returns ``https://api.skore.probabl.ai``.
+        2.2. If the URI from the environment is empty and the one associated with the
+             token isn't, returns the URI associated wit the token.
+        2.2. If the URI associated with the token is empty, and the one from the
+             environment isn't, returns the URI from the environment.
+        2.2. If both the URIs associated with the token and the one from the
+             environment are not empty:
+                2.2.1. If both are equal, returns one of them.
+                2.2.2. If both aren't equal, raises a conflicting exception.
+    """
+    uri_from_environment = environ.get(ENVARNAME)
+
+    if environ.get("SKORE_HUB_API_KEY"):
+        return uri_from_environment or DEFAULT
+
     filepath = Filepath()
     uri_from_persistence = filepath.read_text() if filepath.exists() else None
-    uri_from_environment = environ.get(ENVARNAME)
 
     if (
         uri_from_persistence

skore-hub-project/tests/unit/client/test_client.py

@@ -11,7 +11,6 @@
 from pytest import mark, raises
 
 from skore_hub_project.authentication import token, uri
-from skore_hub_project.authentication.uri import DEFAULT as URI
 from skore_hub_project.client.client import Client, HUBClient
 
 DATETIME_MIN = datetime.min.replace(tzinfo=timezone.utc).isoformat()
@@ -123,7 +122,7 @@ def sleep(timeout):
 class TestHUBClient:
     def test_request_with_api_key(self, monkeypatch, respx_mock):
         monkeypatch.setenv("SKORE_HUB_API_KEY", "<api-key>")
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(200))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
 
         with HUBClient() as client:
             client.get("foo")
@@ -132,7 +131,7 @@ def test_request_with_api_key(self, monkeypatch, respx_mock):
         assert respx_mock.calls.last.request.headers["X-API-Key"] == "<api-key>"
 
     def test_request_with_token(self, respx_mock):
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(200))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
 
         assert not token.Filepath().exists()
 
@@ -150,12 +149,12 @@ def test_request_with_token(self, respx_mock):
         assert respx_mock.calls.last.request.headers["authorization"] == "Bearer A"
 
     def test_request_with_token_and_uri(self, respx_mock):
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(200))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
 
         assert not token.Filepath().exists()
 
         token.persist("A", "B", DATETIME_MAX)
-        uri.persist(URI)
+        uri.persist(uri.DEFAULT)
 
         assert token.Filepath().exists()
         assert token.access(refresh=False) == "A"
@@ -165,10 +164,48 @@ def test_request_with_token_and_uri(self, respx_mock):
 
         assert token.Filepath().exists()
         assert token.access(refresh=False) == "A"
-        assert uri.URI() == URI
+        assert uri.URI() == uri.DEFAULT
         assert "X-API-Key" not in respx_mock.calls.last.request.headers
         assert respx_mock.calls.last.request.headers["authorization"] == "Bearer A"
 
+    def test_request_with_api_key_and_token_and_uri(self, monkeypatch, respx_mock):
+        TOKEN_URI = "https://token.com"
+        API_KEY_URI = "https://apikey.com"
+
+        assert not uri.Filepath().exists()
+
+        # simulate user with token
+        uri.persist(TOKEN_URI)
+
+        assert uri.URI() == TOKEN_URI
+
+        # simulate user with token and API key
+        monkeypatch.setenv("SKORE_HUB_API_KEY", "<api-key>")
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
+
+        assert uri.URI() == uri.DEFAULT
+
+        with HUBClient() as client:
+            client.get("foo")
+
+        assert respx_mock.calls.last.request.url == urljoin(uri.DEFAULT, "foo")
+        assert "authorization" not in respx_mock.calls.last.request.headers
+        assert respx_mock.calls.last.request.headers["X-API-Key"] == "<api-key>"
+
+        # simulate user with token, API key and URI in environment
+        monkeypatch.setenv("SKORE_HUB_API_KEY", "<api-key>")
+        monkeypatch.setenv("SKORE_HUB_URI", API_KEY_URI)
+        respx_mock.get(urljoin(API_KEY_URI, "foo")).mock(Response(200))
+
+        assert uri.URI() == API_KEY_URI
+
+        with HUBClient() as client:
+            client.get("foo")
+
+        assert respx_mock.calls.last.request.url == urljoin(API_KEY_URI, "foo")
+        assert "authorization" not in respx_mock.calls.last.request.headers
+        assert respx_mock.calls.last.request.headers["X-API-Key"] == "<api-key>"
+
     @mark.respx(assert_all_mocked=False)
     def test_request_with_invalid_token_raises(self, respx_mock):
         with raises(token.TokenError, match="not logged in"), HUBClient() as client:
@@ -179,7 +216,7 @@ def test_request_with_invalid_token_raises(self, respx_mock):
     @mark.respx(assert_all_mocked=False)
     def test_request_with_conflicting_uri_raises(self, respx_mock, monkeypatch):
         token.persist("A", "B", DATETIME_MAX)
-        uri.persist(URI)
+        uri.persist(uri.DEFAULT)
         monkeypatch.setenv(uri.ENVARNAME, "https://my-conflicting-uri")
 
         with (
@@ -193,7 +230,7 @@ def test_request_with_conflicting_uri_raises(self, respx_mock, monkeypatch):
         assert not respx_mock.calls
 
     def test_request_with_expired_token(self, tmp_path, respx_mock):
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(200))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
         respx_mock.post(REFRESH_URL).mock(
             Response(
                 200,
@@ -221,7 +258,7 @@ def test_request_with_expired_token(self, tmp_path, respx_mock):
         assert respx_mock.calls.last.request.headers["authorization"] == "Bearer D"
 
     def test_request_raises(self, tmp_path, respx_mock):
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(404))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(404))
 
         assert not token.Filepath().exists()
 
@@ -244,7 +281,7 @@ def test_request_without_package_version(self, respx_mock):
         assert version("skore-hub-project") == "0.0.0+unknown"
         assert PACKAGE_VERSION is None
 
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(200))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
 
         with HUBClient(authenticated=False) as client:
             client.get("foo")
@@ -253,7 +290,7 @@ def test_request_without_package_version(self, respx_mock):
 
     def test_request_with_package_version(self, monkeypatch, respx_mock):
         monkeypatch.setattr("skore_hub_project.client.client.PACKAGE_VERSION", "1.0.0")
-        respx_mock.get(urljoin(URI, "foo")).mock(Response(200))
+        respx_mock.get(urljoin(uri.DEFAULT, "foo")).mock(Response(200))
 
         with HUBClient(authenticated=False) as client:
             client.get("foo")