Decide who may ask, with none and bearer

README.md

@@ -90,12 +90,23 @@ Configuration is by environment variable, and `example.env` lists them:
 | `BIOCHEF_RUN_TIMEOUT` | `900` | seconds before a run's whole process group is killed |
 | `BIOCHEF_KEEP_WORKSPACE` | `false` | leave a run's directory behind, for debugging |
 | `BIOCHEF_MAX_UPLOAD_BYTES` | `536870912` | largest request body accepted, in bytes |
+| `BIOCHEF_AUTH` | `none` | who may call it: `none` or `bearer` |
+| `BIOCHEF_AUTH_TOKEN` | | the shared token, required when `BIOCHEF_AUTH=bearer` |
 | `BIOCHEF_RUNNER` | `subprocess` | how a workflow executes: `subprocess` or `apptainer` |
 | `BIOCHEF_CONTAINER_IMAGE` | `docker://debian:stable-slim` | image each step runs in, under the `apptainer` runner |
 | `BIOCHEF_APPTAINER_CACHE` | `apptainer-cache` | where pulled container images are kept between runs |
 | `BIOCHEF_APPTAINER_ARGS` | `--contain` | extra flags for apptainer itself |
 
-Three of those decide how isolated a run is, and are worth reading twice before
+`BIOCHEF_AUTH` defaults to `none`, which means **any caller that can open a
+socket to this service can make it execute tool binaries**. That is a reasonable
+default on a laptop and the wrong one anywhere else. `bearer` requires
+`Authorization: Bearer <token>` matching `BIOCHEF_AUTH_TOKEN`; a shared secret is
+not identity -- every holder is the same caller -- but it is the difference
+between an open endpoint and a closed one. Selecting `bearer` without a token
+stops the service from starting rather than letting it run with a token nobody
+has to guess.
+
+Three more decide how isolated a run is, and are worth reading twice before
 changing.
 
 `BIOCHEF_RUNNER` defaults to `subprocess`, which runs every step **on the host,
@@ -141,15 +152,22 @@ run's data**. Emptying this variable turns that off deliberately.
 
 ## Before deploying this
 
-**It is not ready to be exposed.** There is no authentication of any kind, and
-several open issues describe defects reachable by anyone who can reach the port.
-The most significant are tracked in the issue tracker; read them before putting
-this anywhere a stranger can send it a request.
+**It is not ready to be exposed.** Several open issues describe defects reachable
+by anyone who can reach the port. The most significant are tracked in the issue
+tracker; read them before putting this anywhere a stranger can send it a request.
+
+Authentication now exists but is **off by default**. `BIOCHEF_AUTH=none` is the
+default, and it means what it says: any caller that can open a socket can make
+this service execute tool binaries. Setting `BIOCHEF_AUTH=bearer` closes that,
+and a deployment that does not is choosing to leave it open.
+
+Even set, a shared token is not identity — every holder is the same caller, and
+nothing yet decides what a given caller may run or read. That is F3 (Passports),
+and it does not exist.
 
 That matters more here than the sentence usually implies. The environments this
 is aimed at are the ones where it would do the most damage: an agent inside a
-TRE sits next to data that is there precisely because it may not leave. Deciding
-what a caller may run and read is C2 and F3, and neither exists yet.
+TRE sits next to data that is there precisely because it may not leave.
 
 Development is organised as numbered workstreams (A–G) in the issues: the
 converter and its intermediate model, asynchronous runs, authentication and

auth.py

