From cef2c047c0353c5dd90665c759aea269fc046c32 Mon Sep 17 00:00:00 2001
From: dbampalikis <dimitris.bampalikis@gmail.com>
Date: Thu, 7 Nov 2024 16:30:53 +0100
Subject: [PATCH 01/20] [postgres] Add api user priviledges

---
 postgresql/initdb.d/01_main.sql   |  3 ++-
 postgresql/initdb.d/04_grants.sql |  3 ++-
 postgresql/migratedb.d/14.sql     |  2 +-
 postgresql/migratedb.d/15.sql     | 22 ++++++++++++++++++++++
 4 files changed, 27 insertions(+), 3 deletions(-)
 create mode 100644 postgresql/migratedb.d/15.sql

diff --git a/postgresql/initdb.d/01_main.sql b/postgresql/initdb.d/01_main.sql
index cc27cf3df..abbe307f6 100644
--- a/postgresql/initdb.d/01_main.sql
+++ b/postgresql/initdb.d/01_main.sql
@@ -28,7 +28,8 @@ VALUES (0, now(), 'Created with version'),
        (11, now(), 'Grant select permission to download on dataset_event_log'),
        (12, now(), 'Add key hash'),
        (13, now(), 'Create API user'),
-       (14, now(), 'Create Auth user');
+       (14, now(), 'Create Auth user'),
+       (15, now(), 'Give API user insert priviledge in logs table');
 
 -- Datasets are used to group files, and permissions are set on the dataset
 -- level
diff --git a/postgresql/initdb.d/04_grants.sql b/postgresql/initdb.d/04_grants.sql
index aa8a423f4..0cc65072d 100644
--- a/postgresql/initdb.d/04_grants.sql
+++ b/postgresql/initdb.d/04_grants.sql
@@ -164,12 +164,13 @@ GRANT USAGE ON SCHEMA sda TO api;
 GRANT SELECT ON sda.files TO api;
 GRANT SELECT ON sda.file_dataset TO api;
 GRANT SELECT ON sda.checksums TO api;
-GRANT SELECT ON sda.file_event_log TO api;
+GRANT SELECT, INSERT ON sda.file_event_log TO api;
 GRANT SELECT ON sda.encryption_keys TO api;
 GRANT SELECT ON sda.datasets TO api;
 GRANT SELECT ON sda.dataset_event_log TO api;
 GRANT INSERT ON sda.encryption_keys TO api;
 GRANT UPDATE ON sda.encryption_keys TO api;
+GRANT USAGE, SELECT ON SEQUENCE sda.file_event_log_id_seq TO api;
 
 -- legacy schema
 GRANT USAGE ON SCHEMA local_ega TO api;
diff --git a/postgresql/migratedb.d/14.sql b/postgresql/migratedb.d/14.sql
index 24a9cff6e..f39a19af1 100644
--- a/postgresql/migratedb.d/14.sql
+++ b/postgresql/migratedb.d/14.sql
@@ -45,4 +45,4 @@ BEGIN
     RAISE NOTICE 'Schema migration from % to % does not apply now, skipping', sourcever, sourcever+1;
   END IF;
 END
-$$
\ No newline at end of file
+$$
diff --git a/postgresql/migratedb.d/15.sql b/postgresql/migratedb.d/15.sql
new file mode 100644
index 000000000..2849382c4
--- /dev/null
+++ b/postgresql/migratedb.d/15.sql
@@ -0,0 +1,22 @@
+
+DO
+$$
+DECLARE
+-- The version we know how to do migration from, at the end of a successful migration
+-- we will no longer be at this version.
+  sourcever INTEGER := 14;
+  changes VARCHAR := 'Give API user insert priviledge in logs table';
+BEGIN
+  IF (select max(version) from sda.dbschema_version) = sourcever then
+    RAISE NOTICE 'Doing migration from schema version % to %', sourcever, sourcever+1;
+    RAISE NOTICE 'Changes: %', changes;
+    INSERT INTO sda.dbschema_version VALUES(sourcever+1, now(), changes);
+    
+    GRANT INSERT ON sda.file_event_log TO api;
+    GRANT USAGE, SELECT ON SEQUENCE sda.file_event_log_id_seq TO api;
+
+  ELSE
+    RAISE NOTICE 'Schema migration from % to % does not apply now, skipping', sourcever, sourcever+1;
+  END IF;
+END
+$$

From 25b456ae68cd8f07d30170c7c642b9873920ba1d Mon Sep 17 00:00:00 2001
From: dbampalikis <dimitris.bampalikis@gmail.com>
Date: Thu, 7 Nov 2024 16:32:29 +0100
Subject: [PATCH 02/20] [api] Add api configuration

---
 sda/internal/config/config.go      | 12 ++++++++++++
 sda/internal/config/config_test.go |  1 +
 2 files changed, 13 insertions(+)

