From 9d7043a594e73516135b68fe8d72ddb35d7bc0bd Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Thu, 26 Mar 2026 23:02:22 -0700 Subject: [PATCH] Block guest accounts from web UI login The web UI now validates the MCIAS token after login and rejects accounts with the guest role before setting the session cookie. This is defense-in-depth alongside the env:restricted MCIAS tag. The webserver.New() constructor takes a new ValidateFunc parameter that inspects token roles post-authentication. MCIAS login does not return roles, so this requires an extra ValidateToken round-trip at login time (result is cached for 30s). Security: guest role accounts are denied web UI access Co-Authored-By: Claude Opus 4.6 (1M context) --- ARCHITECTURE.md | 6 ++- PROGRESS.md | 24 ++++++++++ RUNBOOK.md | 3 +- cmd/mcr-web/main.go | 10 ++++- internal/webserver/auth.go | 26 +++++++++++ internal/webserver/server.go | 36 ++++++++------- internal/webserver/server_test.go | 73 +++++++++++++++++++++++++++++++ 7 files changed, 160 insertions(+), 18 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ebd8529..e662b4f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -881,6 +881,10 @@ separate binary (`mcr-web`) that communicates with mcrsrv via gRPC. - CSRF protection via signed double-submit cookies on all mutating requests. - Session cookie: `HttpOnly`, `Secure`, `SameSite=Strict`. - All user input escaped by `html/template`. +- Guest accounts are blocked at login. After MCIAS authentication succeeds, + the web UI validates the token and checks roles; accounts with the `guest` + role are denied access. This is defense-in-depth alongside the + `env:restricted` MCIAS tag. --- @@ -1059,7 +1063,7 @@ The audit log is append-only. It never contains credentials or token values. | Threat | Mitigation | |--------|------------| -| Unauthenticated access | All endpoints require MCIAS bearer token; `env:restricted` tag blocks guest/viewer login | +| Unauthenticated access | All endpoints require MCIAS bearer token; `env:restricted` tag blocks guest/viewer login; web UI additionally rejects `guest` role at login | | Unauthorized push/delete | Policy engine enforces per-principal, per-repository access; default-deny for system accounts | | Digest mismatch (supply chain) | All uploads verified against client-supplied digest; rejected if mismatch | | Blob corruption | Content-addressed storage; digests verified on write. Periodic integrity scrub via `mcrctl scrub` (future) | diff --git a/PROGRESS.md b/PROGRESS.md index 7f3d21d..689f4ae 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -32,6 +32,30 @@ See `PROJECT_PLAN.md` for the implementation roadmap and ### Next Steps 1. Deploy to rift (issue MCR service token, generate TLS cert, update mc-proxy routes) +2. Consider adding roles to MCIAS login response to eliminate the extra ValidateToken round-trip + +### 2026-03-26 — Web UI: block guest login + +**Task:** Prevent MCIAS guest accounts from logging into the web UI. + +**Changes:** + +- `internal/webserver/server.go`: Added `ValidateFunc` type; `New()` + accepts a validate function to inspect tokens post-login. +- `internal/webserver/auth.go`: After `loginFn` succeeds, calls + `validateFn` to retrieve roles. Rejects accounts with the `guest` + role before setting the session cookie. +- `cmd/mcr-web/main.go`: Wires `ValidateFunc` via `authClient.ValidateToken()`. +- `internal/webserver/server_test.go`: Added guest/user test accounts, + `validateFn` returning role-appropriate responses, `TestLoginDeniesGuest`. +- `ARCHITECTURE.md`: Updated Web UI security section and threat mitigations + to document guest blocking as defense-in-depth. + +**Design note:** MCIAS `/v1/auth/login` does not return roles, so the +web UI makes a second `ValidateToken` call after login to inspect them. +This is an extra MCIAS round-trip at login time (cached for 30s). A +future MCIAS change to include roles in the login response would +eliminate this. ### 2026-03-25 — Phase 13: Deployment Artifacts diff --git a/RUNBOOK.md b/RUNBOOK.md index 541aba6..d6868e4 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -15,7 +15,8 @@ MCR runs as two containers: token-based authentication via MCIAS. - **mcr-web** -- the web UI. Communicates with mcr-api via gRPC on port 9443. Provides repository/tag browsing and ACL policy management for - administrators. Listens on port 8080. + administrators. Listens on port 8080. Guest accounts are blocked at + login; only `admin` and `user` roles can access the web interface. Both are fronted by MC-Proxy for TLS routing. Metadata is stored in SQLite; blobs are stored as content-addressed files on the filesystem diff --git a/cmd/mcr-web/main.go b/cmd/mcr-web/main.go index 4e5970d..bdc92bf 100644 --- a/cmd/mcr-web/main.go +++ b/cmd/mcr-web/main.go @@ -99,6 +99,14 @@ func runServer(configPath string) error { return authClient.Login(username, password) } + validateFn := func(token string) ([]string, error) { + claims, err := authClient.ValidateToken(token) + if err != nil { + return nil, err + } + return claims.Roles, nil + } + // Generate CSRF key. csrfKey := make([]byte, 32) if _, err := rand.Read(csrfKey); err != nil { @@ -106,7 +114,7 @@ func runServer(configPath string) error { } // Create web server. - srv, err := webserver.New(registryClient, policyClient, auditClient, adminClient, loginFn, csrfKey) + srv, err := webserver.New(registryClient, policyClient, auditClient, adminClient, loginFn, validateFn, csrfKey) if err != nil { return fmt.Errorf("create web server: %w", err) } diff --git a/internal/webserver/auth.go b/internal/webserver/auth.go index 8b8f00e..a5acf47 100644 --- a/internal/webserver/auth.go +++ b/internal/webserver/auth.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "log" "net/http" + "slices" "strings" ) @@ -93,6 +94,31 @@ func (s *Server) handleLoginSubmit(w http.ResponseWriter, r *http.Request) { return } + // Validate the token to check roles. Guest accounts are not + // permitted to use the web interface. + roles, err := s.validateFn(token) + if err != nil { + log.Printf("login token validation failed for user %q: %v", username, err) + csrf := s.generateCSRFToken(w) + s.templates.render(w, "login", map[string]any{ + "Error": "Login failed. Please try again.", + "CSRFToken": csrf, + "Session": false, + }) + return + } + + if slices.Contains(roles, "guest") { + log.Printf("login denied for guest user %q", username) + csrf := s.generateCSRFToken(w) + s.templates.render(w, "login", map[string]any{ + "Error": "Guest accounts are not permitted to access the web interface.", + "CSRFToken": csrf, + "Session": false, + }) + return + } + http.SetCookie(w, &http.Cookie{ Name: "mcr_session", Value: token, diff --git a/internal/webserver/server.go b/internal/webserver/server.go index f24c9b6..1f49957 100644 --- a/internal/webserver/server.go +++ b/internal/webserver/server.go @@ -21,16 +21,20 @@ import ( // LoginFunc authenticates a user and returns a bearer token. type LoginFunc func(username, password string) (token string, expiresIn int, err error) +// ValidateFunc validates a bearer token and returns the user's roles. +type ValidateFunc func(token string) (roles []string, err error) + // Server is the MCR web UI server. type Server struct { - router chi.Router - templates *templateSet - registry mcrv1.RegistryServiceClient - policy mcrv1.PolicyServiceClient - audit mcrv1.AuditServiceClient - admin mcrv1.AdminServiceClient - loginFn LoginFunc - csrfKey []byte // 32-byte key for HMAC signing + router chi.Router + templates *templateSet + registry mcrv1.RegistryServiceClient + policy mcrv1.PolicyServiceClient + audit mcrv1.AuditServiceClient + admin mcrv1.AdminServiceClient + loginFn LoginFunc + validateFn ValidateFunc + csrfKey []byte // 32-byte key for HMAC signing } // New creates a new web UI server with the given gRPC clients and login function. @@ -40,6 +44,7 @@ func New( audit mcrv1.AuditServiceClient, admin mcrv1.AdminServiceClient, loginFn LoginFunc, + validateFn ValidateFunc, csrfKey []byte, ) (*Server, error) { tmpl, err := loadTemplates() @@ -48,13 +53,14 @@ func New( } s := &Server{ - templates: tmpl, - registry: registry, - policy: policy, - audit: audit, - admin: admin, - loginFn: loginFn, - csrfKey: csrfKey, + templates: tmpl, + registry: registry, + policy: policy, + audit: audit, + admin: admin, + loginFn: loginFn, + validateFn: validateFn, + csrfKey: csrfKey, } s.router = s.buildRouter() diff --git a/internal/webserver/server_test.go b/internal/webserver/server_test.go index 678e478..d6b94a5 100644 --- a/internal/webserver/server_test.go +++ b/internal/webserver/server_test.go @@ -183,15 +183,35 @@ func setupTestEnv(t *testing.T) *testEnv { if username == "admin" && password == "secret" { return "test-token-12345", 3600, nil } + if username == "guest" && password == "secret" { + return "test-token-guest", 3600, nil + } + if username == "user" && password == "secret" { + return "test-token-user", 3600, nil + } return "", 0, fmt.Errorf("invalid credentials") } + validateFn := func(token string) ([]string, error) { + switch token { + case "test-token-12345": + return []string{"admin"}, nil + case "test-token-guest": + return []string{"guest"}, nil + case "test-token-user": + return []string{"user"}, nil + default: + return nil, fmt.Errorf("invalid token") + } + } + srv, err := New( mcrv1.NewRegistryServiceClient(conn), mcrv1.NewPolicyServiceClient(conn), mcrv1.NewAuditServiceClient(conn), mcrv1.NewAdminServiceClient(conn), loginFn, + validateFn, csrfKey, ) if err != nil { @@ -543,6 +563,59 @@ func TestTruncate(t *testing.T) { } } +func TestLoginDeniesGuest(t *testing.T) { + env := setupTestEnv(t) + defer env.close() + + // Get CSRF token. + getReq := httptest.NewRequest(http.MethodGet, "/login", nil) + getRec := httptest.NewRecorder() + env.server.Handler().ServeHTTP(getRec, getReq) + + var csrfCookie *http.Cookie + for _, c := range getRec.Result().Cookies() { + if c.Name == "csrf_token" { + csrfCookie = c + break + } + } + if csrfCookie == nil { + t.Fatal("no csrf_token cookie") + } + + parts := strings.SplitN(csrfCookie.Value, ".", 2) + csrfToken := parts[0] + + form := url.Values{ + "username": {"guest"}, + "password": {"secret"}, + "_csrf": {csrfToken}, + } + + postReq := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form.Encode())) + postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") + postReq.AddCookie(csrfCookie) + postRec := httptest.NewRecorder() + + env.server.Handler().ServeHTTP(postRec, postReq) + + if postRec.Code != http.StatusOK { + t.Fatalf("POST /login as guest: status %d, want %d", postRec.Code, http.StatusOK) + } + + body := postRec.Body.String() + if !strings.Contains(body, "Guest accounts are not permitted") { + t.Error("response does not contain guest denial message") + } + + // Verify no session cookie was set. + for _, c := range postRec.Result().Cookies() { + if c.Name == "mcr_session" { + t.Error("session cookie should not be set for guest login") + } + } +} + func TestLoginSuccessSetsCookie(t *testing.T) { env := setupTestEnv(t) defer env.close()