SEP-1875: Warn when a reused sep service account's Grafana role is below Admin

changelog.d/SEP-1875.changed.md

@@ -0,0 +1 @@
+When the side-car mints a Grafana token onto an existing `sep` service account whose org role ranks below Admin, it now warns on stderr naming the account and the role gap, then keeps the token; a freshly created account is unchanged.

sidecar/grafana_service_account.py

@@ -290,7 +290,7 @@ async def search_account(provider: GrafanaSDK) -> int | None:
     return None
 
 
-async def find_or_create_account(provider: GrafanaSDK) -> int:
+async def find_or_create_account(provider: GrafanaSDK) -> tuple[int, bool]:
     """Return the id of SEP's service account, creating it when absent.
 
     A refused creation is re-checked against a second lookup rather than
@@ -299,16 +299,18 @@ async def find_or_create_account(provider: GrafanaSDK) -> int:
     The lookup decides that, not the refusal's status, because Grafana's
     duplicate-name status varies by release.
 
+    The second element is ``True`` when this call created the account.
+
     :param provider: The open Grafana client, authenticated as the admin.
-    :return: The service account's id.
+    :return: The service account's id, and whether this call created it.
     :raises MintError: When Grafana creates an account but answers no id.
     :raises HTTPException: When Grafana refuses the creation and no account of
         that name exists afterwards.
     :raises ClientError: When Grafana cannot be reached at all.
     """
     existing = await search_account(provider)
     if existing is not None:
