From 903cbfe0df3427eb07327d009aeb5f5cf4375b1b Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Fri, 1 Nov 2024 10:54:19 +0100 Subject: [PATCH 1/2] Add some basic sanity checking when creating a dataset --- sda/cmd/api/api.go | 56 +++++++++++++- sda/cmd/api/api_test.go | 163 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 206 insertions(+), 13 deletions(-) diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go index b0efe1476..31162dace 100644 --- a/sda/cmd/api/api.go +++ b/sda/cmd/api/api.go @@ -27,6 +27,12 @@ import ( log "github.com/sirupsen/logrus" ) +type dataset struct { + AccessionIDs []string `json:"accession_ids"` + DatasetID string `json:"dataset_id"` + User string `json:"user"` +} + var ( Conf *config.Config err error @@ -313,7 +319,7 @@ func setAccession(c *gin.Context) { } func createDataset(c *gin.Context) { - var dataset schema.DatasetMapping + var dataset dataset if err := c.BindJSON(&dataset); err != nil { c.AbortWithStatusJSON( http.StatusBadRequest, @@ -326,8 +332,52 @@ func createDataset(c *gin.Context) { return } - dataset.Type = "mapping" - marshaledMsg, _ := json.Marshal(&dataset) + if len(dataset.AccessionIDs) == 0 { + c.AbortWithStatusJSON(http.StatusBadRequest, "at least one accessionID is reqired") + + return + } + + for _, stableID := range dataset.AccessionIDs { + inboxPath, err := Conf.API.DB.GetInboxPath(stableID) + if err != nil { + switch { + case err.Error() == "sql: no rows in result set": + log.Debugln(err.Error()) + c.AbortWithStatusJSON(http.StatusBadRequest, fmt.Sprintf("accession ID not found: %s", stableID)) + + return + default: + log.Debugln(err.Error()) + c.AbortWithStatusJSON(http.StatusInternalServerError, err.Error()) + + return + } + } + + _, err = Conf.API.DB.GetCorrID(dataset.User, inboxPath) + if err != nil { + switch { + case err.Error() == "sql: no rows in result set": + log.Debugln(err.Error()) + c.AbortWithStatusJSON(http.StatusBadRequest, "accession ID owned by other user") + + return + default: + log.Debugln(err.Error()) + c.AbortWithStatusJSON(http.StatusInternalServerError, err.Error()) + + return + } + } + } + + mapping := schema.DatasetMapping{ + Type: "mapping", + AccessionIDs: dataset.AccessionIDs, + DatasetID: dataset.DatasetID, + } + marshaledMsg, _ := json.Marshal(&mapping) if err := schema.ValidateJSON(fmt.Sprintf("%s/dataset-mapping.json", Conf.Broker.SchemasPath), marshaledMsg); err != nil { log.Debugln(err.Error()) c.AbortWithStatusJSON(http.StatusBadRequest, err.Error()) diff --git a/sda/cmd/api/api_test.go b/sda/cmd/api/api_test.go index 9332893ae..3ffdf7c7b 100644 --- a/sda/cmd/api/api_test.go +++ b/sda/cmd/api/api_test.go @@ -953,11 +953,7 @@ func (suite *TestSuite) TestCreateDataset() { Conf.API.Admins = []string{"dummy"} Conf.Broker.SchemasPath = "../../schemas/isolated" - type dataset struct { - AccessionIDs []string `json:"accession_ids"` - DatasetID string `json:"dataset_id"` - } - accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{"API:accession-id-11", "API:accession-id-12", "API:accession-id-13"}, DatasetID: "API:dataset-01"}) + accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{"API:accession-id-11"}, DatasetID: "API:dataset-01", User: "dummy"}) // Mock request and response holders w := httptest.NewRecorder() r := httptest.NewRequest(http.MethodPost, "/dataset/create", bytes.NewBuffer(accessionMsg)) @@ -989,16 +985,160 @@ func (suite *TestSuite) TestCreateDataset() { } func (suite *TestSuite) TestCreateDataset_BadFormat() { + user := "dummy" + filePath := "/inbox/dummy/file12.c4gh" + + fileID, err := Conf.API.DB.RegisterFile(filePath, user) + assert.NoError(suite.T(), err, "failed to register file in database") + err = Conf.API.DB.UpdateFileEventLog(fileID, "uploaded", fileID, user, "{}", "{}") + assert.NoError(suite.T(), err, "failed to update satus of file in database") + + encSha := sha256.New() + _, err = encSha.Write([]byte("Checksum")) + assert.NoError(suite.T(), err) + + decSha := sha256.New() + _, err = decSha.Write([]byte("DecryptedChecksum")) + assert.NoError(suite.T(), err) + + fileInfo := database.FileInfo{ + Checksum: fmt.Sprintf("%x", encSha.Sum(nil)), + Size: 1000, + Path: filePath, + DecryptedChecksum: fmt.Sprintf("%x", decSha.Sum(nil)), + DecryptedSize: 948, + } + err = Conf.API.DB.SetArchived(fileInfo, fileID, fileID) + assert.NoError(suite.T(), err, "failed to mark file as Archived") + + err = Conf.API.DB.SetVerified(fileInfo, fileID, fileID) + assert.NoError(suite.T(), err, "got (%v) when marking file as verified", err) + + err = Conf.API.DB.SetAccessionID("API:accession-id-11", fileID) + assert.NoError(suite.T(), err, "got (%v) when marking file as verified", err) + + err = Conf.API.DB.SetAccessionID("API:accession-id-11", fileID) + assert.NoError(suite.T(), err, "got (%v) when marking file as verified", err) + + err = Conf.API.DB.UpdateFileEventLog(fileID, "ready", fileID, "finalize", "{}", "{}") + assert.NoError(suite.T(), err, "got (%v) when marking file as ready", err) + gin.SetMode(gin.ReleaseMode) assert.NoError(suite.T(), setupJwtAuth()) Conf.API.Admins = []string{"dummy"} Conf.Broker.SchemasPath = "../../schemas/federated" - type dataset struct { - AccessionIDs []string `json:"accession_ids"` - DatasetID string `json:"dataset_id"` + accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{"API:accession-id-11"}, DatasetID: "API:dataset-01", User: "dummy"}) + // Mock request and response holders + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/dataset/create", bytes.NewBuffer(accessionMsg)) + r.Header.Add("Authorization", "Bearer "+suite.Token) + + _, router := gin.CreateTestContext(w) + router.POST("/dataset/create", isAdmin(), createDataset) + + router.ServeHTTP(w, r) + response := w.Result() + body, err := io.ReadAll(response.Body) + assert.NoError(suite.T(), err) + response.Body.Close() + + assert.Equal(suite.T(), http.StatusBadRequest, response.StatusCode) + assert.Contains(suite.T(), string(body), "does not match pattern") +} + +func (suite *TestSuite) TestCreateDataset_MissingAccessionIDs() { + gin.SetMode(gin.ReleaseMode) + assert.NoError(suite.T(), setupJwtAuth()) + Conf.API.Admins = []string{"dummy"} + Conf.Broker.SchemasPath = "../../schemas/isolated" + + accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{}, DatasetID: "failure", User: "dummy"}) + // Mock request and response holders + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/dataset/create", bytes.NewBuffer(accessionMsg)) + r.Header.Add("Authorization", "Bearer "+suite.Token) + + _, router := gin.CreateTestContext(w) + router.POST("/dataset/create", isAdmin(), createDataset) + + router.ServeHTTP(w, r) + response := w.Result() + body, err := io.ReadAll(response.Body) + assert.NoError(suite.T(), err) + response.Body.Close() + + assert.Equal(suite.T(), http.StatusBadRequest, response.StatusCode) + assert.Contains(suite.T(), string(body), "at least one accessionID is reqired") +} + +func (suite *TestSuite) TestCreateDataset_WrongIDs() { + gin.SetMode(gin.ReleaseMode) + assert.NoError(suite.T(), setupJwtAuth()) + Conf.API.Admins = []string{"dummy"} + Conf.Broker.SchemasPath = "../../schemas/isolated" + + accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{"API:accession-id-11"}, DatasetID: "API:dataset-01", User: "dummy"}) + // Mock request and response holders + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/dataset/create", bytes.NewBuffer(accessionMsg)) + r.Header.Add("Authorization", "Bearer "+suite.Token) + + _, router := gin.CreateTestContext(w) + router.POST("/dataset/create", isAdmin(), createDataset) + + router.ServeHTTP(w, r) + response := w.Result() + body, err := io.ReadAll(response.Body) + assert.NoError(suite.T(), err) + response.Body.Close() + + assert.Equal(suite.T(), http.StatusBadRequest, response.StatusCode) + assert.Contains(suite.T(), string(body), "accession ID not found: ") +} + +func (suite *TestSuite) TestCreateDataset_WrongUser() { + user := "dummy" + filePath := "/inbox/dummy/file12.c4gh" + + fileID, err := Conf.API.DB.RegisterFile(filePath, user) + assert.NoError(suite.T(), err, "failed to register file in database") + err = Conf.API.DB.UpdateFileEventLog(fileID, "uploaded", fileID, user, "{}", "{}") + assert.NoError(suite.T(), err, "failed to update satus of file in database") + + encSha := sha256.New() + _, err = encSha.Write([]byte("Checksum")) + assert.NoError(suite.T(), err) + + decSha := sha256.New() + _, err = decSha.Write([]byte("DecryptedChecksum")) + assert.NoError(suite.T(), err) + + fileInfo := database.FileInfo{ + Checksum: fmt.Sprintf("%x", encSha.Sum(nil)), + Size: 1000, + Path: filePath, + DecryptedChecksum: fmt.Sprintf("%x", decSha.Sum(nil)), + DecryptedSize: 948, } - accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{"API:accession-id-11", "API:accession-id-12", "API:accession-id-13"}, DatasetID: "API:dataset-01"}) + err = Conf.API.DB.SetArchived(fileInfo, fileID, fileID) + assert.NoError(suite.T(), err, "failed to mark file as Archived") + + err = Conf.API.DB.SetVerified(fileInfo, fileID, fileID) + assert.NoError(suite.T(), err, "got (%v) when marking file as verified", err) + + err = Conf.API.DB.SetAccessionID("API:accession-id-11", fileID) + assert.NoError(suite.T(), err, "got (%v) when marking file as verified", err) + + err = Conf.API.DB.SetAccessionID("API:accession-id-11", fileID) + assert.NoError(suite.T(), err, "got (%v) when marking file as verified", err) + + gin.SetMode(gin.ReleaseMode) + assert.NoError(suite.T(), setupJwtAuth()) + Conf.API.Admins = []string{"dummy"} + Conf.Broker.SchemasPath = "../../schemas/isolated" + + accessionMsg, _ := json.Marshal(dataset{AccessionIDs: []string{"API:accession-id-11"}, DatasetID: "API:dataset-01", User: "tester"}) // Mock request and response holders w := httptest.NewRecorder() r := httptest.NewRequest(http.MethodPost, "/dataset/create", bytes.NewBuffer(accessionMsg)) @@ -1009,9 +1149,12 @@ func (suite *TestSuite) TestCreateDataset_BadFormat() { router.ServeHTTP(w, r) response := w.Result() - defer response.Body.Close() + body, err := io.ReadAll(response.Body) + assert.NoError(suite.T(), err) + response.Body.Close() assert.Equal(suite.T(), http.StatusBadRequest, response.StatusCode) + assert.Contains(suite.T(), string(body), "accession ID owned by other user") } func (suite *TestSuite) TestReleaseDataset() { From 268133d381761a63728ee09a444580015f58615c Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Fri, 1 Nov 2024 10:57:55 +0100 Subject: [PATCH 2/2] Update Readme --- sda/cmd/api/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sda/cmd/api/api.md b/sda/cmd/api/api.md index fa96b4aeb..2c921dacb 100644 --- a/sda/cmd/api/api.md +++ b/sda/cmd/api/api.md @@ -58,7 +58,7 @@ Admin endpoints are only available to a set of whitelisted users specified in th ``` - `/dataset/create` - - accepts `POST` requests with JSON data with the format: `{"accession_ids": ["", ""], "dataset_id": ""}` + - accepts `POST` requests with JSON data with the format: `{"accession_ids": ["", ""], "dataset_id": "", "user": ""}` - creates a dataset from the list of accession IDs and the dataset ID. - Error codes