diff --git a/ocs/pkg/server/http/svc_test.go b/ocs/pkg/server/http/svc_test.go index 9a284d0c118..428550352a3 100644 --- a/ocs/pkg/server/http/svc_test.go +++ b/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), } diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index 3cabff97731..eef8ed5354b 100644 --- a/ocs/pkg/service/v0/users.go +++ b/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.GetOwnerScope() if err != nil { diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go index 6aa829c49a3..f7379eda494 100644 --- a/proxy/pkg/middleware/account_resolver.go +++ b/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" diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index df710c6e4b1..1f8d0a807d4 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/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 } diff --git a/proxy/pkg/webdav/response.go b/proxy/pkg/webdav/response.go new file mode 100644 index 00000000000..0279042719e --- /dev/null +++ b/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) +} diff --git a/proxy/pkg/webdav/webdav.go b/proxy/pkg/webdav/webdav.go new file mode 100644 index 00000000000..cfb59329a31 --- /dev/null +++ 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 +} diff --git a/tests/acceptance/expected-failures-API-on-OCIS-storage.md b/tests/acceptance/expected-failures-API-on-OCIS-storage.md index a73b9c521b3..dadb2584436 100644 --- a/tests/acceptance/expected-failures-API-on-OCIS-storage.md +++ b/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) diff --git a/tests/acceptance/expected-failures-API-on-OWNCLOUD-storage.md b/tests/acceptance/expected-failures-API-on-OWNCLOUD-storage.md index 2b794c769e2..c913e53cec4 100644 --- a/tests/acceptance/expected-failures-API-on-OWNCLOUD-storage.md +++ b/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)