Harden result server admin controls

Signed-off-by: Yoshifumi Nakamura <nakamura@riken.jp>

Author: yoshifuminakamura

docs/cx/BENCHKIT_GAP_ANALYSIS.md

@@ -58,7 +58,7 @@ Continuous estimation has now moved beyond a mere entry point: a common estimati
 However, estimation is still not yet broadly deployed across multiple applications, and AI-driven optimization integration remains mostly at the integration-point stage.
 
 As of the current repository survey, BenchKit has six benchmark applications with `build.sh`/`run.sh`, but only `qws` has an `estimate.sh`.
-The result portal also already has a meaningful test base (`result_server/tests`: 32 `test_*.py` modules), and the repository now has a repo-local Python dependency manifest, a standard portal test entrypoint under `result_server/tests`, and a lightweight GitHub Actions verification path for portal-oriented changes.
+The result portal also already has a meaningful test base (`result_server/tests`: 33 `test_*.py` modules), and the repository now has a repo-local Python dependency manifest, a standard portal test entrypoint under `result_server/tests`, and a lightweight GitHub Actions verification path for portal-oriented changes.
 The main GitLab pipeline still intentionally skips heavy benchmark execution when a direct or manually triggered GitLab pipeline sees changes limited to `result_server/**/*` or portal display metadata such as `config/system_info.csv`. Protected-branch synchronization itself uses `ci.skip`, so the dedicated lightweight GitHub Actions path should continue to be kept in sync as portal-side files evolve.
 
 ## 2.1 現時点で明示しておく設計負債 / Explicit Design Debts to Keep Visible
@@ -296,7 +296,7 @@ Once the estimation specification is clarified, many other design decisions beco
 
 今回のコードベース調査では、性能推定に次ぐ実務上の詰まりどころとして、`result_server` の検証導線が見えた。
 
-- `result_server/tests` には 32 個の `test_*.py` モジュールがあり、portal 側はすでに「検証すべき対象」になっている
+- `result_server/tests` には 33 個の `test_*.py` モジュールがあり、portal 側はすでに「検証すべき対象」になっている
 - repo-local な依存関係定義として `requirements-result-server.txt` があり、`result_server/tests/run_result_server_tests.py` が標準 test entrypoint として使える
 - portal-oriented 変更向けの lightweight GitHub Actions として `.github/workflows/result-server-tests.yml` が用意されている
 - `.gitlab-ci.yml` は直接または手動起動されたGitLab pipelineで `result_server/**/*` や `config/system_info.csv` 変更時に重い benchmark pipeline を skip する。保護ブランチ同期自体は `ci.skip` を使うため、GitHub Actions 側の path filter を portal 周辺の実ファイルに追従させ続ける必要がある

docs/guides/developer-reference.md

@@ -221,6 +221,7 @@ For production portal deployments:
 - See `docs/deploy/key-management.md` for generation and rotation guidance.
 - `REDIS_URL` must point to a monitored Redis instance; production authentication refuses login when Redis is unavailable.
 - API ingest and query endpoints use Redis-backed rate limits by default; set `RESULT_SERVER_MAX_UPLOAD_MB` and `RESULT_SERVER_MAX_ARCHIVE_MEMBER_MB` when deployment-specific upload limits are needed.
+- Admin-managed affiliations are only rejected when they contain unsafe path/control characters or the comma delimiter used by the form; set `RESULT_SERVER_ALLOWED_AFFILIATIONS` only when a deployment wants to enforce a fixed comma-separated allowlist.
 - `app_dev.py` is localhost-only, uses ephemeral development secrets when none are provided, and enables the Werkzeug debugger only with `RESULT_SERVER_DEV_DEBUG=1`.
 
 ### Result Quality Visibility

result_server/app.py

@@ -9,6 +9,7 @@
 from routes.estimated import estimated_bp
 from routes.home import register_home_routes
 from routes.results import results_bp
+from utils.admin_policy import parse_allowed_affiliations
 from utils.auth import parse_ingest_keys
 from utils.csrf import init_csrf
 from utils.preflight import validate_production_config
