diff --git a/README.md b/README.md index 8033038..cebf585 100644 --- a/README.md +++ b/README.md @@ -171,7 +171,7 @@ RETURNS: an ACL. ## Download a file from a node ``` AUTHORIZATION OPTIONAL -GET /node/?download[_raw][&seek=#][&length=#] +GET /node/?download[_raw][&seek=#][&length=#][&del] RETURNS: the file content. ``` @@ -186,6 +186,10 @@ to the file size is an error. Defaults to 0. `length` may be greater than the remaining file length. Defaults to 0, which indicates that the remainder of the file should be returned. +`del` causes the node to be deleted once the file contents have been streamed. The user must +be the node owner or a service administrator. Note this is playing very fast and loose with the +semantics of an HTTP GET. + ## Set a node to be publicly readable ``` AUTHORIZATION REQUIRED diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 43c2f8c..fb7efea 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,17 +1,22 @@ +# 0.1.4 + +* Added the `del` param when downloading the file from a node. + # 0.1.3 -- Added GHA workflows and removed Travis CI -- MongoController is now compatible with Mongo versions 2 through 7 -- Updated test config file to specify the auth2 shadow jar path vs. the jars repo path +* Added GHA workflows and removed Travis CI +* MongoController is now compatible with Mongo versions 2 through 7 +* Updated test config file to specify the auth2 shadow jar path vs. the jars repo path # 0.1.2 -- Support for disabling SSL verification of remote S3 certificates (default false) with the s3-disable-ssl-verify option in the configuration file. +* Support for disabling SSL verification of remote S3 certificates (default false) + with the s3-disable-ssl-verify option in the configuration file. # 0.1.1 -- Added seek & length parameters to file download requests +* Added seek & length parameters to file download requests # 0.1.0 -- Initial release +* Initial release diff --git a/service/errortypes.go b/service/errortypes.go index b08e72e..8c7ebe4 100644 --- a/service/errortypes.go +++ b/service/errortypes.go @@ -14,6 +14,20 @@ const ( invalidAuthHeader = "Invalid authorization header or content" ) +// UnauthorizedCustomError denotes that an unauthorized operation was requested that needs +// special explanation in the error string. +type UnauthorizedCustomError string + +// UnauthorizedCustomError creates a new UnauthorizedCustomError. +func NewUnauthorizedCustomError(err string) *UnauthorizedCustomError { + e := UnauthorizedCustomError(err) + return &e +} + +func (e *UnauthorizedCustomError) Error() string { + return string(*e) +} + func translateError(err error) (code int, errstr string) { // not sure about this approach. Alternative is to add some state to every error that // can be mapped to a code, and I'm not super thrilled about that either. @@ -30,6 +44,8 @@ func translateError(err error) (code int, errstr string) { // Shock compatibility, really should be 403 forbidden return http.StatusBadRequest, "Users that are not node owners can only delete " + "themselves from ACLs." + case *UnauthorizedCustomError: + return http.StatusUnauthorized, t.Error() case *auth.InvalidUserError: // no equivalent shock error, it accepts any string as a username return http.StatusBadRequest, t.Error() diff --git a/service/integration_test.go b/service/integration_test.go index ae4a07b..4877f94 100644 --- a/service/integration_test.go +++ b/service/integration_test.go @@ -544,6 +544,23 @@ func (t *TestSuite) TestStoreAndGetNodeAsAdminWithFormatAndTrailingSlashAndSeekA t.checkFile(t.url+path+dlrlen, path, &t.kBaseAdmin, 7, "", []byte("foobarb")) } +func (t *TestSuite) TestGetFileWithDelete() { + t.testGetFileWithDelete(t.noRole) + t.testGetFileWithDelete(t.stdRole) +} + +func (t *TestSuite) testGetFileWithDelete(deleter User) { + body := t.req("POST", t.url+"/node", strings.NewReader("foobarbaz"), + "OAuth "+t.noRole.token, 374, 200) + id := (body["data"].(map[string]interface{}))["id"].(string) + t.loggerhook.Reset() + + path := "/node/" + id + dl := "?download&del" + t.checkFile(t.url+path+dl, path, &deleter, 9, id, []byte("foobarbaz")) + t.checkNodeDeleted(id, t.noRole) +} + func (t *TestSuite) TestStoreMIMEMultipartFilenameFormat() { partsuffix := ` filename="myfile.txt"` format := "gasbomb" @@ -1065,7 +1082,7 @@ func (t *TestSuite) TestGetNodeFailPerms() { ) } -func (t *TestSuite) TestGetNodeFailSeekAndLength() { +func (t *TestSuite) TestGetFileFailSeekAndLength() { body := t.req("POST", t.url+"/node", strings.NewReader("foobarbaz"), "OAuth "+t.kBaseAdmin.token, 374, 200) id := (body["data"].(map[string]interface{}))["id"].(string) @@ -1104,6 +1121,49 @@ func (t *TestSuite) TestGetNodeFailSeekAndLength() { ) } +func (t *TestSuite) TestGetFileFailDelete() { + // only tests differences between the standard path and the delete path + body := t.req("POST", t.url+"/node", strings.NewReader("foobarbaz"), + "OAuth "+t.noRole.token, 374, 200) + id := (body["data"].(map[string]interface{}))["id"].(string) + t.loggerhook.Reset() + + uid := uuid.New() + body2 := t.get(t.url+"/node/"+uid.String()+"?download&del", &t.noRole, 75, 404) + t.checkError(body2, 404, "Node not found") + t.checkLogs(logEvent{logrus.ErrorLevel, "GET", "/node/"+uid.String(), 404, &t.noRole.user, + "Node not found", mtmap(), false}, + ) + body3 := t.get(t.url+"/node/"+id+"?download&del", &t.noRole2, 78, 401) + t.checkError(body3, 401, "User Unauthorized") + t.checkLogs(logEvent{logrus.ErrorLevel, "GET", "/node/" + id, 401, &t.noRole2.user, + "User Unauthorized", mtmap(), false}, + ) + body4 := t.get(t.url+"/node/"+id+"?download&del", nil, 78, 401) + t.checkError(body4, 401, "User Unauthorized") + t.checkLogs(logEvent{logrus.ErrorLevel, "GET", "/node/" + id, 401, nil, + "User Unauthorized", mtmap(), false}, + ) + // add user 2 to read acl + t.req("PUT", t.url+"/node/"+id+"/acl/read?users="+t.noRole2.user, nil, + "Oauth "+t.noRole.token, 441, 200) + t.loggerhook.Reset() + body5 := t.get(t.url+"/node/"+id+"?download&del", &t.noRole2, 94, 401) + t.checkError(body5, 401, "Only node owners can delete nodes") + t.checkLogs(logEvent{logrus.ErrorLevel, "GET", "/node/" + id, 401, &t.noRole2.user, + "Only node owners can delete nodes", mtmap(), false}, + ) + // make node public + t.req("PUT", t.url+"/node/"+id+"/acl/public_read", nil, + "Oauth "+t.noRole.token, 440, 200) + t.loggerhook.Reset() + body6 := t.get(t.url+"/node/"+id+"?download&del", nil, 94, 401) + t.checkError(body6, 401, "Only node owners can delete nodes") + t.checkLogs(logEvent{logrus.ErrorLevel, "GET", "/node/" + id, 401, nil, + "Only node owners can delete nodes", mtmap(), false}, + ) +} + func (t *TestSuite) TestUnexpectedError() { defer t.createTestBucket() body := t.req("POST", t.url+"/node", strings.NewReader("foobarbaz"), @@ -1150,6 +1210,7 @@ func (t *TestSuite) TestDeleteNode() { t.checkLogs(logEvent{logrus.InfoLevel, "DELETE", "/node/" + id, 200, &t.noRole.user, "request complete", mtmap(), true}, ) + t.checkNodeDeleted(id, t.noRole) // test delete as admin and with trailing slash body = t.req("POST", t.url+"/node", strings.NewReader("foobarbaz"), @@ -1164,6 +1225,15 @@ func (t *TestSuite) TestDeleteNode() { t.checkLogs(logEvent{logrus.InfoLevel, "DELETE", "/node/" + id + "/", 200, &t.kBaseAdmin.user, "request complete", mtmap(), true}, ) + t.checkNodeDeleted(id, t.noRole) +} + +func (t *TestSuite) checkNodeDeleted(id string, user User) { + body := t.get(t.url+"/node/"+id, &user, 75, 404) + t.checkError(body, 404, "Node not found") + t.checkLogs(logEvent{logrus.ErrorLevel, "GET", "/node/" + id, 404, &user.user, + "Node not found", mtmap(), false}, + ) } func (t *TestSuite) TestDeleteNodeFail() { diff --git a/service/server.go b/service/server.go index e331aef..b8f33ef 100644 --- a/service/server.go +++ b/service/server.go @@ -514,6 +514,11 @@ func (s *Server) getNode(w http.ResponseWriter, r *http.Request) { user := getUser(r) download := download(r.URL) if download != "" { + del, err := checkDel(s, user, id, r.URL) + if err != nil { + writeError(le, err, w) + return + } seek, length, err := getSeekAndLengthFromQuery(r.URL) if err != nil { writeError(le, err, w) @@ -534,6 +539,9 @@ func (s *Server) getNode(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-length", strconv.FormatInt(size, 10)) w.Header().Set("content-type", "application/octet-stream") io.Copy(w, datareader) + if del { + s.store.DeleteNode(*user, *id) // ignore errors since file is written to output + } } else { node, err := s.store.Get(user, *id) if err != nil { @@ -544,6 +552,29 @@ func (s *Server) getNode(w http.ResponseWriter, r *http.Request) { } } +func checkDel(s *Server, user *auth.User, id *uuid.UUID, u *url.URL) (del bool, err error) { + delete := false + if _, ok := u.Query()["del"]; ok { + delete = true + // If we want to move the deletion into the core logic it could go like this: + // * pass in deletion flag and a pre-write function that accepts a node + // * do the ACL checks, etc. + // * the pre-write function is called, which writes the headers to the writer + // * the core logic should not know the writer is a http.ResponseWriter + // * stream the file + // * delete the node + // not worth the trouble for now + node, err := s.store.Get(user, *id) + if err != nil { + return false, err + } + if user == nil || (user.GetUserName() != node.Owner.AccountName && !user.IsAdmin()) { + return false, NewUnauthorizedCustomError("Only node owners can delete nodes") + } + } + return delete, nil +} + func getSeekAndLengthFromQuery(u *url.URL) (uint64, uint64, error) { seek, err := getUint64FromQuery(u, "seek") if err != nil {