diff --git a/cmd/server/dav_integration_test.go b/cmd/server/dav_integration_test.go index f5b0e9a..bc63ff1 100644 --- a/cmd/server/dav_integration_test.go +++ b/cmd/server/dav_integration_test.go @@ -327,7 +327,7 @@ func TestWellKnownRedirects(t *testing.T) { if err != nil { t.Fatal(err) } - resp.Body.Close() + _ = resp.Body.Close() if resp.StatusCode != http.StatusMovedPermanently { t.Errorf("GET %s = %d, want 301", tc.path, resp.StatusCode) } @@ -337,6 +337,72 @@ func TestWellKnownRedirects(t *testing.T) { } } +// TestWellKnownRedirectsInDavRootMode covers issue #47: discovery must keep +// working in the mode the OpenCloud proxy uses, where the DAV handler owns +// every path and would otherwise claim the well-known paths as a collection. +func TestWellKnownRedirectsInDavRootMode(t *testing.T) { + dir := t.TempDir() + st := store.New(dir) + srv := httptest.NewServer(newHandler(st, filepath.Join(dir, "missing-ui"), "alice", true, defaultLimits())) + t.Cleanup(srv.Close) + + client := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }} + for _, tc := range []struct{ path, want string }{ + {"/.well-known/caldav", "/caldav/alice/"}, + {"/.well-known/carddav", "/carddav/alice/"}, + } { + resp, err := client.Get(srv.URL + tc.path) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusMovedPermanently { + t.Errorf("GET %s = %d, want 301", tc.path, resp.StatusCode) + } + if got := resp.Header.Get("Location"); got != tc.want { + t.Errorf("GET %s redirected to %q, want %q", tc.path, got, tc.want) + } + } + + // The redirect target must still be served by the catch-all DAV handler. + resp, body := do(t, "PROPFIND", srv.URL+"/caldav/alice/", "", map[string]string{"Depth": "0"}) + if resp.StatusCode != http.StatusMultiStatus { + t.Fatalf("PROPFIND /caldav/alice/ in dav-root mode = %d, want 207: %s", resp.StatusCode, body) + } + if !strings.Contains(body, "current-user-principal") { + t.Errorf("PROPFIND body has no current-user-principal: %s", body) + } +} + +// TestOptionsNeedsNoIdentity covers issue #48: the OpenCloud proxy does not +// authenticate OPTIONS, so capability probing must work without a user while +// every method that touches data keeps requiring one. +func TestOptionsNeedsNoIdentity(t *testing.T) { + dir := t.TempDir() + srv := httptest.NewServer(newHandler(store.New(dir), filepath.Join(dir, "missing-ui"), "", true, defaultLimits())) + t.Cleanup(srv.Close) + + resp, body := do(t, "OPTIONS", srv.URL+"/caldav/", "", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("OPTIONS without identity = %d, want 200: %s", resp.StatusCode, body) + } + if dav := resp.Header.Get("DAV"); !strings.Contains(dav, "calendar-access") { + t.Errorf("DAV header = %q, want calendar-access", dav) + } + if allow := resp.Header.Get("Allow"); !strings.Contains(allow, "REPORT") || !strings.Contains(allow, "PROPFIND") { + t.Errorf("Allow header = %q, want the DAV method list", allow) + } + + for _, method := range []string{"GET", "PROPFIND", "REPORT", "PUT", "DELETE"} { + resp, body := do(t, method, srv.URL+"/caldav/alice/", "", map[string]string{"Depth": "0"}) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("%s without identity = %d, want 401: %s", method, resp.StatusCode, body) + } + } +} + // TestPutGetAreByteIdentical is the core of issue #6: the file on disk must // be exactly the request body (like Radicale, which stores the item as sent) // and GET must serve exactly those bytes. That is what keeps the data volume diff --git a/cmd/server/main.go b/cmd/server/main.go index f49578d..a484280 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -96,6 +96,14 @@ func newHandler(st *store.Store, uiDir, devUser string, davRoot bool, lim limits mux := http.NewServeMux() if davRoot { + // RFC 6764 discovery does not depend on how the DAV endpoints are + // mounted. The OpenCloud proxy forwards /.well-known/caldav and + // /.well-known/carddav here with X-Remote-User already resolved, and a + // client that is only given a hostname asks these paths before any + // other. Without them the catch-all below treats the path as + // // and rejects it as another user's data. + mux.Handle("/.well-known/caldav", mw(wellKnownHandler("/caldav"))) + mux.Handle("/.well-known/carddav", mw(wellKnownHandler("/carddav"))) // Behind the OpenCloud proxy every path is DAV and the only signal // telling calendars from address books is X-Script-Name, which the // proxy sets to /caldav or /carddav. diff --git a/internal/auth/auth.go b/internal/auth/auth.go index a6088ba..b23fd10 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -24,6 +24,13 @@ func User(ctx context.Context) string { // Middleware extracts the identity. In dev mode (devUser != "") the header is // ignored and the given user is forced. +// +// OPTIONS is the one exception: it is capability discovery, answered with the +// compliance headers and an Allow list that are identical for every user and +// touch no data, while some proxies (OpenCloud's) do not authenticate that +// method and therefore never set X-Remote-User. Rejecting it made every +// capability probe look like an authentication failure. Every method that +// reads or writes data still requires the identity. func Middleware(devUser string) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -31,7 +38,7 @@ func Middleware(devUser string) func(http.Handler) http.Handler { if user == "" { user = r.Header.Get("X-Remote-User") } - if user == "" { + if user == "" && r.Method != http.MethodOptions { http.Error(w, "davcloud: missing identity (OpenCloud proxy must set X-Remote-User)", http.StatusUnauthorized) return }