@@ -94,6 +95,13 @@ def payload_too_large(_error):
         ), 413
 
 
+def _configure_admin_policy(app):
+    """Configure admin-route policy settings."""
+    app.config["ALLOWED_AFFILIATIONS"] = parse_allowed_affiliations(
+        os.environ.get("RESULT_SERVER_ALLOWED_AFFILIATIONS")
+    )
+
+
 def _register_portal_blueprints(app, prefix):
     """Register all portal blueprints using the given URL prefix."""
     from routes.admin import admin_bp
@@ -127,6 +135,7 @@ def create_app(prefix="", base_dir=None):
     _configure_totp_issuer(app, prefix)
     _configure_result_directories(app, base_dir)
     _configure_upload_limits(app)
+    _configure_admin_policy(app)
     init_csrf(app, exempt_blueprints=(api_bp,))
 
     register_home_routes(app, prefix=prefix)

result_server/app_dev.py

@@ -162,6 +162,7 @@ def create_dev_app(base_dir):
 
     from routes.home import register_home_routes
     from routes.security_metadata import register_security_metadata_routes
+    from utils.admin_policy import parse_allowed_affiliations
     from utils.auth import parse_ingest_keys
     from utils.csrf import init_csrf
     from utils.system_info import get_all_systems_info, summarize_systems_info
@@ -185,6 +186,9 @@ def create_dev_app(base_dir):
             * 1024
             * 1024
         ),
+        ALLOWED_AFFILIATIONS=parse_allowed_affiliations(
+            os.environ.get("RESULT_SERVER_ALLOWED_AFFILIATIONS")
+        ),
     )
     Session(app)
 

result_server/create_admin.py