@@ -0,0 +1,179 @@
+"""Who may ask this service to run something (#10).
+
+The same shape as runner.py, and for the same reason. An interface, providers
+selected by name, and the policy that every provider shares written once. F3
+(Passports) should be a third provider here rather than a rewrite of the second.
+
+What a provider decides is narrow on purpose: given a request, either it names a
+caller or it refuses. It does not decide what that caller may do -- there is no
+authorisation model yet, and pretending otherwise by returning roles nobody
+consults would be worse than the gap.
+"""
+
+import hmac
+import json
+import os
+from typing import Optional
+
+from fastapi import HTTPException, Request
+
+
+class Unauthenticated(HTTPException):
+    """401, with the challenge the specification requires.
+
+    A 401 without WWW-Authenticate is not a well-formed refusal: RFC 9110 makes
+    the header mandatory on 401, and it is what tells a client which scheme to
+    try. 403 would be the wrong code -- it means "you are known and still may
+    not", which is a statement this service is not yet in a position to make.
+    """
+
+    def __init__(self, detail: str, scheme: str = "Bearer"):
+        super().__init__(status_code=401, detail=detail,
+                         headers={"WWW-Authenticate": scheme})
+
+
+AUTH_TOKEN = os.getenv("BIOCHEF_AUTH_TOKEN", "")
+"""The shared secret, when BIOCHEF_AUTH=bearer.
+
+Read from the environment rather than a file or an argument, so it does not end
+up in a process listing or in shell history.
+"""
+
+
+class AuthProvider:
+    """Decides whether a request may proceed, and on whose behalf."""
+
+    name = "provider"
+
+    def authenticate(self, request: Request) -> Optional[str]:
+        """Return an identity for the caller, or raise Unauthenticated.
+
+        The return value is deliberately just a name. Nothing consults it yet;
+        it exists so that a provider which knows more about the caller -- a
+        Passport carries claims -- has somewhere to put it without changing the
+        signature of everything that calls this.
+        """
+        raise NotImplementedError
+
+    def describe(self) -> str:
+        return self.name
+
+
+class NoAuth(AuthProvider):
+    """Current behaviour, kept as an explicit choice rather than an absence.
+
+    Naming it matters. "No authentication" as a configured provider appears in
+    the settings, can be logged, and can be seen to be wrong in a deployment
+    review. The same state as an unconfigured service, but visible.
+    """
+
+    name = "none"
+
+    def authenticate(self, request: Request) -> Optional[str]:
+        return None
+
+
+class BearerAuth(AuthProvider):
+    """A single shared token, presented as `Authorization: Bearer <token>`.
+
+    A shared secret is not identity, and this does not pretend to be: every
+    holder is the same caller. It is the smallest thing that stops an open
+    endpoint being open, and the step before F3.
+    """
+
+    name = "bearer"
+
+    def __init__(self, token: str = None):
+        token = AUTH_TOKEN if token is None else token
+        # Fail at startup, not on the first request. A deployment that asked for
+        # bearer and supplied no token would otherwise start, look configured,
+        # and refuse every request -- or worse, if this compared against an empty
+        # string, admit anyone who sent an empty one.
+        if not token or not token.strip():
+            raise ValueError(
+                "BIOCHEF_AUTH=bearer needs BIOCHEF_AUTH_TOKEN set to a "
+                "non-empty value. Refusing to start rather than run with a "
+                "token nobody has to guess."
+            )
+        self._token = token
+
+    def authenticate(self, request: Request) -> Optional[str]:
+        header = request.headers.get("authorization")
+        if not header:
+            raise Unauthenticated("no credentials were presented")
+
+        scheme, _, presented = header.partition(" ")
+        if scheme.lower() != "bearer" or not presented:
+            raise Unauthenticated("expected an Authorization: Bearer <token>")
+
+        # compare_digest, not ==. String comparison returns as soon as it finds a
+        # difference, so how long it takes leaks how much of the token was right,
+        # and a token can be recovered a character at a time.
+        if not hmac.compare_digest(presented, self._token):
+            raise Unauthenticated("the token presented is not the one configured")
+
+        return "bearer-token"
+
+
+class AuthenticationMiddleware:
+    """Refuse before the body is read, not after.
+
+    A route dependency would be the obvious place, and it is the wrong one:
+    starlette parses and spools the whole multipart payload before the endpoint
+    is entered, so an anonymous caller would still have uploaded up to
+    BIOCHEF_MAX_UPLOAD_BYTES before anything asked who they were. The same
+    reason bodylimit.py is middleware.
+
+    Headers are in the ASGI scope from the start, so this costs nothing and can
+    answer before a single byte of body is accepted. It must therefore sit
+    OUTSIDE the body limit -- added last, since starlette makes the last-added
+    middleware outermost.
+    """
+
+    def __init__(self, app, provider: AuthProvider):
+        self.app = app
+        self.provider = provider
+
+    async def __call__(self, scope, receive, send):
+        if scope["type"] != "http":
+            await self.app(scope, receive, send)
+            return
+
+        try:
+            self.provider.authenticate(Request(scope))
+        except HTTPException as refusal:
+            body = json.dumps({"detail": refusal.detail}).encode()
+            headers = [(b"content-type", b"application/json"),
+                       (b"content-length", str(len(body)).encode())]
+            for key, value in (refusal.headers or {}).items():
+                headers.append((key.encode().lower(), value.encode()))
+            await send({"type": "http.response.start",
+                        "status": refusal.status_code, "headers": headers})
+            await send({"type": "http.response.body", "body": body})
+            return
+
+        await self.app(scope, receive, send)
+
+
+PROVIDERS = {
+    NoAuth.name: NoAuth,
+    BearerAuth.name: BearerAuth,
+}
+
+
+def get_auth(name: str) -> AuthProvider:
+    """Resolve a provider by name, refusing to start on one that is not there.
+
+    Deliberately the same shape as runner.get_runner. A name that is not a
+    provider must stop the process: silently falling back would mean a typo in
+    BIOCHEF_AUTH turns an authenticated deployment into an open one, which is
+    the single worst way for this setting to fail.
+    """
+    try:
+        provider = PROVIDERS[name]
+    except KeyError:
+        raise ValueError(
+            f"BIOCHEF_AUTH={name!r} is not an authentication provider. "
+            f"Available: {', '.join(sorted(PROVIDERS))}."
+        ) from None
+    return provider()

