Merge pull request #1731 from candleindark/no-hardcoded-api-key-name

Obtain API key for DANDI instance from corresponding env var

Author: yarikoptic

DEVELOPMENT.md

@@ -55,8 +55,17 @@ development command line options.
   otherwise be hidden from the user-visible (`--help`) interface, unless this
   env variable is set to a non-empty value
 
-- `DANDI_API_KEY` -- avoids using keyrings, thus making it possible to
-  "temporarily" use another account etc for the "API" version of the server.
+- `{CAPITALIZED_INSTANCE_NAME_WITH_UNDERSCORE}_API_KEY` --
+  Provides the API key to access a known DANDI instance.
+  Respective keys for multiple instances can be provided. The name of the environment
+  variable providing the key for a specific known DANDI instance corresponds to the name
+  of the instance. For example, the environment variable `DANDI_API_KEY` provides the key
+  for the known instance named `dandi` and the environment variable
+  `EMBER_SANDBOX_API_KEY` provides the key for the known instance named `ember-sandbox`.
+  I.e., the environment variable name is the capitalized version of the instance's name
+  with "-" replaced by "_" suffixed by "_API_KEY". Providing API keys through environment
+  variables avoids using keyrings, thus making it possible to "temporarily" use another
+  account etc for the "API" version of the server.
 
 - `DANDI_LOG_LEVEL` -- set log level. By default `INFO`, should be an int (`10` - `DEBUG`).
 

dandi/cli/tests/test_service_scripts.py

