Skip to content

Commit

Permalink
Add query param to delete after file stream
Browse files Browse the repository at this point in the history
This is useful for temporary nodes where once the user has the data, the node is no longer needed, and the link to the data is expected to work once.
  • Loading branch information
MrCreosote committed May 9, 2024
1 parent 790928a commit 44bf972
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 8 deletions.
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ RETURNS: an ACL.
## Download a file from a node
```
AUTHORIZATION OPTIONAL
GET /node/<id>?download[_raw][&seek=#][&length=#]
GET /node/<id>?download[_raw][&seek=#][&length=#][&del]
RETURNS: the file content.
```
Expand All @@ -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
Expand Down
17 changes: 11 additions & 6 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions service/errortypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand Down
72 changes: 71 additions & 1 deletion service/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand All @@ -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() {
Expand Down
31 changes: 31 additions & 0 deletions service/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 44bf972

Please sign in to comment.