Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 67 additions & 1 deletion cmd/server/dav_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
// /<user>/<collection> 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.
Expand Down
9 changes: 8 additions & 1 deletion internal/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,21 @@ 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) {
user := devUser
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
}
Expand Down
Loading