-        return existing
+        return existing, False
     try:
         created = await provider.post(
             SERVICE_ACCOUNTS_PATH,
@@ -322,18 +324,18 @@ async def find_or_create_account(provider: GrafanaSDK) -> int:
         winner = await search_account(provider)
         if winner is None:
             raise
-        return winner
+        return winner, False
     account_id = created.get("id") if isinstance(created, Mapping) else None
     if account_id is None:
         raise MintError(
             f"Grafana answered no service-account id when creating "
             f"{SERVICE_ACCOUNT_NAME!r}; the response was a "
             f"{type(created).__name__}."
         )
-    return account_id
+    return account_id, True
 
 
-async def mint(provider: GrafanaSDK, credentials: str) -> str:
+async def mint(provider: GrafanaSDK, credentials: str) -> tuple[str, bool]:
     """Obtain a fresh service-account token from Grafana in one attempt.
 
     ``secondsToLive`` is left out of the request: Grafana reads its absence as
@@ -342,7 +344,7 @@ async def mint(provider: GrafanaSDK, credentials: str) -> str:
 
     :param provider: The open Grafana client.
     :param credentials: The Base64 admin pair to authenticate with.
-    :return: The minted token.
+    :return: The minted token, and whether it was minted onto a reused account.
     :raises MintError: When Grafana answers a token response carrying no key, or
         creates an account but answers no id.
     :raises HTTPException: When Grafana answers an error status.
@@ -352,7 +354,7 @@ async def mint(provider: GrafanaSDK, credentials: str) -> str:
         provider.auth(credentials, auth_scheme="Basic"),
         quiet_client_logging(provider, logging.INFO),
     ):
-        account_id = await find_or_create_account(provider)
+        account_id, account_created = await find_or_create_account(provider)
         created = await provider.post(
             f"{SERVICE_ACCOUNTS_PATH}/{account_id}/tokens", json={"name": token_name()}
         )
@@ -363,12 +365,12 @@ async def mint(provider: GrafanaSDK, credentials: str) -> str:
             f"service account {account_id}; the response was a "
             f"{type(created).__name__}."
         )
-    return token.strip()
+    return token.strip(), not account_created
 
 
 async def mint_with_retry(
     provider: GrafanaSDK, credentials: str, deadline: float
-) -> str:
+) -> tuple[str, bool]:
     """Mint a token, retrying only what a still-starting Grafana explains.
 
     Each attempt carries its own timeout off ``deadline``: the client's session
@@ -379,7 +381,7 @@ async def mint_with_retry(
     :param provider: The open Grafana client.
     :param credentials: The Base64 admin pair to authenticate with.
     :param deadline: The :func:`time.monotonic` reading to give up at.
-    :return: The minted token.
+    :return: The minted token, and whether it was minted onto a reused account.
     :raises MintError: When Grafana answers a fault no retry can clear, or when
         the deadline passes first.
     """
@@ -480,6 +482,19 @@ def resolve_provider() -> GrafanaSDK | None:
     return provider
 
 
+def warn_role_gap(subject: str) -> None:
+    """Warn that SEP's service account ranks below Admin in its org.
+
+    :param subject: What Grafana accepted, e.g. ``"persisted token"``.
+    """
+    warn(
+        f"Grafana accepts the {subject} but the "
+        f"{SERVICE_ACCOUNT_NAME!r} service account ranks below "
+        f"{SERVICE_ACCOUNT_ROLE} in its org; a re-mint would carry the same "
+        "role, so the token is kept."
+    )
+
+
 async def keep_persisted_token(provider: GrafanaSDK, token: str) -> bool:
     """Return whether Grafana's answer leaves the persisted token usable.
 
@@ -489,12 +504,7 @@ async def keep_persisted_token(provider: GrafanaSDK, token: str) -> bool:
     """
     state = await validate_token(provider, token)
     if state is TokenStateEnum.FORBIDDEN:
-        warn(
-            f"Grafana accepts the persisted token but the "
-            f"{SERVICE_ACCOUNT_NAME!r} service account ranks below "
-            f"{SERVICE_ACCOUNT_ROLE} in its org; a re-mint would carry the same "
-            "role, so the token is kept."
-        )
+        warn_role_gap("persisted token")
     elif state is TokenStateEnum.UNREACHABLE:
         warn(
             f"Could not reach Grafana at {provider.endpoint} to revalidate "
@@ -506,6 +516,13 @@ async def keep_persisted_token(provider: GrafanaSDK, token: str) -> bool:
 async def resolve_token() -> str | None:
     """Resolve the token for the three ranks below the mounted secrets channel.
 
+    When a token is minted onto a reused service account, the token is probed
+    with :func:`validate_token`. A ``FORBIDDEN`` answer means the account's
+    org role ranks below ``Admin``; a diagnostic is written and the token is
+    still returned, because a re-mint cannot raise the role. An
+    ``UNREACHABLE`` answer gets the same keep-the-token treatment with a
+    reachability diagnostic, matching :func:`keep_persisted_token`.
+
     :return: The resolved token, or ``None`` when there is nothing to resolve.
     :raises MintError: When a token is needed and cannot be minted.
     """
@@ -522,7 +539,18 @@ async def resolve_token() -> str | None:
                 provider, persisted
             ):
                 return persisted
-            token = await mint_with_retry(provider, admin_credentials(), deadline)
+            token, reused = await mint_with_retry(
+                provider, admin_credentials(), deadline
+            )
+            if reused:
+                probe = await validate_token(provider, token)
+                if probe is TokenStateEnum.FORBIDDEN:
+                    warn_role_gap("freshly minted token")
+                elif probe is TokenStateEnum.UNREACHABLE:
+                    warn(
+                        f"Could not reach Grafana at {provider.endpoint} to "
+                        "probe the freshly minted token; using it unvalidated."
+                    )
     write_persisted_token(directory, token)
     return token
 

tests/sidecar/test_grafana_service_account.py

@@ -341,9 +341,10 @@ async def test_a_mint_creates_the_account_then_a_token_on_it(
 ):
     """Find nothing, create the account, then create a token on the new account."""
     async with provider:
-        token = await helper.mint(provider, helper.admin_credentials())
+        token, reused = await helper.mint(provider, helper.admin_credentials())
 
     assert token == MINTED_TOKEN
+    assert not reused
     assert [request.route for request in grafana_stub.requests] == [
         StubRoute.SEARCH,
         StubRoute.CREATE_ACCOUNT,
@@ -415,8 +416,10 @@ async def test_an_existing_account_is_reused_rather_than_duplicated(
     )
 
     async with provider:
-        await helper.mint(provider, helper.admin_credentials())
+        token, reused = await helper.mint(provider, helper.admin_credentials())
 
+    assert token == MINTED_TOKEN
+    assert reused
     assert not grafana_stub.calls(StubRoute.CREATE_ACCOUNT)
     assert grafana_stub.calls(StubRoute.CREATE_TOKEN)[0].path.endswith(
         f"/serviceaccounts/{ACCOUNT_ID}/tokens"
@@ -477,9 +480,10 @@ async def test_a_lost_account_creation_race_mints_on_the_winner(
     )
 
     async with provider:
-        token = await helper.mint(provider, helper.admin_credentials())
+        token, reused = await helper.mint(provider, helper.admin_credentials())
 
     assert token == MINTED_TOKEN
+    assert reused
     assert grafana_stub.calls(StubRoute.CREATE_TOKEN)[0].path.endswith(
         f"/serviceaccounts/{ACCOUNT_ID}/tokens"
     )
@@ -612,13 +616,14 @@ async def test_a_starting_grafana_is_retried_until_it_answers(
     )
 
     async with provider:
-        token = await helper.mint_with_retry(
+        token, reused = await helper.mint_with_retry(
             provider,
             helper.admin_credentials(),
             time.monotonic() + PATIENT_BOUND_SECONDS,
         )
 
     assert token == MINTED_TOKEN
+    assert not reused
     assert len(grafana_stub.calls(StubRoute.SEARCH)) == EXPECTED_SEARCH_ATTEMPTS
 
 
@@ -705,9 +710,10 @@ async def test_the_minted_token_reaches_no_log_record(
 
     with caplog.at_level(logging.DEBUG):
         async with provider:
-            token = await helper.mint(provider, helper.admin_credentials())
+            token, reused = await helper.mint(provider, helper.admin_credentials())
 
     assert token == MINTED_TOKEN
+    assert not reused
     assert not [
         record for record in caplog.records if MINTED_TOKEN in record.getMessage()
     ]
@@ -855,7 +861,78 @@ def test_the_mint_bound_falls_back_rather_than_raising(
 async def test_a_first_start_mints_and_persists_a_token(
     grafana_stub: GrafanaStub, tmp_path: Path, state_dir: Path
 ):
-    """Resolve a token on a fresh install with nothing configured anywhere."""
+    """Resolve a token on a fresh install with nothing configured anywhere.
+
+    The create path already requests Admin, so the post-mint role probe is
+    skipped — only a reused account needs that round trip.
+    """
+    run = await run_helper(
+        profile_cwd(tmp_path),
+        AUTH__PROVIDER__GRAFANA__ENDPOINT=grafana_stub.endpoint,
+        SEP_STATE_DIR=str(state_dir),
+    )
+
+    assert run.returncode == 0, run.stderr
+    assert run.token == MINTED_TOKEN
+    assert helper.read_persisted_token(state_dir) == MINTED_TOKEN
+    assert grafana_stub.calls(StubRoute.CREATE_ACCOUNT)
+    assert not grafana_stub.calls(StubRoute.VALIDATE)
+
+
+@pytest.mark.asyncio
+async def test_a_reused_account_at_admin_probes_without_warning(
+    grafana_stub: GrafanaStub, tmp_path: Path, state_dir: Path
+):
+    """Probe a reused account that already holds Admin, and stay quiet on ACCEPTED.
+
+    Guards the FORBIDDEN-only warn: a flipped guard would emit a false role-gap
+    diagnostic on every healthy reuse.
+    """
+    grafana_stub.queue(
+        StubRoute.SEARCH,
+        StubResponse(
+            {
+                "totalCount": 1,
+                "serviceAccounts": [{"id": ACCOUNT_ID, "name": "sep"}],
+            }
+        ),
+    )
+
+    run = await run_helper(
+        profile_cwd(tmp_path),
+        AUTH__PROVIDER__GRAFANA__ENDPOINT=grafana_stub.endpoint,
+        SEP_STATE_DIR=str(state_dir),
+    )
+
+    assert run.returncode == 0, run.stderr
+    assert run.token == MINTED_TOKEN
+    assert helper.read_persisted_token(state_dir) == MINTED_TOKEN
+    assert (
+        grafana_stub.calls(StubRoute.VALIDATE)[0].headers["Authorization"]
+        == f"Bearer {MINTED_TOKEN}"
+    )
+    assert "ranks below" not in run.stderr
+
+
+@pytest.mark.asyncio
+async def test_a_reused_account_below_admin_warns_after_mint(
+    grafana_stub: GrafanaStub, tmp_path: Path, state_dir: Path
+):
+    """Warn on FORBIDDEN after minting onto an existing under-privileged account.
+
+    The token is still returned and persisted: a re-mint cannot raise the role.
+    """
+    grafana_stub.queue(
+        StubRoute.SEARCH,
+        StubResponse(
+            {
+                "totalCount": 1,
+                "serviceAccounts": [{"id": ACCOUNT_ID, "name": "sep"}],
+            }
+        ),
+    )
+    grafana_stub.queue(StubRoute.VALIDATE, StubResponse(status=403))
+
     run = await run_helper(
         profile_cwd(tmp_path),
         AUTH__PROVIDER__GRAFANA__ENDPOINT=grafana_stub.endpoint,
@@ -865,6 +942,90 @@ async def test_a_first_start_mints_and_persists_a_token(
     assert run.returncode == 0, run.stderr
     assert run.token == MINTED_TOKEN
     assert helper.read_persisted_token(state_dir) == MINTED_TOKEN
+    assert (
+        grafana_stub.calls(StubRoute.VALIDATE)[0].headers["Authorization"]
+        == f"Bearer {MINTED_TOKEN}"
+    )
+    assert f"{helper.SERVICE_ACCOUNT_NAME!r}" in run.stderr
+    assert helper.SERVICE_ACCOUNT_ROLE in run.stderr
+    assert "ranks below" in run.stderr
+
+
+@pytest.mark.asyncio
+async def test_a_reused_account_with_unreachable_probe_warns_after_mint(
+    grafana_stub: GrafanaStub, tmp_path: Path, state_dir: Path
+):
+    """Warn on UNREACHABLE after minting onto an existing account.
+
+    Grafana may go down between the mint and the probe; the token is still
+    returned and persisted, matching :func:`keep_persisted_token`.
+    """
+    grafana_stub.queue(
+        StubRoute.SEARCH,
+        StubResponse(
+            {
+                "totalCount": 1,
+                "serviceAccounts": [{"id": ACCOUNT_ID, "name": "sep"}],
+            }
+        ),
+    )
+    grafana_stub.queue(StubRoute.VALIDATE, StubResponse(status=503))
+
+    run = await run_helper(
+        profile_cwd(tmp_path),
+        AUTH__PROVIDER__GRAFANA__ENDPOINT=grafana_stub.endpoint,
+        SEP_STATE_DIR=str(state_dir),
+    )
+
+    assert run.returncode == 0, run.stderr
+    assert run.token == MINTED_TOKEN
+    assert helper.read_persisted_token(state_dir) == MINTED_TOKEN
+    assert (
+        grafana_stub.calls(StubRoute.VALIDATE)[0].headers["Authorization"]
+        == f"Bearer {MINTED_TOKEN}"
+    )
+    assert "Could not reach Grafana" in run.stderr
+    assert "probe the freshly minted token" in run.stderr
+    assert "using it unvalidated" in run.stderr
+
+
+@pytest.mark.asyncio
+async def test_a_race_recovery_reuse_probes_the_minted_token(
+    grafana_stub: GrafanaStub, tmp_path: Path, state_dir: Path
+):
+    """Probe after minting onto the account a concurrent side-car created.
+
+    Losing the create race still reuses an account whose role SEP never set, so
+    the same FORBIDDEN diagnostic applies.
+    """
+    grafana_stub.queue(
+        StubRoute.SEARCH,
+        StubResponse({"totalCount": 0, "serviceAccounts": []}),
+        StubResponse(
+            {"totalCount": 1, "serviceAccounts": [{"id": ACCOUNT_ID, "name": "sep"}]}
+        ),
+    )
+    grafana_stub.queue(
+        StubRoute.CREATE_ACCOUNT,
+        StubResponse({"message": "service account already exists"}, status=400),
+    )
+    grafana_stub.queue(StubRoute.VALIDATE, StubResponse(status=403))
+
+    run = await run_helper(
+        profile_cwd(tmp_path),
+        AUTH__PROVIDER__GRAFANA__ENDPOINT=grafana_stub.endpoint,
+        SEP_STATE_DIR=str(state_dir),
+    )
+
+    assert run.returncode == 0, run.stderr
+    assert run.token == MINTED_TOKEN
+    assert helper.read_persisted_token(state_dir) == MINTED_TOKEN
+    assert (
+        grafana_stub.calls(StubRoute.VALIDATE)[0].headers["Authorization"]
+        == f"Bearer {MINTED_TOKEN}"
+    )
+    assert "ranks below" in run.stderr
+    assert helper.SERVICE_ACCOUNT_ROLE in run.stderr
 
 
 @pytest.mark.asyncio