Merge pull request #2294 from owncloud/fix-trashbin-expected-failures

Fix Trashbin Expected Failures

ocs/pkg/server/http/svc_test.go

@@ -650,7 +650,7 @@ func mintToken(ctx context.Context, su *User, roleIds []string) (token string, e
 				},
 			},
 		},
-		Groups: []string{},
+		Groups:    []string{},
 		UidNumber: int64(su.UIDNumber),
 		GidNumber: int64(su.GIDNumber),
 	}

ocs/pkg/service/v0/users.go

@@ -486,9 +486,9 @@ func (o Ocs) mintTokenForUser(ctx context.Context, account *accounts.Account) (s
 			OpaqueId: account.Id,
 			Idp:      o.config.IdentityManagement.Address,
 		},
-		Groups: []string{},
-		UidNumber:                account.UidNumber,
-		GidNumber:                account.GidNumber,
+		Groups:    []string{},
+		UidNumber: account.UidNumber,
+		GidNumber: account.GidNumber,
 	}
 	s, err := scope.AddOwnerScope(nil)
 	if err != nil {

proxy/pkg/middleware/account_resolver.go

@@ -1,9 +1,10 @@
 package middleware
 
 import (
+	"net/http"
+
 	"github.com/cs3org/reva/pkg/auth/scope"
 	"github.com/owncloud/ocis/proxy/pkg/user/backend"
-	"net/http"
 
 	tokenPkg "github.com/cs3org/reva/pkg/token"
 	"github.com/cs3org/reva/pkg/token/manager/jwt"

proxy/pkg/middleware/basic_auth.go

@@ -2,11 +2,13 @@ package middleware
 
 import (
 	"fmt"
+	"net/http"
+	"strings"
+
 	"github.com/owncloud/ocis/ocis-pkg/log"
 	"github.com/owncloud/ocis/ocis-pkg/oidc"
 	"github.com/owncloud/ocis/proxy/pkg/user/backend"
-	"net/http"
-	"strings"
+	"github.com/owncloud/ocis/proxy/pkg/webdav"
 )
 
 const publicFilesEndpoint = "/remote.php/dav/public-files/"
@@ -61,7 +63,22 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler {
 						writeSupportedAuthenticateHeader(w, req)
 					}
 
+					// if the request is a PROPFIND return a WebDAV error code.
+					// TODO: The proxy has to be smart enough to detect when a request is directed towards a webdav server
+					// and react accordingly.
+
 					w.WriteHeader(http.StatusUnauthorized)
+
+					if webdav.IsWebdavRequest(req) {
+						b, err := webdav.Marshal(webdav.Exception{
+							Code:    webdav.SabredavPermissionDenied,
+							Message: "Authentication error",
+						})
+
+						webdav.HandleWebdavError(w, b, err)
+						return
+					}
+
 					return
 				}
 
@@ -90,6 +107,6 @@ func (m basicAuth) isPublicLink(req *http.Request) bool {
 }
 
 func (m basicAuth) isBasicAuth(req *http.Request) bool {
-	login, password, ok := req.BasicAuth()
-	return m.enabled && ok && login != "" && password != ""
+	_, _, ok := req.BasicAuth()
+	return m.enabled && ok
 }

proxy/pkg/webdav/response.go

@@ -0,0 +1,77 @@
+package webdav
+
+import (
+	"encoding/xml"
+	"net/http"
+)
+
+type code int
+
+const (
+	// SabredavBadRequest maps to HTTP 400
+	SabredavBadRequest code = iota
+	// SabredavMethodNotAllowed maps to HTTP 405
+	SabredavMethodNotAllowed
+	// SabredavNotAuthenticated maps to HTTP 401
+	SabredavNotAuthenticated
+	// SabredavPreconditionFailed maps to HTTP 412
+	SabredavPreconditionFailed
+	// SabredavPermissionDenied maps to HTTP 403
+	SabredavPermissionDenied
+	// SabredavNotFound maps to HTTP 404
+	SabredavNotFound
+	// SabredavConflict maps to HTTP 409
+	SabredavConflict
+)
+
+var (
+	codesEnum = []string{
+		"Sabre\\DAV\\Exception\\BadRequest",
+		"Sabre\\DAV\\Exception\\MethodNotAllowed",
+		"Sabre\\DAV\\Exception\\NotAuthenticated",
+		"Sabre\\DAV\\Exception\\PreconditionFailed",
+		"Sabre\\DAV\\Exception\\PermissionDenied",
+		"Sabre\\DAV\\Exception\\NotFound",
+		"Sabre\\DAV\\Exception\\Conflict",
+	}
+)
+
+type Exception struct {
+	Code    code
+	Message string
+	Header  string
+}
+
+// Marshal just calls the xml marshaller for a given Exception.
+func Marshal(e Exception) ([]byte, error) {
+	xmlstring, err := xml.Marshal(&errorXML{
+		Xmlnsd:    "DAV",
+		Xmlnss:    "http://sabredav.org/ns",
+		Exception: codesEnum[e.Code],
+		Message:   e.Message,
+		Header:    e.Header,
+	})
+	if err != nil {
+		return []byte(""), err
+	}
+	return []byte(xml.Header + string(xmlstring)), err
+}
+
+// http://www.webdav.org/specs/rfc4918.html#ELEMENT_error
+type errorXML struct {
+	XMLName   xml.Name `xml:"d:error"`
+	Xmlnsd    string   `xml:"xmlns:d,attr"`
+	Xmlnss    string   `xml:"xmlns:s,attr"`
+	Exception string   `xml:"s:Exception"`
+	Message   string   `xml:"s:Message"`
+	InnerXML  []byte   `xml:",innerxml"`
+	Header    string   `xml:"s:Header,omitempty"`
+}
+
+func HandleWebdavError(w http.ResponseWriter, b []byte, err error) {
+	if err != nil {
+		w.WriteHeader(http.StatusInternalServerError)
+		return
+	}
+	_, _ = w.Write(b)
+}

proxy/pkg/webdav/webdav.go

@@ -0,0 +1,18 @@
+package webdav
+
+import "net/http"
+
+var methods = []string{"PROPFIND", "DELETE", "PROPPATCH", "MKCOL", "COPY", "MOVE", "LOCK", "UNLOCK"}
+
+// This is a non exhaustive way to detect if a request is directed to a webdav server. This naïve implementation
+// only deals with the set of methods exclusive to WebDAV. Since WebDAV is a superset of HTTP, GET, POST and so on
+// are valid methods, but this implementation would require a larger effort than we can build upon in this file.
+// This is needed because the proxy might need to create a response with a webdav body; such as unauthorized.
+func IsWebdavRequest(r *http.Request) bool {
+	for i := range methods {
+		if methods[i] == r.Method {
+			return true
+		}
+	}
+	return false
+}

tests/acceptance/expected-failures-API-on-OCIS-storage.md

@@ -3,12 +3,6 @@
 ### File
 Basic file management like up and download, move, copy, properties, trash, versions and chunking.
 
-#### [invalid webdav responses for unauthorized requests.](https://github.com/owncloud/product/issues/273)
--   [apiTrashbin/trashbinFilesFolders.feature:215](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L215)
--   [apiTrashbin/trashbinFilesFolders.feature:216](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L216)
--   [apiTrashbin/trashbinFilesFolders.feature:230](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L230)
--   [apiTrashbin/trashbinFilesFolders.feature:231](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L231)
-
 #### [cannot restore to a different file-name](https://github.com/owncloud/ocis/issues/1122)
 -   [apiTrashbinRestore/trashbinRestore.feature:309](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbinRestore/trashbinRestore.feature#L309)
 -   [apiTrashbinRestore/trashbinRestore.feature:310](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbinRestore/trashbinRestore.feature#L310)

tests/acceptance/expected-failures-API-on-OWNCLOUD-storage.md

@@ -13,12 +13,6 @@ The following scenarios fail on OWNCLOUD storage but not on OCIS storage:
 -   [apiTrashbin/trashbinFilesFolders.feature:104](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L104)
 -   [apiTrashbin/trashbinFilesFolders.feature:105](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L105)
 
-#### [invalid webdav responses for unauthorized requests.](https://github.com/owncloud/product/issues/273)
--   [apiTrashbin/trashbinFilesFolders.feature:215](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L215)
--   [apiTrashbin/trashbinFilesFolders.feature:216](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L216)
--   [apiTrashbin/trashbinFilesFolders.feature:230](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L230)
--   [apiTrashbin/trashbinFilesFolders.feature:231](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L231)
-
 #### [PROPFIND on trashbin with Depth: infinity only shows the first level](https://github.com/owncloud/ocis/issues/1116)
 -   [apiTrashbin/trashbinFilesFolders.feature:278](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L278)
 -   [apiTrashbin/trashbinFilesFolders.feature:279](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L279)