@@ -31,7 +31,7 @@ def test_reextract_metadata(
     asset_id = nwb_dandiset.dandiset.get_asset_by_path(
         "sub-mouse001/sub-mouse001.nwb"
     ).identifier
-    monkeypatch.setenv("DANDI_API_KEY", nwb_dandiset.api.api_key)
+    nwb_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     r = CliRunner().invoke(
         service_scripts,
         ["reextract-metadata", "--when=always", nwb_dandiset.dandiset.version_api_url],
@@ -74,7 +74,7 @@ def test_update_dandiset_from_doi(
 ) -> None:
     dandiset_id = new_dandiset.dandiset_id
     repository = new_dandiset.api.instance.gui
-    monkeypatch.setenv("DANDI_API_KEY", new_dandiset.api.api_key)
+    new_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     if os.environ.get("DANDI_TESTS_NO_VCR", "") or sys.version_info <= (3, 10):
         # Older vcrpy has an issue with Python 3.9 and newer urllib2 >= 2
         # But we require newer urllib2 for more correct operation, and

dandi/dandiapi.py

@@ -488,20 +488,23 @@ def authenticate(self, token: str, save_to_keyring: bool = False) -> None:
     def dandi_authenticate(self) -> None:
         """
         Acquire and set the authentication token/API key used by the
-        `DandiAPIClient`.  If the :envvar:`DANDI_API_KEY` environment variable
-        is set, its value is used as the token.  Otherwise, the token is looked
-        up in the user's keyring under the service
-        ":samp:`dandi-api-{INSTANCE_NAME}`" [#auth]_ and username "``key``".
-        If no token is found there, the user is prompted for the token, and, if
-        it proves to be valid, it is stored in the user's keyring.
+        `DandiAPIClient`.
+        If the :envvar:`{INSTANCE_NAME}_API_KEY` environment variable is set, its value
+        is used as the token. Here, ``{INSTANCE_NAME}`` is the uppercased instance name
+        with hyphens replaced by underscores. Otherwise, the token is looked up in the
+        user's keyring under the service ":samp:`dandi-api-{self.dandi_instance.name}`"
+        [#auth]_ and username "``key``". If no token is found there, the user is
+        prompted for the token, and, if it proves to be valid, it is stored in the
+        user's keyring.
 
         .. [#auth] E.g., "``dandi-api-dandi``" for the production server or
                    "``dandi-api-dandi-sandbox``" for the sandbox server
         """
         # Shortcut for advanced folks
-        api_key = os.environ.get("DANDI_API_KEY", None)
+        env_var_name = self.api_key_env_var
+        api_key = os.environ.get(env_var_name, None)
         if api_key:
-            lgr.debug("Using api key from DANDI_API_KEY environment variable")
+            lgr.debug(f"Using `{env_var_name}` environment variable as the API key")
             self.authenticate(api_key)
             return
         client_name, app_id = self._get_keyring_ids()
@@ -733,6 +736,14 @@ def get_asset(self, asset_id: str) -> BaseRemoteAsset:
         metadata = info.pop("metadata", None)
         return BaseRemoteAsset.from_base_data(self, info, metadata)
 
+    @property
+    def api_key_env_var(self) -> str:
+        """
+        Get the name of the environment variable that can be used to specify the
+        API key for the associated DANDI instance.
+        """
+        return f"{self.dandi_instance.name.upper().replace('-', '_')}_API_KEY"
+
 
 # `arbitrary_types_allowed` is needed for `client: DandiAPIClient`
 class APIBase(BaseModel, populate_by_name=True, arbitrary_types_allowed=True):

dandi/tests/fixtures.py

@@ -535,6 +535,23 @@ def instance_id(self) -> str:
     def api_url(self) -> str:
         return self.instance.api
 
+    def monkeypatch_set_api_key_env(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """
+        Monkeypatch the environment variable that provides the API key for accessing
+        the associated DANDI instance
+        """
+        monkeypatch.setenv(
+            self.client.api_key_env_var,
+            self.api_key,
+        )
+
+    def monkeypatch_del_api_key_env(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """
+        Monkeypatch to remove the environment variable that provides the API key for
+        accessing the associated DANDI instance.
+        """
+        monkeypatch.delenv(self.client.api_key_env_var, raising=False)
+
 
 @pytest.fixture(scope="session")
 def local_dandi_api(docker_compose_setup: dict[str, str]) -> Iterator[DandiAPI]:
@@ -558,7 +575,7 @@ def client(self) -> DandiAPIClient:
 
     def upload(self, paths: list[str | Path] | None = None, **kwargs: Any) -> None:
         with pytest.MonkeyPatch().context() as m:
-            m.setenv("DANDI_API_KEY", self.api.api_key)
+            self.api.monkeypatch_set_api_key_env(m)
             upload(
                 paths=paths or [self.dspath],
                 dandi_instance=self.api.instance_id,

dandi/tests/test_dandiapi.py

@@ -23,6 +23,7 @@
 from ..consts import (
     DRAFT,
     VERSION_REGEX,
+    DandiInstance,
     dandiset_identifier_regex,
     dandiset_metadata_file,
 )
@@ -138,7 +139,7 @@ def test_authenticate_bad_key_good_key_input(
     )
     confirm_mock = mocker.patch("click.confirm", return_value=True)
 
-    monkeypatch.delenv("DANDI_API_KEY", raising=False)
+    local_dandi_api.monkeypatch_del_api_key_env(monkeypatch)
 
     client = DandiAPIClient(local_dandi_api.api_url)
     assert "Authorization" not in client.session.headers
@@ -169,7 +170,7 @@ def test_authenticate_good_key_keyring(
     is_interactive_spy = mocker.spy(dandiapi, "is_interactive")
     confirm_spy = mocker.spy(click, "confirm")
 
-    monkeypatch.delenv("DANDI_API_KEY", raising=False)
+    local_dandi_api.monkeypatch_del_api_key_env(monkeypatch)
 
     client = DandiAPIClient(local_dandi_api.api_url)
     assert "Authorization" not in client.session.headers
@@ -201,7 +202,7 @@ def test_authenticate_bad_key_keyring_good_key_input(
     )
     confirm_mock = mocker.patch("click.confirm", return_value=True)
 
-    monkeypatch.delenv("DANDI_API_KEY", raising=False)
+    local_dandi_api.monkeypatch_del_api_key_env(monkeypatch)
 
     client = DandiAPIClient(local_dandi_api.api_url)
     assert "Authorization" not in client.session.headers
@@ -860,3 +861,21 @@ def test_asset_as_readable_open(new_dandiset: SampleDandiset, tmp_path: Path) ->
         assert fp.read() == b"This is test text.\n"
     finally:
         fp.close()
+
+
+@pytest.mark.parametrize(
+    ("instance_name", "expected_env_var_name"),
+    [
+        ("dandi", "DANDI_API_KEY"),
+        ("dandi-api-local-docker-tests", "DANDI_API_LOCAL_DOCKER_TESTS_API_KEY"),
+        ("dandi-sandbox", "DANDI_SANDBOX_API_KEY"),
+        ("ember-sandbox", "EMBER_SANDBOX_API_KEY"),
+    ],
+)
+def test_get_api_key_env_var(instance_name: str, expected_env_var_name: str) -> None:
+    dandi_api_client = DandiAPIClient(
+        dandi_instance=DandiInstance(
+            name=instance_name, gui="https://example.com", api="https://api.example.com"
+        )
+    )
+    assert dandi_api_client.api_key_env_var == expected_env_var_name

dandi/tests/test_delete.py

@@ -67,7 +67,7 @@ def test_delete_paths(
     remainder: list[Path],
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -92,7 +92,7 @@ def test_delete_path_confirm(
     text_dandiset: SampleDandiset,
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -113,7 +113,7 @@ def test_delete_path_pyout(
     text_dandiset: SampleDandiset,
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
     delete(["subdir2/coconut.txt"], dandi_instance=instance, force=True)
@@ -143,7 +143,7 @@ def test_delete_dandiset(
     paths: list[str],
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -166,7 +166,7 @@ def test_delete_dandiset_confirm(
     text_dandiset: SampleDandiset,
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -187,7 +187,7 @@ def test_delete_dandiset_mismatch(
     text_dandiset: SampleDandiset,
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     not_dandiset = str(int(dandiset_id) - 1).zfill(6)
@@ -216,7 +216,7 @@ def test_delete_instance_mismatch(
     text_dandiset: SampleDandiset,
 ) -> None:
     monkeypatch.chdir(text_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -242,7 +242,7 @@ def test_delete_instance_mismatch(
 def test_delete_nonexistent_dandiset(
     local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", local_dandi_api.api_key)
+    local_dandi_api.monkeypatch_set_api_key_env(monkeypatch)
     instance = local_dandi_api.instance_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
     with pytest.raises(NotFoundError) as excinfo:
@@ -259,7 +259,7 @@ def test_delete_nonexistent_dandiset(
 def test_delete_nonexistent_dandiset_skip_missing(
     local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", local_dandi_api.api_key)
+    local_dandi_api.monkeypatch_set_api_key_env(monkeypatch)
     instance = local_dandi_api.instance_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
     delete(
@@ -277,7 +277,7 @@ def test_delete_nonexistent_asset(
     monkeypatch: pytest.MonkeyPatch,
     text_dandiset: SampleDandiset,
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -304,7 +304,7 @@ def test_delete_nonexistent_asset_skip_missing(
     text_dandiset: SampleDandiset,
     tmp_path: Path,
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -333,7 +333,7 @@ def test_delete_nonexistent_asset_folder(
     monkeypatch: pytest.MonkeyPatch,
     text_dandiset: SampleDandiset,
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -360,7 +360,7 @@ def test_delete_nonexistent_asset_folder_skip_missing(
     text_dandiset: SampleDandiset,
     tmp_path: Path,
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key)
+    text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = text_dandiset.api.instance_id
     dandiset_id = text_dandiset.dandiset_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
@@ -387,7 +387,7 @@ def test_delete_nonexistent_asset_folder_skip_missing(
 def test_delete_version(
     local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    monkeypatch.setenv("DANDI_API_KEY", local_dandi_api.api_key)
+    local_dandi_api.monkeypatch_set_api_key_env(monkeypatch)
     instance = local_dandi_api.instance_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
     with pytest.raises(NotImplementedError) as excinfo:
@@ -430,7 +430,7 @@ def test_delete_zarr_path(
     tmp_path: Path,
 ) -> None:
     monkeypatch.chdir(zarr_dandiset.dspath)
-    monkeypatch.setenv("DANDI_API_KEY", zarr_dandiset.api.api_key)
+    zarr_dandiset.api.monkeypatch_set_api_key_env(monkeypatch)
     instance = zarr_dandiset.api.instance_id
     delete_spy = mocker.spy(RESTFullAPIClient, "delete")
     delete(["sample.zarr"], dandi_instance=instance, devel_debug=True, force=True)

dandi/tests/test_keyring.py

@@ -29,7 +29,7 @@ def ensure_keyring_backends() -> None:
 def test_dandi_authenticate_no_env_var(
     local_dandi_api: DandiAPI, monkeypatch: pytest.MonkeyPatch, mocker: MockerFixture
 ) -> None:
-    monkeypatch.delenv("DANDI_API_KEY", raising=False)
+    local_dandi_api.monkeypatch_del_api_key_env(monkeypatch)
     monkeypatch.setenv("PYTHON_KEYRING_BACKEND", "keyring.backends.null.Keyring")
     inputmock = mocker.patch(
         "dandi.dandiapi.input", return_value=local_dandi_api.api_key