X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/2c7b49e8791154db907616243c930c552aeb3df3..0b714d15c15340f3f52c331528d5cbb4cc422f07:/services/keep-web/handler.go diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go index fab004c89f..4ea2fa2f6d 100644 --- a/services/keep-web/handler.go +++ b/services/keep-web/handler.go @@ -6,7 +6,6 @@ package main import ( "encoding/json" - "fmt" "html" "html/template" "io" @@ -19,13 +18,14 @@ import ( "strings" "sync" - "git.curoverse.com/arvados.git/sdk/go/arvados" - "git.curoverse.com/arvados.git/sdk/go/arvadosclient" - "git.curoverse.com/arvados.git/sdk/go/auth" - "git.curoverse.com/arvados.git/sdk/go/health" - "git.curoverse.com/arvados.git/sdk/go/httpserver" - "git.curoverse.com/arvados.git/sdk/go/keepclient" - log "github.com/sirupsen/logrus" + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/arvadosclient" + "git.arvados.org/arvados.git/sdk/go/auth" + "git.arvados.org/arvados.git/sdk/go/ctxlog" + "git.arvados.org/arvados.git/sdk/go/health" + "git.arvados.org/arvados.git/sdk/go/httpserver" + "git.arvados.org/arvados.git/sdk/go/keepclient" + "github.com/sirupsen/logrus" "golang.org/x/net/webdav" ) @@ -62,6 +62,9 @@ func parseCollectionIDFromDNSName(s string) string { var urlPDHDecoder = strings.NewReplacer(" ", "+", "-", "+") +var notFoundMessage = "404 Not found\r\n\r\nThe requested path was not found, or you do not have permission to access it.\r" +var unauthorizedMessage = "401 Unauthorized\r\n\r\nA valid Arvados token must be provided to access this resource.\r" + // parseCollectionIDFromURL returns a UUID or PDH if s is a UUID or a // PDH (even if it is a PDH with "+" replaced by " " or "-"); // otherwise "". @@ -76,9 +79,12 @@ func parseCollectionIDFromURL(s string) string { } func (h *handler) setup() { - h.clientPool = arvadosclient.MakeClientPool() + // Errors will be handled at the client pool. + arv, _ := arvados.NewClientFromConfig(h.Config.cluster) + h.clientPool = arvadosclient.MakeClientPoolWith(arv) keepclient.RefreshServiceDiscoveryOnSIGHUP() + keepclient.DefaultBlockCache.MaxBlocks = h.Config.cluster.Collections.WebDAVCache.MaxBlockEntries h.healthHandler = &health.Handler{ Token: h.Config.cluster.ManagementToken, @@ -101,6 +107,7 @@ func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) { // are ignored (all response writes return the update error). type updateOnSuccess struct { httpserver.ResponseWriter + logger logrus.FieldLogger update func() error sentHeader bool err error @@ -125,7 +132,7 @@ func (uos *updateOnSuccess) WriteHeader(code int) { if err, ok := uos.err.(*arvados.TransactionError); ok { code = err.StatusCode } - log.Printf("update() changes response to HTTP %d: %T %q", code, uos.err, uos.err) + uos.logger.WithError(uos.err).Errorf("update() returned error type %T, changing response to HTTP %d", uos.err, code) http.Error(uos.ResponseWriter, uos.err.Error(), code) return } @@ -181,31 +188,11 @@ var ( func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { h.setupOnce.Do(h.setup) - var statusCode = 0 - var statusText string - - remoteAddr := r.RemoteAddr - if xff := r.Header.Get("X-Forwarded-For"); xff != "" { - remoteAddr = xff + "," + remoteAddr - } if xfp := r.Header.Get("X-Forwarded-Proto"); xfp != "" && xfp != "http" { r.URL.Scheme = xfp } w := httpserver.WrapResponseWriter(wOrig) - defer func() { - if statusCode == 0 { - statusCode = w.WroteStatus() - } else if w.WroteStatus() == 0 { - w.WriteHeader(statusCode) - } else if w.WroteStatus() != statusCode { - log.WithField("RequestID", r.Header.Get("X-Request-Id")).Warn( - fmt.Sprintf("Our status changed from %d to %d after we sent headers", w.WroteStatus(), statusCode)) - } - if statusText == "" { - statusText = http.StatusText(statusCode) - } - }() if strings.HasPrefix(r.URL.Path, "/_health/") && r.Method == "GET" { h.healthHandler.ServeHTTP(w, r) @@ -214,19 +201,18 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { if method := r.Header.Get("Access-Control-Request-Method"); method != "" && r.Method == "OPTIONS" { if !browserMethod[method] && !webdavMethod[method] { - statusCode = http.StatusMethodNotAllowed + w.WriteHeader(http.StatusMethodNotAllowed) return } w.Header().Set("Access-Control-Allow-Headers", corsAllowHeadersHeader) w.Header().Set("Access-Control-Allow-Methods", "COPY, DELETE, GET, LOCK, MKCOL, MOVE, OPTIONS, POST, PROPFIND, PROPPATCH, PUT, RMCOL, UNLOCK") w.Header().Set("Access-Control-Allow-Origin", "*") w.Header().Set("Access-Control-Max-Age", "86400") - statusCode = http.StatusOK return } if !browserMethod[r.Method] && !webdavMethod[r.Method] { - statusCode, statusText = http.StatusMethodNotAllowed, r.Method + w.WriteHeader(http.StatusMethodNotAllowed) return } @@ -240,6 +226,10 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { w.Header().Set("Access-Control-Expose-Headers", "Content-Range") } + if h.serveS3(w, r) { + return + } + pathParts := strings.Split(r.URL.Path[1:], "/") var stripParts int @@ -281,16 +271,18 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { stripParts = 4 pathToken = true } else { - log.Info(" !!!! ATTN: Into /collections/uuid/path with anon token: ", h.Config.cluster.Users.AnonymousUserToken) // /collections/ID/PATH... collectionID = parseCollectionIDFromURL(pathParts[1]) - tokens = []string{h.Config.cluster.Users.AnonymousUserToken} stripParts = 2 + // This path is only meant to work for public + // data. Tokens provided with the request are + // ignored. + credentialsOK = false } } if collectionID == "" && !useSiteFS { - statusCode = http.StatusNotFound + http.Error(w, notFoundMessage, http.StatusNotFound) return } @@ -299,37 +291,43 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { forceReload = true } + if credentialsOK { + reqTokens = auth.CredentialsFromRequest(r).Tokens + } + formToken := r.FormValue("api_token") - if formToken != "" && r.Header.Get("Origin") != "" && attachment && r.URL.Query().Get("api_token") == "" { - // The client provided an explicit token in the POST - // body. The Origin header indicates this *might* be - // an AJAX request, in which case redirect-with-cookie - // won't work: we should just serve the content in the - // POST response. This is safe because: + origin := r.Header.Get("Origin") + cors := origin != "" && !strings.HasSuffix(origin, "://"+r.Host) + safeAjax := cors && (r.Method == http.MethodGet || r.Method == http.MethodHead) + safeAttachment := attachment && r.URL.Query().Get("api_token") == "" + if formToken == "" { + // No token to use or redact. + } else if safeAjax || safeAttachment { + // If this is a cross-origin request, the URL won't + // appear in the browser's address bar, so + // substituting a clipboard-safe URL is pointless. + // Redirect-with-cookie wouldn't work anyway, because + // it's not safe to allow third-party use of our + // cookie. // - // * We're supplying an attachment, not inline - // content, so we don't need to convert the POST to - // a GET and avoid the "really resubmit form?" - // problem. - // - // * The token isn't embedded in the URL, so we don't - // need to worry about bookmarks and copy/paste. - tokens = append(tokens, formToken) - } else if formToken != "" && browserMethod[r.Method] { - // The client provided an explicit token in the query - // string, or a form in POST body. We must put the - // token in an HttpOnly cookie, and redirect to the - // same URL with the query param redacted and method = - // GET. + // If we're supplying an attachment, we don't need to + // convert POST to GET to avoid the "really resubmit + // form?" problem, so provided the token isn't + // embedded in the URL, there's no reason to do + // redirect-with-cookie in this case either. + reqTokens = append(reqTokens, formToken) + } else if browserMethod[r.Method] { + // If this is a page view, and the client provided a + // token via query string or POST body, we must put + // the token in an HttpOnly cookie, and redirect to an + // equivalent URL with the query param redacted and + // method = GET. h.seeOtherWithCookie(w, r, "", credentialsOK) return } if useSiteFS { - if tokens == nil { - tokens = auth.CredentialsFromRequest(r).Tokens - } - h.serveSiteFS(w, r, tokens, credentialsOK, attachment) + h.serveSiteFS(w, r, reqTokens, credentialsOK, attachment) return } @@ -348,9 +346,6 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { } if tokens == nil { - if credentialsOK { - reqTokens = auth.CredentialsFromRequest(r).Tokens - } tokens = append(reqTokens, h.Config.cluster.Users.AnonymousUserToken) } @@ -367,7 +362,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { arv := h.clientPool.Get() if arv == nil { - statusCode, statusText = http.StatusInternalServerError, "Pool failed: "+h.clientPool.Err().Error() + http.Error(w, "client pool error: "+h.clientPool.Err().Error(), http.StatusInternalServerError) return } defer h.clientPool.Put(arv) @@ -391,7 +386,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { } } // Something more serious is wrong - statusCode, statusText = http.StatusInternalServerError, err.Error() + http.Error(w, "cache error: "+err.Error(), http.StatusInternalServerError) return } if collection == nil { @@ -401,14 +396,14 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { // for additional credentials would just be // confusing), or we don't even accept // credentials at this path. - statusCode = http.StatusNotFound + http.Error(w, notFoundMessage, http.StatusNotFound) return } for _, t := range reqTokens { if tokenResult[t] == 404 { // The client provided valid token(s), but the // collection was not found. - statusCode = http.StatusNotFound + http.Error(w, notFoundMessage, http.StatusNotFound) return } } @@ -422,13 +417,13 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { // data that has been deleted. Allow a referrer to // provide this context somehow? w.Header().Add("WWW-Authenticate", "Basic realm=\"collections\"") - statusCode = http.StatusUnauthorized + http.Error(w, unauthorizedMessage, http.StatusUnauthorized) return } kc, err := keepclient.MakeKeepClient(arv) if err != nil { - statusCode, statusText = http.StatusInternalServerError, err.Error() + http.Error(w, "error setting up keep client: "+err.Error(), http.StatusInternalServerError) return } kc.RequestID = r.Header.Get("X-Request-Id") @@ -447,14 +442,14 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { fs, err := collection.FileSystem(client, kc) if err != nil { - statusCode, statusText = http.StatusInternalServerError, err.Error() + http.Error(w, "error creating collection filesystem: "+err.Error(), http.StatusInternalServerError) return } writefs, writeOK := fs.(arvados.CollectionFileSystem) targetIsPDH := arvadosclient.PDHMatch(collectionID) if (targetIsPDH || !writeOK) && writeMethod[r.Method] { - statusCode, statusText = http.StatusMethodNotAllowed, errReadOnly.Error() + http.Error(w, errReadOnly.Error(), http.StatusMethodNotAllowed) return } @@ -466,6 +461,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { // collection can't be saved. w = &updateOnSuccess{ ResponseWriter: w, + logger: ctxlog.FromContext(r.Context()), update: func() error { return h.Config.Cache.Update(client, *collection, writefs) }} @@ -480,7 +476,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { LockSystem: h.webdavLS, Logger: func(_ *http.Request, err error) { if err != nil { - log.Printf("error from webdav handler: %q", err) + ctxlog.FromContext(r.Context()).WithError(err).Error("error reported by webdav handler") } }, } @@ -491,13 +487,13 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { openPath := "/" + strings.Join(targetPath, "/") if f, err := fs.Open(openPath); os.IsNotExist(err) { // Requested non-existent path - statusCode = http.StatusNotFound + http.Error(w, notFoundMessage, http.StatusNotFound) } else if err != nil { // Some other (unexpected) error - statusCode, statusText = http.StatusInternalServerError, err.Error() + http.Error(w, "open: "+err.Error(), http.StatusInternalServerError) } else if stat, err := f.Stat(); err != nil { // Can't get Size/IsDir (shouldn't happen with a collectionFS!) - statusCode, statusText = http.StatusInternalServerError, err.Error() + http.Error(w, "stat: "+err.Error(), http.StatusInternalServerError) } else if stat.IsDir() && !strings.HasSuffix(r.URL.Path, "/") { // If client requests ".../dirname", redirect to // ".../dirname/". This way, relative links in the @@ -508,49 +504,57 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { h.serveDirectory(w, r, collection.Name, fs, openPath, true) } else { http.ServeContent(w, r, basename, stat.ModTime(), f) - if r.Header.Get("Range") == "" && int64(w.WroteBodyBytes()) != stat.Size() { + if wrote := int64(w.WroteBodyBytes()); wrote != stat.Size() && r.Header.Get("Range") == "" { // If we wrote fewer bytes than expected, it's // too late to change the real response code // or send an error message to the client, but // at least we can try to put some useful // debugging info in the logs. n, err := f.Read(make([]byte, 1024)) - statusCode, statusText = http.StatusInternalServerError, fmt.Sprintf("f.Size()==%d but only wrote %d bytes; read(1024) returns %d, %s", stat.Size(), w.WroteBodyBytes(), n, err) + ctxlog.FromContext(r.Context()).Errorf("stat.Size()==%d but only wrote %d bytes; read(1024) returns %d, %s", stat.Size(), wrote, n, err) } } } +func (h *handler) getClients(reqID, token string) (arv *arvadosclient.ArvadosClient, kc *keepclient.KeepClient, client *arvados.Client, release func(), err error) { + arv = h.clientPool.Get() + if arv == nil { + err = h.clientPool.Err() + return + } + release = func() { h.clientPool.Put(arv) } + arv.ApiToken = token + kc, err = keepclient.MakeKeepClient(arv) + if err != nil { + release() + return + } + kc.RequestID = reqID + client = (&arvados.Client{ + APIHost: arv.ApiServer, + AuthToken: arv.ApiToken, + Insecure: arv.ApiInsecure, + }).WithRequestID(reqID) + return +} + func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []string, credentialsOK, attachment bool) { if len(tokens) == 0 { w.Header().Add("WWW-Authenticate", "Basic realm=\"collections\"") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + http.Error(w, unauthorizedMessage, http.StatusUnauthorized) return } if writeMethod[r.Method] { http.Error(w, errReadOnly.Error(), http.StatusMethodNotAllowed) return } - arv := h.clientPool.Get() - if arv == nil { - http.Error(w, "Pool failed: "+h.clientPool.Err().Error(), http.StatusInternalServerError) - return - } - defer h.clientPool.Put(arv) - arv.ApiToken = tokens[0] - - kc, err := keepclient.MakeKeepClient(arv) + fs, err := h.Config.Cache.GetSession(tokens[0]) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - kc.RequestID = r.Header.Get("X-Request-Id") - client := (&arvados.Client{ - APIHost: arv.ApiServer, - AuthToken: arv.ApiToken, - Insecure: arv.ApiInsecure, - }).WithRequestID(r.Header.Get("X-Request-Id")) - fs := client.SiteFileSystem(kc) + fs.ForwardSlashNameSubstitution(h.Config.cluster.Collections.ForwardSlashNameSubstitution) f, err := fs.Open(r.URL.Path) if os.IsNotExist(err) { http.Error(w, err.Error(), http.StatusNotFound) @@ -582,7 +586,7 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s LockSystem: h.webdavLS, Logger: func(_ *http.Request, err error) { if err != nil { - log.Printf("error from webdav handler: %q", err) + ctxlog.FromContext(r.Context()).WithError(err).Error("error reported by webdav handler") } }, } @@ -696,7 +700,7 @@ func (h *handler) serveDirectory(w http.ResponseWriter, r *http.Request, collect return nil } if err := walk(""); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, "error getting directory listing: "+err.Error(), http.StatusInternalServerError) return } @@ -707,7 +711,7 @@ func (h *handler) serveDirectory(w http.ResponseWriter, r *http.Request, collect } tmpl, err := template.New("dir").Funcs(funcs).Parse(dirListingTemplate) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, "error parsing template: "+err.Error(), http.StatusInternalServerError) return } sort.Slice(files, func(i, j int) bool { @@ -747,7 +751,7 @@ func (h *handler) seeOtherWithCookie(w http.ResponseWriter, r *http.Request, loc // into a cookie unless the current vhost // (origin) serves only a single collection or // we are in TrustAllContent mode. - w.WriteHeader(http.StatusBadRequest) + http.Error(w, "cannot serve inline content at this URL (possible configuration error; see https://doc.arvados.org/install/install-keep-web.html#dns)", http.StatusBadRequest) return } @@ -767,6 +771,7 @@ func (h *handler) seeOtherWithCookie(w http.ResponseWriter, r *http.Request, loc Value: auth.EncodeTokenCookie([]byte(formToken)), Path: "/", HttpOnly: true, + SameSite: http.SameSiteLaxMode, }) } @@ -779,7 +784,7 @@ func (h *handler) seeOtherWithCookie(w http.ResponseWriter, r *http.Request, loc if location != "" { newu, err := u.Parse(location) if err != nil { - w.WriteHeader(http.StatusInternalServerError) + http.Error(w, "error resolving redirect target: "+err.Error(), http.StatusInternalServerError) return } u = newu