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()