example.env

@@ -26,3 +26,9 @@ BIOCHEF_RUN_TIMEOUT=900
 BIOCHEF_KEEP_WORKSPACE=false
 BIOCHEF_MAX_UPLOAD_BYTES=536870912
 BIOCHEF_TOOL_CACHE=tool-cache
+
+# Who may call this service: none (anyone who can reach it) or bearer.
+# With bearer, requests need Authorization: Bearer <BIOCHEF_AUTH_TOKEN>.
+# Selecting bearer without a token stops the service from starting.
+BIOCHEF_AUTH=none
+BIOCHEF_AUTH_TOKEN=

main.py

@@ -11,6 +11,7 @@
 import base64
 
 from workspace import UnsafeName, check_name, make_workspace
+from auth import AuthenticationMiddleware, NoAuth, get_auth
 from bodylimit import BodySizeLimitMiddleware, MAX_UPLOAD_BYTES
 from runner import SubprocessRunner, get_runner
 
@@ -21,6 +22,18 @@
 # refusing bytes that are already on disk (#11).
 app.add_middleware(BodySizeLimitMiddleware)
 
+AUTH = get_auth(os.getenv("BIOCHEF_AUTH", NoAuth.name))
+"""Who may ask this service to run something.
+
+Resolved at import so a deployment naming a provider it does not have, or asking
+for bearer without a token, fails to start rather than accepting work.
+"""
+
+# Added last, so it is OUTERMOST and runs before the body limit -- and therefore
+# before any of the body is accepted. An anonymous caller should not be able to
+# make this service buffer half a gigabyte before being told no (#10).
+app.add_middleware(AuthenticationMiddleware, provider=AUTH)
+
 
 @app.exception_handler(UnsafeName)
 async def unusable_name(request, exc):