Skip to content

Commit ff2dfac

Browse files
authored
fix(portal): wire ADMIN_PASS through env_file; treat empty pass as unset (#47)
## Problem The multitenant compose file (`infra/docker-compose-multitenant-nats.yml`) documents `ADMIN_PASS` as settable "via env var or `.env` file", but the `portal` service has **no `env_file`** and no inline `ADMIN_PASS` reference. Compose's project-level `.env` is **interpolation-only** and never reaches the container, so a deployer's `infra/.env` `ADMIN_PASS` is silently ignored — the portal mints a **fresh random admin password on every rebuild**, breaking the existing admin login. In practice operators work around this with an untracked local edit (`ADMIN_PASS: ${ADMIN_PASS}`) that is easily lost on `git pull`/`checkout` during an upgrade. There's a second, nastier trap in the config logic: ```python ADMIN_PASS = os.environ.get("ADMIN_PASS") or secrets.token_urlsafe(16) ADMIN_PASS_GENERATED = "ADMIN_PASS" not in os.environ ``` If `ADMIN_PASS=""` (set but empty), the value path generates a random password (because `""` is falsy), but `ADMIN_PASS_GENERATED` is `False` — so it **generates a password and never logs it**, locking the operator out with no recovery. ## Fix 1. **compose** — add an optional `env_file` (`infra/.env`) to the `portal` service so `ADMIN_PASS` and other deploy secrets actually reach the container. `required: false` keeps the inline-env-var usage (`DC_TENANTS=... docker compose up`) working when no `.env` exists. 2. **config.py** — treat an empty `ADMIN_PASS` the same as unset: generate **and** log. Closes the silent-lockout path regardless of how the env is provided. 3. **docs** — document `ADMIN_USER`/`ADMIN_PASS` in `.env.multitenant.example`. ## Notes - `env_file` paths in this compose file resolve relative to the compose file's dir (`infra/`), matching the existing `build.context: ../../..` convention, so `path: .env` → `infra/.env`. - `environment:` still wins over `env_file` for keys it sets (e.g. `ADMIN_USER`), so no precedence surprises. - No version bump included.
1 parent 5ea5d30 commit ff2dfac

4 files changed

Lines changed: 92 additions & 4 deletions

File tree

packages/device-connect-server/device_connect_server/portal/config.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,14 @@
3737
ETCD_PORT = int(os.environ.get("ETCD_PORT", "2379"))
3838

3939
# Admin credentials
40+
# Treat an empty ADMIN_PASS the same as unset: generate a password AND flag it
41+
# as generated so it gets logged. Keying ADMIN_PASS_GENERATED off membership in
42+
# os.environ alone would silently generate a password (because "" is falsy) but
43+
# never log it, locking the operator out.
4044
ADMIN_USER = os.environ.get("ADMIN_USER", "admin")
41-
ADMIN_PASS = os.environ.get("ADMIN_PASS") or secrets.token_urlsafe(16)
42-
ADMIN_PASS_GENERATED = "ADMIN_PASS" not in os.environ
45+
_admin_pass = os.environ.get("ADMIN_PASS") or None
46+
ADMIN_PASS = _admin_pass or secrets.token_urlsafe(16)
47+
ADMIN_PASS_GENERATED = _admin_pass is None
4348

4449
# Paths
4550
SECURITY_INFRA_DIR = Path(os.environ.get(

packages/device-connect-server/infra/.env.multitenant.example

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,11 @@
22
#
33
# List of tenants (comma-separated, must match tenants created with manage_tenants.sh)
44
DC_TENANTS=alpha,beta,gamma,delta,epsilon
5+
6+
# Admin login for the portal. Once this file is copied to infra/.env (see the
7+
# header above), both values are injected into the portal container via the
8+
# portal service's env_file. ADMIN_USER defaults to "admin" if omitted. If
9+
# ADMIN_PASS is left unset/empty, the portal generates a random password on
10+
# startup and prints it to the logs.
11+
ADMIN_USER=admin
12+
# ADMIN_PASS=change-me

packages/device-connect-server/infra/docker-compose-multitenant-nats.yml

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,22 @@ services:
110110
NATS_CONTAINER: dc-nats
111111
ETCD_HOST: etcd
112112
ETCD_PORT: "2379"
113-
ADMIN_USER: ${ADMIN_USER:-admin}
114-
# ADMIN_PASS: set via env var or .env file; auto-generated if omitted
113+
# Admin creds (ADMIN_USER, ADMIN_PASS) come from infra/.env via env_file
114+
# below -- deliberately NOT listed here, since an environment: entry would
115+
# override env_file. The compose-level .env is interpolation-only and
116+
# never reaches the container. If ADMIN_PASS is unset the portal
117+
# auto-generates one and logs it; ADMIN_USER defaults to "admin".
115118
SECURITY_INFRA_DIR: /app/security_infra
116119
CREDS_DIR: /root/.device-connect/credentials
120+
# Inject infra/.env into the container so admin creds (and any other deploy
121+
# secrets) reach the portal. This is the ONLY path that delivers ADMIN_USER/
122+
# ADMIN_PASS to the container -- host env vars (e.g. ADMIN_PASS=... docker
123+
# compose up) are not forwarded. Optional (required: false): if infra/.env
124+
# is absent, ADMIN_USER falls back to "admin" and the portal auto-generates
125+
# and logs the admin password.
126+
env_file:
127+
- path: .env # NOTE: requires a Docker Compose version that supports env_file objects + `required`; otherwise create infra/.env
128+
required: false
117129
volumes:
118130
- ~/.device-connect/credentials:/root/.device-connect/credentials
119131
- ../security_infra:/app/security_infra
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Copyright (c) 2024-2026, Arm Limited and Contributors. All rights reserved.
2+
#
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
"""Regression tests for portal admin-password resolution.
6+
7+
``portal.config`` resolves ``ADMIN_PASS`` at import time and exposes
8+
``ADMIN_PASS_GENERATED`` so the startup path knows whether to log the
9+
generated password. The trap these tests pin: an *empty* ``ADMIN_PASS``
10+
must be treated exactly like an unset one -- generate a random password
11+
AND flag it as generated so it gets logged. The earlier implementation
12+
keyed ``ADMIN_PASS_GENERATED`` off ``"ADMIN_PASS" not in os.environ``,
13+
so ``ADMIN_PASS=""`` silently generated a password but never logged it,
14+
locking the operator out.
15+
"""
16+
17+
from __future__ import annotations
18+
19+
import importlib
20+
21+
import pytest
22+
23+
24+
def _reload_config(monkeypatch, value):
25+
"""Reload portal.config with ADMIN_PASS set to ``value`` (None = unset)."""
26+
if value is None:
27+
monkeypatch.delenv("ADMIN_PASS", raising=False)
28+
else:
29+
monkeypatch.setenv("ADMIN_PASS", value)
30+
from device_connect_server.portal import config
31+
32+
return importlib.reload(config)
33+
34+
35+
def test_unset_admin_pass_is_generated(monkeypatch):
36+
config = _reload_config(monkeypatch, None)
37+
assert config.ADMIN_PASS_GENERATED is True
38+
assert config.ADMIN_PASS # a password was generated
39+
40+
41+
def test_empty_admin_pass_is_generated(monkeypatch):
42+
config = _reload_config(monkeypatch, "")
43+
# Empty string must behave like unset: generated AND flagged so it logs.
44+
assert config.ADMIN_PASS_GENERATED is True
45+
assert config.ADMIN_PASS
46+
47+
48+
def test_explicit_admin_pass_is_not_generated(monkeypatch):
49+
config = _reload_config(monkeypatch, "s3cret-pass")
50+
assert config.ADMIN_PASS_GENERATED is False
51+
assert config.ADMIN_PASS == "s3cret-pass"
52+
53+
54+
@pytest.fixture(scope="module", autouse=True)
55+
def _restore_config():
56+
"""Reload config from the ambient environment once the module finishes so
57+
the import-time ADMIN_PASS state doesn't leak into other test modules. This
58+
is module-scoped so it runs after every per-test ``monkeypatch`` has already
59+
restored ``os.environ``."""
60+
yield
61+
from device_connect_server.portal import config
62+
63+
importlib.reload(config)

0 commit comments

Comments
 (0)