@@ -22,9 +22,11 @@
 """
 
 import argparse
+import os
 
 import redis
 
+from utils.admin_policy import parse_affiliations, parse_allowed_affiliations
 from utils.user_store import UserStore
 
 
@@ -53,7 +55,10 @@ def main():
     )
     args = parser.parse_args()
 
-    affiliations = [item.strip() for item in args.affiliations.split(",") if item.strip()]
+    allowed = parse_allowed_affiliations(os.environ.get("RESULT_SERVER_ALLOWED_AFFILIATIONS"))
+    affiliations, invalid = parse_affiliations(args.affiliations, allowed)
+    if invalid:
+        parser.error(f"Invalid affiliations: {', '.join(sorted(invalid))}.")
 
     r_conn = redis.from_url(args.redis_url, decode_responses=True)
     store = UserStore(r_conn, args.prefix)

result_server/routes/admin.py

@@ -5,6 +5,7 @@
 from flask import (
     Blueprint,
     abort,
+    current_app,
     flash,
     make_response,
     redirect,
@@ -14,8 +15,9 @@
     url_for,
 )
 
-from utils.user_store import get_user_store
+from utils.admin_policy import parse_affiliations
 from utils.rate_limit import rate_limited
+from utils.user_store import get_user_store
 
 admin_bp = Blueprint("admin", __name__, url_prefix="/admin")
 
@@ -39,6 +41,40 @@ def _admin_rate_key(_request):
     return f"admin:{session.get('user_email', 'anon')}"
 
 
+def _allowed_affiliations():
+    """Return the configured affiliation allowlist, if one is enforced."""
+    return current_app.config.get("ALLOWED_AFFILIATIONS")
+
+
+def _parse_requested_affiliations():
+    """Parse submitted affiliations and flash an error for invalid values."""
+    affiliations_raw = request.form.get("affiliations", "").strip()
+    affiliations, invalid = parse_affiliations(affiliations_raw, _allowed_affiliations())
+    if invalid:
+        flash(f"Invalid affiliations: {', '.join(sorted(invalid))}.")
+        return None
+    return affiliations
+
+
+def _user_affiliations(store, email):
+    """Return the affiliations for a user, handling missing records uniformly."""
+    if hasattr(store, "get_user"):
+        user = store.get_user(email)
+    else:
+        user = next(
+            (item for item in store.list_users() if item.get("email") == email),
+            None,
+        )
+    if not user:
+        return None
+    return list(user.get("affiliations", []))
+
+
+def _admin_user_count(store):
+    """Return the number of stored users with the admin affiliation."""
+    return sum(1 for user in store.list_users() if "admin" in user.get("affiliations", []))
+
+
 def admin_required(f):
     """Allow access only to authenticated users with the admin affiliation."""
 
@@ -68,8 +104,9 @@ def add_user():
     """Create a user invitation and show the generated invitation URL."""
     store = get_user_store()
     email = request.form.get("email", "").strip()
-    affiliations_raw = request.form.get("affiliations", "").strip()
-    affiliations = [item.strip() for item in affiliations_raw.split(",") if item.strip()]
+    affiliations = _parse_requested_affiliations()
+    if affiliations is None:
+        return redirect(url_for("admin.users"))
 
     if not email:
         flash("Email is required.")
@@ -86,7 +123,7 @@ def add_user():
     return _render_users_page(invitation_url)
 
 
-@admin_bp.route("/users/<path:email>/delete", methods=["POST"])
+@admin_bp.route("/users/<email>/delete", methods=["POST"])
 @admin_required
 @rate_limited(max_per_minute=20, key_fn=_admin_rate_key, scope="admin_write")
 def delete_user(email):
@@ -95,25 +132,44 @@ def delete_user(email):
         flash("You cannot delete your own account.")
         return redirect(url_for("admin.users"))
     store = get_user_store()
+    affiliations = _user_affiliations(store, email)
+    if affiliations is None:
+        flash(f"User {email} not found.")
+        return redirect(url_for("admin.users"))
+    if "admin" in affiliations and _admin_user_count(store) <= 1:
+        flash("You cannot delete the only admin user.")
+        return redirect(url_for("admin.users"))
     store.delete_user(email)
     flash(f"User {email} has been deleted.")
     return redirect(url_for("admin.users"))
 
 
-@admin_bp.route("/users/<path:email>/affiliations", methods=["POST"])
+@admin_bp.route("/users/<email>/affiliations", methods=["POST"])
 @admin_required
 @rate_limited(max_per_minute=20, key_fn=_admin_rate_key, scope="admin_write")
 def update_affiliations(email):
     """Update the affiliations stored for a user."""
     store = get_user_store()
-    affiliations_raw = request.form.get("affiliations", "").strip()
-    affiliations = [item.strip() for item in affiliations_raw.split(",") if item.strip()]
+    current_affiliations = _user_affiliations(store, email)
+    if current_affiliations is None:
+        flash(f"User {email} not found.")
+        return redirect(url_for("admin.users"))
+    affiliations = _parse_requested_affiliations()
+    if affiliations is None:
+        return redirect(url_for("admin.users"))
+    if (
+        "admin" in current_affiliations
+        and "admin" not in affiliations
+        and _admin_user_count(store) <= 1
+    ):
+        flash("You cannot remove admin from the only admin user.")
+        return redirect(url_for("admin.users"))
     store.update_affiliations(email, affiliations)
     flash(f"Affiliations updated for {email}.")
     return redirect(url_for("admin.users"))
 
 
-@admin_bp.route("/users/<path:email>/reinvite", methods=["POST"])
+@admin_bp.route("/users/<email>/reinvite", methods=["POST"])
 @admin_required
 @rate_limited(max_per_minute=20, key_fn=_admin_rate_key, scope="admin_write")
 def reinvite_user(email):

result_server/test_support.py

@@ -81,15 +81,15 @@ def users():
     def add_user():
         return ""
 
-    @admin_bp.route("/users/<path:email>/affiliations", methods=["POST"])
+    @admin_bp.route("/users/<email>/affiliations", methods=["POST"])
     def update_affiliations(email):
         return email
 
-    @admin_bp.route("/users/<path:email>/reinvite", methods=["POST"])
+    @admin_bp.route("/users/<email>/reinvite", methods=["POST"])
     def reinvite_user(email):
         return email
 
-    @admin_bp.route("/users/<path:email>/delete", methods=["POST"])
+    @admin_bp.route("/users/<email>/delete", methods=["POST"])
     def delete_user(email):
         return email