diff --git a/sda/internal/config/config.go b/sda/internal/config/config.go
index ae08188da..3b5e149fc 100644
--- a/sda/internal/config/config.go
+++ b/sda/internal/config/config.go
@@ -83,6 +83,7 @@ type APIConf struct {
 	Session    SessionConfig
 	DB         *database.SDAdb
 	MQ         *broker.AMQPBroker
+	INBOX      storage.Backend
 }
 
 type SessionConfig struct {
@@ -214,6 +215,15 @@ func NewConfig(app string) (*Config, error) {
 			"db.user",
 			"db.password",
 			"db.database",
+			"inbox.type",
+		}
+		switch viper.GetString("inbox.type") {
+		case S3:
+			requiredConfVars = append(requiredConfVars, []string{"inbox.url", "inbox.accesskey", "inbox.secretkey", "inbox.bucket"}...)
+		case POSIX:
+			requiredConfVars = append(requiredConfVars, []string{"inbox.location"}...)
+		default:
+			return nil, fmt.Errorf("inbox.type not set")
 		}
 	case "auth":
 		requiredConfVars = []string{
@@ -464,6 +474,8 @@ func NewConfig(app string) (*Config, error) {
 			return nil, err
 		}
 
+		c.configInbox()
+
 		err = c.configAPI()
 		if err != nil {
 			return nil, err
diff --git a/sda/internal/config/config_test.go b/sda/internal/config/config_test.go
index 3b877cb1b..563fcca3f 100644
--- a/sda/internal/config/config_test.go
+++ b/sda/internal/config/config_test.go
@@ -65,6 +65,7 @@ func (suite *ConfigTestSuite) SetupTest() {
 	viper.Set("inbox.accesskey", "testaccess")
 	viper.Set("inbox.secretkey", "testsecret")
 	viper.Set("inbox.bucket", "testbucket")
+	viper.Set("inbox.type", "s3")
 	viper.Set("server.jwtpubkeypath", "testpath")
 	viper.Set("log.level", "debug")
 }

From 85508472b65f6941f2ff6fbdc8496c70b900db30 Mon Sep 17 00:00:00 2001
From: dbampalikis <dimitris.bampalikis@gmail.com>
Date: Thu, 7 Nov 2024 16:33:02 +0100
Subject: [PATCH 03/20] [api] Add fileID to api struct

---
 sda/internal/database/database.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sda/internal/database/database.go b/sda/internal/database/database.go
index 651e097c0..eed386a8c 100644
--- a/sda/internal/database/database.go
+++ b/sda/internal/database/database.go
@@ -46,6 +46,7 @@ type SyncData struct {
 }
 
 type SubmissionFileInfo struct {
+	FileID    string `json:"fileID"`
 	InboxPath string `json:"inboxPath"`
 	Status    string `json:"fileStatus"`
 	CreateAt  string `json:"createAt"`

From 222fcb65280de738372d57c7a435f6f0989594a8 Mon Sep 17 00:00:00 2001
From: dbampalikis <dimitris.bampalikis@gmail.com>
Date: Thu, 7 Nov 2024 16:36:07 +0100
Subject: [PATCH 04/20] [api] Change list to return id and skip disabled

---
 sda/internal/database/db_functions.go | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sda/internal/database/db_functions.go b/sda/internal/database/db_functions.go
index 8d58503a1..15e5c9ddc 100644
--- a/sda/internal/database/db_functions.go
+++ b/sda/internal/database/db_functions.go
@@ -639,7 +639,7 @@ func (dbs *SDAdb) getUserFiles(userID string) ([]*SubmissionFileInfo, error) {
 	db := dbs.DB
 
 	// select all files (that are not part of a dataset) of the user, each one annotated with its latest event
-	const query = "SELECT f.submission_file_path, e.event, f.created_at FROM sda.files f " +
+	const query = "SELECT f.id, f.submission_file_path, e.event, f.created_at FROM sda.files f " +
 		"LEFT JOIN (SELECT DISTINCT ON (file_id) file_id, started_at, event FROM sda.file_event_log ORDER BY file_id, started_at DESC) e ON f.id = e.file_id WHERE f.submission_user = $1 " +
 		"AND f.id NOT IN (SELECT f.id FROM sda.files f RIGHT JOIN sda.file_dataset d ON f.id = d.file_id); "
 
@@ -654,13 +654,15 @@ func (dbs *SDAdb) getUserFiles(userID string) ([]*SubmissionFileInfo, error) {
 	for rows.Next() {
 		// Read rows into struct
 		fi := &SubmissionFileInfo{}
-		err := rows.Scan(&fi.InboxPath, &fi.Status, &fi.CreateAt)
+		err := rows.Scan(&fi.FileID, &fi.InboxPath, &fi.Status, &fi.CreateAt)
 		if err != nil {
 			return nil, err
 		}
 
-		// Add instance of struct (file) to array
-		files = append(files, fi)
+		// Add instance of struct (file) to array if the status is not disabled
+		if fi.Status != "disabled" {
+			files = append(files, fi)
+		}
 	}
 
 	return files, nil

From c27e85ea01265b7893430a06dce43a6d5c2a35a0 Mon Sep 17 00:00:00 2001
From: dbampalikis <dimitris.bampalikis@gmail.com>
Date: Thu, 7 Nov 2024 16:37:07 +0100
Subject: [PATCH 05/20] [api] Add disable file event log function

---
 sda/internal/database/db_functions.go | 31 +++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/sda/internal/database/db_functions.go b/sda/internal/database/db_functions.go
index 15e5c9ddc..ef8b2ea73 100644
--- a/sda/internal/database/db_functions.go
+++ b/sda/internal/database/db_functions.go
@@ -63,6 +63,37 @@ func (dbs *SDAdb) getFileID(corrID string) (string, error) {
 	return fileID, nil
 }
 
+// UserFileExists checks if a file exists in the database
+// for a given user and fileID
+func (dbs *SDAdb) GetFilePathFromID(submission_user, fileID string) (string, error) {
+	var (
+		err      error
+		count    int
+		filePath string
+	)
+
+	for count == 0 || (err != nil && count < RetryTimes) {
+		filePath, err = dbs.getFilePathFromID(submission_user, fileID)
+		count++
+	}
+
+	return filePath, err
+}
+func (dbs *SDAdb) getFilePathFromID(submission_user, fileID string) (string, error) {
+	dbs.checkAndReconnectIfNeeded()
+	db := dbs.DB
+
+	const getFilePath = "SELECT submission_file_path from sda.files where submission_user= $1 and id = $2;"
+
+	var filePath string
+	err := db.QueryRow(getFilePath, submission_user, fileID).Scan(&filePath)
+	if err != nil {
+		return "", err
+	}
+
+	return filePath, nil
+}
+
 // UpdateFileEventLog updates the status in of the file in the database.
 // The message parameter is the rabbitmq message sent on file upload.
 func (dbs *SDAdb) UpdateFileEventLog(fileUUID, event, corrID, user, details, message string) error {

From 3980efd7c7b5d450a5bb443bd4212e2242486d84 Mon Sep 17 00:00:00 2001
From: dbampalikis <dimitris.bampalikis@gmail.com>
Date: Thu, 7 Nov 2024 16:37:48 +0100
Subject: [PATCH 06/20] [api] Add delete file functionality

---
 sda/cmd/api/api.go | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index c8c2cdee3..94107cdd1 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -8,6 +8,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"math"
 	"net/http"
 	"os"
 	"os/signal"
@@ -25,6 +26,7 @@ import (
 	"github.com/neicnordic/sensitive-data-archive/internal/database"
 	"github.com/neicnordic/sensitive-data-archive/internal/jsonadapter"
 	"github.com/neicnordic/sensitive-data-archive/internal/schema"
+	"github.com/neicnordic/sensitive-data-archive/internal/storage"
 	"github.com/neicnordic/sensitive-data-archive/internal/userauth"
 	log "github.com/sirupsen/logrus"
 )
@@ -54,6 +56,10 @@ func main() {
 	if err != nil {
 		log.Fatal(err)
 	}
+	Conf.API.INBOX, err = storage.NewBackend(Conf.Inbox)
+	if err != nil {
+		log.Fatal(err)
+	}
 
 	if err := setupJwtAuth(); err != nil {
 		log.Fatalf("error when setting up JWT auth, reason %s", err.Error())
@@ -100,6 +106,7 @@ func setup(config *config.Config) *http.Server {
 	r.POST("/c4gh-keys/add", rbac(e), addC4ghHash)                      // Adds a key hash to the database
 	r.GET("/c4gh-keys/list", rbac(e), listC4ghHashes)                   // Lists key hashes in the database
 	r.POST("/c4gh-keys/deprecate/*keyHash", rbac(e), deprecateC4ghHash) // Deprecate a given key hash
+	r.DELETE("/file/:username/*file", rbac(e), deleteFile)              // Delete a file from inbox
 	// submission endpoints below here
 	r.POST("/file/ingest", rbac(e), ingestFile)                  // start ingestion of a file
 	r.POST("/file/accession", rbac(e), setAccession)             // assign accession ID to a file
@@ -285,6 +292,66 @@ func ingestFile(c *gin.Context) {
 	c.Status(http.StatusOK)
 }
 
+// The deleteFile function will take the user id and the file id
+// because the user id won't be in the path of the file in the future
+// therefore we need to make the list files, get the file id and then
+// call the delete file
+func deleteFile(c *gin.Context) {
+
+	inbox, err := storage.NewBackend(Conf.Inbox)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	submissionUser := c.Param("username")
+	log.Warn("submission user:", submissionUser)
+
+	// TODO: Add check for the input file path
+	fileID := c.Param("file")
+	fileID = strings.TrimPrefix(fileID, "/")
+	log.Warn("submission file:", fileID)
+	if fileID == "" {
+		c.AbortWithStatusJSON(http.StatusBadRequest, "file ID is required")
+	}
+
+	filePath := ""
+	// Get the file path from the fileID and submission user
+	if filePath, err = Conf.API.DB.GetFilePathFromID(submissionUser, fileID); err != nil {
+		log.Errorf("getting file from fileID failed, reason: (%v)", err)
+		c.AbortWithStatusJSON(http.StatusNotFound, "File could not be found in inbox")
+
+		return
+	}
+
+	// Requires a filepath instead of fileID
+	// TODO: The remove fails randomly sometimes, maybe we should retry
+	var RetryTimes = 5
+	for count := 1; count <= RetryTimes; count++ {
+		log.Warn("trying to remove file from inbox, try", count)
+		err = inbox.RemoveFile(filePath)
+		if err != nil {
+			log.Errorf("Remove file from inbox failed, reason: %v", err)
+		}
+
+		// The GetFileSize returns zero in case of error after retrying a number of times
+		fileSize, _ := inbox.GetFileSize(filePath)
+		if fileSize == 0 {
+			break
+		}
+
+		time.Sleep(time.Duration(math.Pow(2, float64(count))) * time.Second)
+	}
+
+	if err := Conf.API.DB.UpdateFileEventLog(fileID, "disabled", fileID, "api", "{}", "{}"); err != nil {
+		log.Errorf("set status deleted failed, reason: (%v)", err)
+		c.AbortWithStatusJSON(http.StatusInternalServerError, err.Error())
+
+		return
+	}
+
+	c.Status(http.StatusOK)
+}
+
 func setAccession(c *gin.Context) {
 	var accession schema.IngestionAccession
 	if err := c.BindJSON(&accession); err != nil {
@@ -477,6 +544,8 @@ func listActiveUsers(c *gin.Context) {
 	c.JSON(http.StatusOK, users)
 }
 
+// listUserFiles returns a list of files for a specific user
+// If the file has status disabled, the file will be skipped
 func listUserFiles(c *gin.Context) {
 	username := c.Param("username")
 	username = strings.TrimPrefix(username, "/")

From ebc071bfdbdff0cc87ea9a9bc2e7fe76f069df48 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Mon, 18 Nov 2024 13:30:16 +0100
Subject: [PATCH 07/20] [api] add integration test

---
 .../tests/sda/60_api_admin_test.sh            | 39 +++++++++++++++++++
 sda/internal/config/config.go                 |  1 -
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/.github/integration/tests/sda/60_api_admin_test.sh b/.github/integration/tests/sda/60_api_admin_test.sh
index 8478afd21..ee36724e8 100644
--- a/.github/integration/tests/sda/60_api_admin_test.sh
+++ b/.github/integration/tests/sda/60_api_admin_test.sh
@@ -1,5 +1,6 @@
 #!/bin/sh
 set -e
+cd shared || true
 
 token="$(curl http://oidc:8080/tokens | jq -r '.[0]')"
 result="$(curl -sk -L "http://api:8080/users/test@dummy.org/files" -H "Authorization: Bearer $token" | jq '. | length')"
@@ -35,4 +36,42 @@ resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer
 if [ "$resp" != "404" ]; then
 	echo "Error when starting re-verification of missing dataset, expected 404 got: $resp"
 	exit 1
+fi
+
+## reupload a file under a different name, test to delete it
+s3cmd -c s3cfg put NA12878.bam.c4gh s3://test_dummy.org/NC12878.bam.c4gh
+
+echo "waiting for upload to complete"
+URI=http://rabbitmq:15672
+if [ -n "$PGSSLCERT" ]; then
+    URI=https://rabbitmq:15671
+fi
+RETRY_TIMES=0
+until [ "$(curl -s -k -u guest:guest $URI/api/queues/sda/inbox | jq -r '."messages_ready"')" -gt 0 ]; do
+    echo "waiting for upload to complete"
+    RETRY_TIMES=$((RETRY_TIMES + 1))
+    if [ "$RETRY_TIMES" -eq 10 ]; then
+        echo "::error::Time out while waiting for upload to complete"
+        exit 1
+    fi
+    echo "read now:" `curl -s -k -u guest:guest $URI/api/queues/sda/inbox | jq -r '."messages_ready"'`
+    sleep 2
+done
+
+# get the fileId of the new file
+fileid="$(curl -k -L -H "Authorization: Bearer $token" -H "Content-Type: application/json" "http://api:8080/users/test@dummy.org/files" | jq -r '.[] | select(.inboxPath == "test_dummy.org/NC12878.bam.c4gh") | .fileID')"
+echo "Found uploaded file " $fileid
+
+# delete it
+resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
+if [ "$resp" != "200" ]; then
+	echo "Error when deleting the file, expected 200 got: $resp"
+	exit 1
+fi
+
+last_event=$(psql -U postgres -h postgres -d sda -At -c "SELECT event FROM sda.file_event_log WHERE file_id='$fileid' order by started_at desc limit 1;")
+echo "last event was" $last_event
+
+if [[ "$last_event" != "disabled" ]]; then
+    echo "The file $fileid does not have the expected las event 'disabled', but $last_event."
 fi
\ No newline at end of file
diff --git a/sda/internal/config/config.go b/sda/internal/config/config.go
index 3b5e149fc..cdc1472c5 100644
--- a/sda/internal/config/config.go
+++ b/sda/internal/config/config.go
@@ -215,7 +215,6 @@ func NewConfig(app string) (*Config, error) {
 			"db.user",
 			"db.password",
 			"db.database",
-			"inbox.type",
 		}
 		switch viper.GetString("inbox.type") {
 		case S3:

From 6ff7504bfb73f84f87a5bbed6206a20016e7bfea Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Mon, 18 Nov 2024 14:59:31 +0100
Subject: [PATCH 08/20] [api delete] only search for unarchived files

---
 sda/cmd/api/api.go                    |  2 +-
 sda/internal/database/db_functions.go | 16 ++++++++++------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index 94107cdd1..f972de131 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -316,7 +316,7 @@ func deleteFile(c *gin.Context) {
 
 	filePath := ""
 	// Get the file path from the fileID and submission user
-	if filePath, err = Conf.API.DB.GetFilePathFromID(submissionUser, fileID); err != nil {
+	if filePath, err = Conf.API.DB.GetInboxFilePathFromID(submissionUser, fileID); err != nil {
 		log.Errorf("getting file from fileID failed, reason: (%v)", err)
 		c.AbortWithStatusJSON(http.StatusNotFound, "File could not be found in inbox")
 
diff --git a/sda/internal/database/db_functions.go b/sda/internal/database/db_functions.go
index ef8b2ea73..ac44b91a2 100644
--- a/sda/internal/database/db_functions.go
+++ b/sda/internal/database/db_functions.go
@@ -63,9 +63,9 @@ func (dbs *SDAdb) getFileID(corrID string) (string, error) {
 	return fileID, nil
 }
 
-// UserFileExists checks if a file exists in the database
-// for a given user and fileID
-func (dbs *SDAdb) GetFilePathFromID(submission_user, fileID string) (string, error) {
+// GetInboxFilePathFromID checks if a file exists in the database for a given user and fileID
+// and that is not yet archived
+func (dbs *SDAdb) GetInboxFilePathFromID(submissionUser, fileID string) (string, error) {
 	var (
 		err      error
 		count    int
@@ -73,17 +73,21 @@ func (dbs *SDAdb) GetFilePathFromID(submission_user, fileID string) (string, err
 	)
 
 	for count == 0 || (err != nil && count < RetryTimes) {
-		filePath, err = dbs.getFilePathFromID(submission_user, fileID)
+		filePath, err = dbs.getInboxFilePathFromID(submissionUser, fileID)
 		count++
 	}
 
 	return filePath, err
 }
-func (dbs *SDAdb) getFilePathFromID(submission_user, fileID string) (string, error) {
+func (dbs *SDAdb) getInboxFilePathFromID(submission_user, fileID string) (string, error) {
 	dbs.checkAndReconnectIfNeeded()
 	db := dbs.DB
 
-	const getFilePath = "SELECT submission_file_path from sda.files where submission_user= $1 and id = $2;"
+	const getFilePath = "SELECT submission_file_path from sda.files where " +
+		"submission_user= $1 and id = $2 " +
+		"AND EXISTS (SELECT 1 FROM " +
+		"(SELECT event from sda.file_event_log where file_id = $2 order by started_at desc limit 1) " +
+		"as subquery WHERE event in ('registered', 'uploaded', 'submitted', 'ingested'))"
 
 	var filePath string
 	err := db.QueryRow(getFilePath, submission_user, fileID).Scan(&filePath)

From 89422c659f004b6b9ce1fbba74fb4a07294eb801 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Tue, 19 Nov 2024 11:52:49 +0100
Subject: [PATCH 09/20] [tests] api admin tests

---
 .../tests/sda/60_api_admin_test.sh            | 109 +++++++++++++++---
 1 file changed, 90 insertions(+), 19 deletions(-)

diff --git a/.github/integration/tests/sda/60_api_admin_test.sh b/.github/integration/tests/sda/60_api_admin_test.sh
index ee36724e8..83fda513e 100644
--- a/.github/integration/tests/sda/60_api_admin_test.sh
+++ b/.github/integration/tests/sda/60_api_admin_test.sh
@@ -3,10 +3,11 @@ set -e
 cd shared || true
 
 token="$(curl http://oidc:8080/tokens | jq -r '.[0]')"
+# Upload a file and mke sure it's listed
 result="$(curl -sk -L "http://api:8080/users/test@dummy.org/files" -H "Authorization: Bearer $token" | jq '. | length')"
 if [ "$result" -ne 2 ]; then
     echo "wrong number of files returned for user test@dummy.org"
-    echo "expected 4 got $result"
+    echo "expected 2 got $result"
     exit 1
 fi
 
@@ -38,40 +39,110 @@ if [ "$resp" != "404" ]; then
 	exit 1
 fi
 
-## reupload a file under a different name, test to delete it
-s3cmd -c s3cfg put NA12878.bam.c4gh s3://test_dummy.org/NC12878.bam.c4gh
+# Reupload a file under a different name, test to delete it
+s3cmd -c s3cfg put "NA12878.bam.c4gh" s3://test_dummy.org/NC12878.bam.c4gh
 
 echo "waiting for upload to complete"
 URI=http://rabbitmq:15672
 if [ -n "$PGSSLCERT" ]; then
-    URI=https://rabbitmq:15671
+   URI=https://rabbitmq:15671
 fi
 RETRY_TIMES=0
-until [ "$(curl -s -k -u guest:guest $URI/api/queues/sda/inbox | jq -r '."messages_ready"')" -gt 0 ]; do
-    echo "waiting for upload to complete"
-    RETRY_TIMES=$((RETRY_TIMES + 1))
-    if [ "$RETRY_TIMES" -eq 10 ]; then
-        echo "::error::Time out while waiting for upload to complete"
-        exit 1
-    fi
-    echo "read now:" `curl -s -k -u guest:guest $URI/api/queues/sda/inbox | jq -r '."messages_ready"'`
-    sleep 2
+until [ "$(curl -s -k -u guest:guest $URI/api/queues/sda/inbox | jq -r '."messages_ready"')" -eq 4 ]; do
+   echo "waiting for upload to complete"
+   RETRY_TIMES=$((RETRY_TIMES + 1))
+   if [ "$RETRY_TIMES" -eq 30 ]; then
+      echo "::error::Time out while waiting for upload to complete"
+      exit 1
+   fi
+   sleep 2
 done
 
 # get the fileId of the new file
 fileid="$(curl -k -L -H "Authorization: Bearer $token" -H "Content-Type: application/json" "http://api:8080/users/test@dummy.org/files" | jq -r '.[] | select(.inboxPath == "test_dummy.org/NC12878.bam.c4gh") | .fileID')"
-echo "Found uploaded file " $fileid
 
 # delete it
 resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
 if [ "$resp" != "200" ]; then
-	echo "Error when deleting the file, expected 200 got: $resp"
-	exit 1
+    echo "Error when deleting the file, expected 200 got: $resp"
+    exit 1
 fi
 
 last_event=$(psql -U postgres -h postgres -d sda -At -c "SELECT event FROM sda.file_event_log WHERE file_id='$fileid' order by started_at desc limit 1;")
-echo "last event was" $last_event
 
-if [[ "$last_event" != "disabled" ]]; then
-    echo "The file $fileid does not have the expected las event 'disabled', but $last_event."
+if [ "$last_event" != "disabled" ]; then
+   echo "The file $fileid does not have the expected las event 'disabled', but $last_event."
+fi
+
+
+# Try to delete an unknown file
+resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/badfileid")"
+if [ "$resp" != "404" ]; then
+    echo "Error when deleting the file, expected error got: $resp"
+    exit 1
+fi
+
+
+# Try to delete file of other user
+fileid="$(curl -k -L -H "Authorization: Bearer $token" -H "Content-Type: application/json" "http://api:8080/users/requester@demo.org/files" | jq -r '.[0]| .fileID')"
+resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
+if [ "$resp" != "404" ]; then
+    echo "Error when deleting the file, expected 404 got: $resp"
+    exit 1
+fi
+
+
+# Re-upload the file and use the api to ingest it, then try to delete it
+s3cmd -c s3cfg put NA12878.bam.c4gh s3://test_dummy.org/NE12878.bam.c4gh
+
+URI=http://rabbitmq:15672
+if [ -n "$PGSSLCERT" ]; then
+   URI=https://rabbitmq:15671
+fi
+RETRY_TIMES=0
+until [ "$(curl -s -k -u guest:guest $URI/api/queues/sda/inbox | jq -r '."messages_ready"')" -eq 6 ]; do
+   echo "waiting for upload to complete"
+   RETRY_TIMES=$((RETRY_TIMES + 1))
+   if [ "$RETRY_TIMES" -eq 3 ]; then
+      echo "::error::Time out while waiting for upload to complete"
+      #exit 1
+      break
+   fi
+   sleep 2
+done
+
+# Ingest it
+new_payload=$(
+jq -c -n \
+	--arg filepath "test_dummy.org/NE12878.bam.c4gh" \
+	--arg user "test@dummy.org" \
+	'$ARGS.named'
+)
+
+resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X POST -d "$new_payload" "http://api:8080/file/ingest")"
+if [ "$resp" != "200" ]; then
+    echo "Error when requesting to ingesting file, expected 200 got: $resp"
+    exit 1
+fi
+
+fileid="$(curl -k -L -H "Authorization: Bearer $token" -H "Content-Type: application/json" "http://api:8080/users/test@dummy.org/files" | jq -r '.[] | select(.inboxPath == "test_dummy.org/NE12878.bam.c4gh") | .fileID')"
+# wait for the fail to get the correct status
+RETRY_TIMES=0
+
+until [ "$(psql -U postgres -h postgres -d sda -At -c "select id from sda.file_events e where e.title in (select event from sda.file_event_log where file_id = '$fileid' order by started_at desc limit 1);")"  -gt 30 ]; do
+   echo "waiting for ingest to complete"
+   RETRY_TIMES=$((RETRY_TIMES + 1))
+   if [ "$RETRY_TIMES" -eq 30 ]; then
+      echo "::error::Time out while waiting for upload to complete"
+      exit 1
+   fi
+   sleep 2
+done
+
+# Try to delete file not in inbox
+fileid="$(curl -k -L -H "Authorization: Bearer $token" -H "Content-Type: application/json" "http://api:8080/users/test@dummy.org/files" | jq -r '.[] | select(.inboxPath == "test_dummy.org/NE12878.bam.c4gh") | .fileID')"
+resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
+if [ "$resp" != "404" ]; then
+	echo "Error when deleting the file, expected 404 got: $resp"
+	exit 1
 fi
\ No newline at end of file

From 6f00fc687d507c15a34b0d1acb6d24fba69d7c17 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Mon, 18 Nov 2024 14:59:31 +0100
Subject: [PATCH 10/20] [api delete] only search for unarchived files

---
 sda/internal/database/db_functions.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sda/internal/database/db_functions.go b/sda/internal/database/db_functions.go
index ac44b91a2..df1c4893b 100644
--- a/sda/internal/database/db_functions.go
+++ b/sda/internal/database/db_functions.go
@@ -79,7 +79,8 @@ func (dbs *SDAdb) GetInboxFilePathFromID(submissionUser, fileID string) (string,
 
 	return filePath, err
 }
-func (dbs *SDAdb) getInboxFilePathFromID(submission_user, fileID string) (string, error) {
+
+func (dbs *SDAdb) getInboxFilePathFromID(submissionUser, fileID string) (string, error) {
 	dbs.checkAndReconnectIfNeeded()
 	db := dbs.DB
 
@@ -90,7 +91,7 @@ func (dbs *SDAdb) getInboxFilePathFromID(submission_user, fileID string) (string
 		"as subquery WHERE event in ('registered', 'uploaded', 'submitted', 'ingested'))"
 
 	var filePath string
-	err := db.QueryRow(getFilePath, submission_user, fileID).Scan(&filePath)
+	err := db.QueryRow(getFilePath, submissionUser, fileID).Scan(&filePath)
 	if err != nil {
 		return "", err
 	}

From 934bb138f0f3460517bb805eddffb1717eac888a Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Tue, 19 Nov 2024 14:40:53 +0100
Subject: [PATCH 11/20] [tests] unit test for db function

---
 sda/internal/database/db_functions_test.go | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/sda/internal/database/db_functions_test.go b/sda/internal/database/db_functions_test.go
index ac86cb02e..11134d9a4 100644
--- a/sda/internal/database/db_functions_test.go
+++ b/sda/internal/database/db_functions_test.go
@@ -1187,3 +1187,28 @@ func (suite *DatabaseTests) TestGetDsatasetFiles() {
 	assert.NoError(suite.T(), err, "failed to get accessions for a dataset")
 	assert.Equal(suite.T(), []string{"accession_User-Q_00", "accession_User-Q_01", "accession_User-Q_02"}, accessions)
 }
+
+func (suite *DatabaseTests) TestGetInboxFilePathFromID() {
+
+	db, err := NewSDAdb(suite.dbConf)
+	assert.NoError(suite.T(), err, "got (%v) when creating new connection", err)
+
+	user := "UserX"
+	filePath := fmt.Sprintf("/%v/Deletefile1.c4gh", user)
+	fileID, err := db.RegisterFile(filePath, user)
+	if err != nil {
+		suite.FailNow("Failed to register file")
+	}
+	err = db.UpdateFileEventLog(fileID, "uploaded", fileID, "User-z", "{}", "{}")
+	if err != nil {
+		suite.FailNow("Failed to update file event log")
+	}
+	path, err := db.getInboxFilePathFromID(user, fileID)
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), path, filePath)
+
+	err = db.UpdateFileEventLog(fileID, "archived", fileID, user, "{}", "{}")
+	assert.NoError(suite.T(), err)
+	_, err = db.getInboxFilePathFromID(user, fileID)
+	assert.Error(suite.T(), err)
+}

From 605d9c541ccfec857feb3d9d4097c9dc55c72a2d Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Thu, 21 Nov 2024 08:24:10 +0100
Subject: [PATCH 12/20] [api] allow deletion of files marked with error

---
 sda/internal/database/db_functions.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sda/internal/database/db_functions.go b/sda/internal/database/db_functions.go
index df1c4893b..0edac5bad 100644
--- a/sda/internal/database/db_functions.go
+++ b/sda/internal/database/db_functions.go
@@ -88,7 +88,7 @@ func (dbs *SDAdb) getInboxFilePathFromID(submissionUser, fileID string) (string,
 		"submission_user= $1 and id = $2 " +
 		"AND EXISTS (SELECT 1 FROM " +
 		"(SELECT event from sda.file_event_log where file_id = $2 order by started_at desc limit 1) " +
-		"as subquery WHERE event in ('registered', 'uploaded', 'submitted', 'ingested'))"
+		"as subquery WHERE event in ('registered', 'uploaded', 'submitted', 'ingested', 'error'))"
 
 	var filePath string
 	err := db.QueryRow(getFilePath, submissionUser, fileID).Scan(&filePath)

From b455a6f47b78d5c540268af51f6c3c4f0e7385e2 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Thu, 21 Nov 2024 08:40:54 +0100
Subject: [PATCH 13/20] [charts] add inbox conf to api

---
 charts/sda-svc/templates/api-deploy.yaml | 49 ++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/charts/sda-svc/templates/api-deploy.yaml b/charts/sda-svc/templates/api-deploy.yaml
index ea68efe54..5d0486baf 100644
--- a/charts/sda-svc/templates/api-deploy.yaml
+++ b/charts/sda-svc/templates/api-deploy.yaml
@@ -147,6 +147,40 @@ spec:
           value: {{ required "A valid DB host is required" .Values.global.db.host | quote }}
         - name: DB_PORT
           value: {{ .Values.global.db.port | quote }}
+        - name: INBOX_TYPE
+      {{- if eq "s3" .Values.global.inbox.storageType }}
+          value: "s3"
+        - name: INBOX_BUCKET
+          value: {{ required "S3 inbox bucket missing" .Values.global.inbox.s3Bucket }}
+      {{- if and .Values.global.inbox.s3CaFile .Values.global.tls.enabled }}
+        - name: INBOX_CACERT
+          value: {{ template "tlsPath" . }}/ca.crt
+      {{- end }}
+        - name: INBOX_REGION
+          value: {{ default "us-east-1" .Values.global.inbox.s3Region }}
+        - name: INBOX_URL
+          value: {{ required "S3 inbox URL missing" .Values.global.inbox.s3Url }}
+      {{- if .Values.global.inbox.s3Port }}
+        - name: INBOX_PORT
+          value: {{ .Values.global.inbox.s3Port | quote }}
+      {{- end }}
+      {{- else }}
+          value: "posix"
+        - name: INBOX_LOCATION
+          value: "{{ .Values.global.inbox.path }}/"
+      {{- end }}
+      {{- if eq "s3" .Values.global.inbox.storageType }}
+        - name: INBOX_ACCESSKEY
+          valueFrom:
+            secretKeyRef:
+              name: {{ template "sda.fullname" . }}-s3inbox-keys
+              key: s3InboxAccessKey
+        - name: INBOX_SECRETKEY
+          valueFrom:
+            secretKeyRef:
+              name: {{ template "sda.fullname" . }}-s3inbox-keys
+              key: s3InboxSecretKey
+      {{- end }}
     {{- if .Values.global.log.format }}
         - name: LOG_FORMAT
           value: {{ .Values.global.log.format | quote }}
@@ -204,6 +238,10 @@ spec:
     {{- if .Values.global.tls.enabled }}
         - name: tls
           mountPath: {{ template "tlsPath" . }}
+    {{- end }}
+    {{- if eq "posix" .Values.global.inbox.storageType }}
+        - name: inbox
+          mountPath: {{ .Values.global.inbox.path | quote }}
     {{- end }}
       volumes:
     {{- if not .Values.global.vaultSecrets }}
@@ -238,4 +276,15 @@ spec:
             secretName: {{ required "An certificate issuer or a TLS secret name is required for api" .Values.api.tls.secretName }}
         {{- end }}
     {{- end }}
+      {{- if eq "posix" .Values.global.inbox.storageType }}
+        - name: inbox
+        {{- if .Values.global.inbox.existingClaim }}
+          persistentVolumeClaim:
+            claimName: {{ .Values.global.inbox.existingClaim }}
+        {{- else }}
+          nfs:
+            server: {{ required "An inbox NFS server is required" .Values.global.inbox.nfsServer | quote }}
+            path: {{ if .Values.global.inbox.nfsPath }}{{ .Values.global.inbox.nfsPath | quote }}{{ else }}{{ "/" }}{{ end }}
+        {{- end }}
+      {{- end }}
 {{- end }}

From f85e9a92bd19d35e5afef1d62c49914c82fd6370 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Thu, 21 Nov 2024 14:33:59 +0100
Subject: [PATCH 14/20] [api] add documentation

---
 sda/cmd/api/api.go |  2 +-
 sda/cmd/api/api.md | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index f972de131..4bd8bba69 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -324,7 +324,7 @@ func deleteFile(c *gin.Context) {
 	}
 
 	// Requires a filepath instead of fileID
-	// TODO: The remove fails randomly sometimes, maybe we should retry
+	// Note: The remove fails randomly sometimes
 	var RetryTimes = 5
 	for count := 1; count <= RetryTimes; count++ {
 		log.Warn("trying to remove file from inbox, try", count)
diff --git a/sda/cmd/api/api.md b/sda/cmd/api/api.md
index 321d9a6e4..5ef7aae8f 100644
--- a/sda/cmd/api/api.md
+++ b/sda/cmd/api/api.md
@@ -90,6 +90,24 @@ Admin endpoints are only available to a set of whitelisted users specified in th
     curl -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X PUT -d '{"accession_id": "my-id-01", "filepath": "/uploads/file.c4gh", "user": "testuser"}' https://HOSTNAME/file/accession
     ```
 
+- `/file/:username/*fileid`
+  - accepts `DELETE` requests
+  - marks the file as `disabled` in the database, and deletes it from the inbox.
+  - The file is identfied by its id, returned by `users/:username/:files`
+
+  - Response codes
+    - `200` Query execute ok.
+    - `400` File id not provided
+    - `401` Token user is not in the list of admins.
+    - `404` File not found
+    - `500` Internal error due to Inbox, DB or MQ failures.
+
+    Example:
+
+    ```bash
+    curl -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X POST -d '{"accession_ids": ["my-id-01", "my-id-02"], "dataset_id": "my-dataset-01"}' https://HOSTNAME/dataset/create
+    ```
+
 - `/dataset/create`
   - accepts `POST` requests with JSON data with the format: `{"accession_ids": ["<FILE_ACCESSION_01>", "<FILE_ACCESSION_02>"], "dataset_id": "<DATASET_01>", "user": "<SUBMISSION_USER>"}`
   - creates a dataset from the list of accession IDs and the dataset ID.

From 91ded8861fc2b1506f48bb5d76c28bf01718081d Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Wed, 27 Nov 2024 09:14:28 +0100
Subject: [PATCH 15/20] [api] typos etc

---
 .github/integration/tests/sda/60_api_admin_test.sh | 2 +-
 sda/cmd/api/api.go                                 | 1 -
 sda/cmd/api/api.md                                 | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/.github/integration/tests/sda/60_api_admin_test.sh b/.github/integration/tests/sda/60_api_admin_test.sh
index 83fda513e..09e5caf02 100644
--- a/.github/integration/tests/sda/60_api_admin_test.sh
+++ b/.github/integration/tests/sda/60_api_admin_test.sh
@@ -3,7 +3,7 @@ set -e
 cd shared || true
 
 token="$(curl http://oidc:8080/tokens | jq -r '.[0]')"
-# Upload a file and mke sure it's listed
+# Upload a file and make sure it's listed
 result="$(curl -sk -L "http://api:8080/users/test@dummy.org/files" -H "Authorization: Bearer $token" | jq '. | length')"
 if [ "$result" -ne 2 ]; then
     echo "wrong number of files returned for user test@dummy.org"
diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index 4bd8bba69..23920d779 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -306,7 +306,6 @@ func deleteFile(c *gin.Context) {
 	submissionUser := c.Param("username")
 	log.Warn("submission user:", submissionUser)
 
-	// TODO: Add check for the input file path
 	fileID := c.Param("file")
 	fileID = strings.TrimPrefix(fileID, "/")
 	log.Warn("submission file:", fileID)
diff --git a/sda/cmd/api/api.md b/sda/cmd/api/api.md
index 5ef7aae8f..c736e50b4 100644
--- a/sda/cmd/api/api.md
+++ b/sda/cmd/api/api.md
@@ -105,7 +105,7 @@ Admin endpoints are only available to a set of whitelisted users specified in th
     Example:
 
     ```bash
-    curl -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X POST -d '{"accession_ids": ["my-id-01", "my-id-02"], "dataset_id": "my-dataset-01"}' https://HOSTNAME/dataset/create
+    curl -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE https://HOSTNAME/file/user@demo.org/123abc
     ```
 
 - `/dataset/create`

From cfcfc273b28f935210f71df2bab205c156cf456d Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Fri, 29 Nov 2024 08:52:02 +0100
Subject: [PATCH 16/20] [api] rephrase comment

---
 sda/cmd/api/api.go | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index 23920d779..fa3ba2f7b 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -292,10 +292,8 @@ func ingestFile(c *gin.Context) {
 	c.Status(http.StatusOK)
 }
 
-// The deleteFile function will take the user id and the file id
-// because the user id won't be in the path of the file in the future
-// therefore we need to make the list files, get the file id and then
-// call the delete file
+// The deleteFile function deletes files from the inbox and marks them as
+// discarded in the db. Files are identified by their ids and the user id.
 func deleteFile(c *gin.Context) {
 
 	inbox, err := storage.NewBackend(Conf.Inbox)

From e93b9cce143def2b8cbeac17c11ac1dd5a1bd018 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Fri, 29 Nov 2024 10:21:31 +0100
Subject: [PATCH 17/20] [tests] add rbac permissions for file deletion endpoint

---
 .github/integration/sda/rbac.json | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.github/integration/sda/rbac.json b/.github/integration/sda/rbac.json
index a4a4ca394..70bb39ae5 100644
--- a/.github/integration/sda/rbac.json
+++ b/.github/integration/sda/rbac.json
@@ -30,6 +30,11 @@
           "path": "/file/accession",
           "action": "POST"
        },
+       {
+          "role": "admin",
+          "path": "/file/*",
+          "action": "(DELETE)"
+       },
        {
           "role": "submission",
           "path": "/users",

From bc41723adc145414341a31d9a7887da8a252f85c Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Mon, 2 Dec 2024 14:20:40 +0100
Subject: [PATCH 18/20] Apply suggestions from code review

---
 .github/integration/tests/sda/60_api_admin_test.sh | 10 ++++++++++
 sda/cmd/api/api.go                                 |  4 ++--
 sda/cmd/api/api.md                                 |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/.github/integration/tests/sda/60_api_admin_test.sh b/.github/integration/tests/sda/60_api_admin_test.sh
index 09e5caf02..71f9046f9 100644
--- a/.github/integration/tests/sda/60_api_admin_test.sh
+++ b/.github/integration/tests/sda/60_api_admin_test.sh
@@ -61,6 +61,11 @@ done
 # get the fileId of the new file
 fileid="$(curl -k -L -H "Authorization: Bearer $token" -H "Content-Type: application/json" "http://api:8080/users/test@dummy.org/files" | jq -r '.[] | select(.inboxPath == "test_dummy.org/NC12878.bam.c4gh") | .fileID')"
 
+output=$(s3cmd -c s3cfg ls s3://test_dummy.org/NC12878.bam.c4gh 2>/dev/null)
+if [ -z "$output" ] ; then
+    echo "Uploaded file not in inbox"
+    exit 1
+fi
 # delete it
 resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
 if [ "$resp" != "200" ]; then
@@ -74,6 +79,11 @@ if [ "$last_event" != "disabled" ]; then
    echo "The file $fileid does not have the expected las event 'disabled', but $last_event."
 fi
 
+output=$(s3cmd -c s3cfg ls s3://test_dummy.org/NC12878.bam.c4gh 2>/dev/null)
+if [ -n "$output" ] ; then
+    echo "Deleted file is still in inbox"
+    exit 1
+fi
 
 # Try to delete an unknown file
 resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/badfileid")"
diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index fa3ba2f7b..6e0302454 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -302,11 +302,11 @@ func deleteFile(c *gin.Context) {
 	}
 
 	submissionUser := c.Param("username")
-	log.Warn("submission user:", submissionUser)
+	log.Debug("submission user:", submissionUser)
 
 	fileID := c.Param("file")
 	fileID = strings.TrimPrefix(fileID, "/")
-	log.Warn("submission file:", fileID)
+	log.Debug("submission file:", fileID)
 	if fileID == "" {
 		c.AbortWithStatusJSON(http.StatusBadRequest, "file ID is required")
 	}
diff --git a/sda/cmd/api/api.md b/sda/cmd/api/api.md
index c736e50b4..a2b8bb087 100644
--- a/sda/cmd/api/api.md
+++ b/sda/cmd/api/api.md
@@ -93,7 +93,7 @@ Admin endpoints are only available to a set of whitelisted users specified in th
 - `/file/:username/*fileid`
   - accepts `DELETE` requests
   - marks the file as `disabled` in the database, and deletes it from the inbox.
-  - The file is identfied by its id, returned by `users/:username/:files`
+  - The file is identified by its id, returned by `users/:username/:files`
 
   - Response codes
     - `200` Query execute ok.

From 31b60b2a636e7f745fd80825badd7ef0539ce3c2 Mon Sep 17 00:00:00 2001
From: Malin Klang <ahlberg.malin@gmail.com>
Date: Thu, 5 Dec 2024 12:14:32 +0100
Subject: [PATCH 19/20] Apply suggestions from code review

Co-authored-by: Joakim Bygdell <joakim.bygdell@nbis.se>
---
 .github/integration/sda/rbac.json                  | 4 ++--
 .github/integration/tests/sda/60_api_admin_test.sh | 2 +-
 sda/cmd/api/api.go                                 | 8 +++-----
 sda/cmd/api/api.md                                 | 2 +-
 sda/internal/database/db_functions_test.go         | 1 -
 5 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/.github/integration/sda/rbac.json b/.github/integration/sda/rbac.json
index 70bb39ae5..a76d6c849 100644
--- a/.github/integration/sda/rbac.json
+++ b/.github/integration/sda/rbac.json
@@ -31,9 +31,9 @@
           "action": "POST"
        },
        {
-          "role": "admin",
+          "role": "submission",
           "path": "/file/*",
-          "action": "(DELETE)"
+          "action": "DELETE"
        },
        {
           "role": "submission",
diff --git a/.github/integration/tests/sda/60_api_admin_test.sh b/.github/integration/tests/sda/60_api_admin_test.sh
index 71f9046f9..811706728 100644
--- a/.github/integration/tests/sda/60_api_admin_test.sh
+++ b/.github/integration/tests/sda/60_api_admin_test.sh
@@ -67,7 +67,7 @@ if [ -z "$output" ] ; then
     exit 1
 fi
 # delete it
-resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
+resp="$(curl -s -k -L -o /dev/null -w "%{http_code}\n" -H "Authorization: Bearer $token" -X DELETE "http://api:8080/file/test@dummy.org/$fileid")"
 if [ "$resp" != "200" ]; then
     echo "Error when deleting the file, expected 200 got: $resp"
     exit 1
diff --git a/sda/cmd/api/api.go b/sda/cmd/api/api.go
index 6e0302454..f06a85496 100644
--- a/sda/cmd/api/api.go
+++ b/sda/cmd/api/api.go
@@ -106,7 +106,7 @@ func setup(config *config.Config) *http.Server {
 	r.POST("/c4gh-keys/add", rbac(e), addC4ghHash)                      // Adds a key hash to the database
 	r.GET("/c4gh-keys/list", rbac(e), listC4ghHashes)                   // Lists key hashes in the database
 	r.POST("/c4gh-keys/deprecate/*keyHash", rbac(e), deprecateC4ghHash) // Deprecate a given key hash
-	r.DELETE("/file/:username/*file", rbac(e), deleteFile)              // Delete a file from inbox
+	r.DELETE("/file/:username/:file", rbac(e), deleteFile)              // Delete a file from inbox
 	// submission endpoints below here
 	r.POST("/file/ingest", rbac(e), ingestFile)                  // start ingestion of a file
 	r.POST("/file/accession", rbac(e), setAccession)             // assign accession ID to a file
@@ -295,7 +295,6 @@ func ingestFile(c *gin.Context) {
 // The deleteFile function deletes files from the inbox and marks them as
 // discarded in the db. Files are identified by their ids and the user id.
 func deleteFile(c *gin.Context) {
-
 	inbox, err := storage.NewBackend(Conf.Inbox)
 	if err != nil {
 		log.Fatal(err)
@@ -311,9 +310,9 @@ func deleteFile(c *gin.Context) {
 		c.AbortWithStatusJSON(http.StatusBadRequest, "file ID is required")
 	}
 
-	filePath := ""
 	// Get the file path from the fileID and submission user
-	if filePath, err = Conf.API.DB.GetInboxFilePathFromID(submissionUser, fileID); err != nil {
+	filePath, err := Conf.API.DB.GetInboxFilePathFromID(submissionUser, fileID)
+	if err != nil {
 		log.Errorf("getting file from fileID failed, reason: (%v)", err)
 		c.AbortWithStatusJSON(http.StatusNotFound, "File could not be found in inbox")
 
@@ -321,7 +320,6 @@ func deleteFile(c *gin.Context) {
 	}
 
 	// Requires a filepath instead of fileID
-	// Note: The remove fails randomly sometimes
 	var RetryTimes = 5
 	for count := 1; count <= RetryTimes; count++ {
 		log.Warn("trying to remove file from inbox, try", count)
diff --git a/sda/cmd/api/api.md b/sda/cmd/api/api.md
index a2b8bb087..bc966da11 100644
--- a/sda/cmd/api/api.md
+++ b/sda/cmd/api/api.md
@@ -90,7 +90,7 @@ Admin endpoints are only available to a set of whitelisted users specified in th
     curl -H "Authorization: Bearer $token" -H "Content-Type: application/json" -X PUT -d '{"accession_id": "my-id-01", "filepath": "/uploads/file.c4gh", "user": "testuser"}' https://HOSTNAME/file/accession
     ```
 
-- `/file/:username/*fileid`
+- `/file/:username/:fileid`
   - accepts `DELETE` requests
   - marks the file as `disabled` in the database, and deletes it from the inbox.
   - The file is identified by its id, returned by `users/:username/:files`
diff --git a/sda/internal/database/db_functions_test.go b/sda/internal/database/db_functions_test.go
index 11134d9a4..53c2a7a8a 100644
--- a/sda/internal/database/db_functions_test.go
+++ b/sda/internal/database/db_functions_test.go
@@ -1189,7 +1189,6 @@ func (suite *DatabaseTests) TestGetDsatasetFiles() {
 }
 
 func (suite *DatabaseTests) TestGetInboxFilePathFromID() {
-
 	db, err := NewSDAdb(suite.dbConf)
 	assert.NoError(suite.T(), err, "got (%v) when creating new connection", err)
 

From 50367ef80661cd8183b98f51c494e5749d29c634 Mon Sep 17 00:00:00 2001
From: MalinAhlberg <ahlberg.malin@gmail.com>
Date: Thu, 5 Dec 2024 14:52:23 +0100
Subject: [PATCH 20/20] [api] only allow deletion of files marked as uploaded

---
 sda/internal/database/db_functions.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sda/internal/database/db_functions.go b/sda/internal/database/db_functions.go
index 0edac5bad..ea7a39344 100644
--- a/sda/internal/database/db_functions.go
+++ b/sda/internal/database/db_functions.go
@@ -88,7 +88,7 @@ func (dbs *SDAdb) getInboxFilePathFromID(submissionUser, fileID string) (string,
 		"submission_user= $1 and id = $2 " +
 		"AND EXISTS (SELECT 1 FROM " +
 		"(SELECT event from sda.file_event_log where file_id = $2 order by started_at desc limit 1) " +
-		"as subquery WHERE event in ('registered', 'uploaded', 'submitted', 'ingested', 'error'))"
+		"as subquery WHERE event = 'uploaded')"
 
 	var filePath string
 	err := db.QueryRow(getFilePath, submissionUser, fileID).Scan(&filePath)