Merge pull request #60 from tranchitella/filetransfer-download-get

Update file transfer download end-point to use GET instead of POST
mendersoftware · Mar 12, 2021 · 922ca6c · 922ca6c
2 parents 4964338 + 4ce84be
commit 922ca6c
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 40 deletions.
diff --git a/api/http/management_filetransfer.go b/api/http/management_filetransfer.go
@@ -63,6 +63,8 @@ const (
 	fieldUploadFile = "file"
 
 	PropertyOffset = "offset"
+
+	paramDownloadPath = "path"
 )
 
 var fileTransferPingInterval = 30 * time.Second
@@ -106,7 +108,7 @@ func (h ManagementController) getFileTransferParams(c *gin.Context) (*fileTransf
 		return nil, http.StatusConflict, app.ErrDeviceNotConnected
 	}
 
-	if c.Request.Body == nil {
+	if c.Request.Method != http.MethodGet && c.Request.Body == nil {
 		return nil, http.StatusBadRequest, errors.New("missing request body")
 	}
 
@@ -451,13 +453,9 @@ func (h ManagementController) DownloadFile(c *gin.Context) {
 		return
 	}
 
-	request := &model.DownloadFileRequest{}
-	if err := c.ShouldBindJSON(request); err != nil {
-		l.Error(err)
-		c.JSON(http.StatusBadRequest, gin.H{
-			"error": errors.Wrap(err, "invalid request body").Error(),
-		})
-		return
+	path := c.Request.URL.Query().Get(paramDownloadPath)
+	request := &model.DownloadFileRequest{
+		Path: &path,
 	}
 
 	if err := request.Validate(); err != nil {

diff --git a/api/http/management_filetransfer_test.go b/api/http/management_filetransfer_test.go
@@ -21,6 +21,7 @@ import (
 	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"strings"
 	"testing"
 	"time"
@@ -78,7 +79,7 @@ func TestManagementDownloadFile(t *testing.T) {
 	testCases := []struct {
 		Name     string
 		DeviceID string
-		Body     []byte
+		Path     string
 		Identity *identity.Identity
 
 		RBACGroups  string
@@ -102,7 +103,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -225,7 +226,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -358,7 +359,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -445,7 +446,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -566,7 +567,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -635,7 +636,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -738,7 +739,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -857,7 +858,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -924,7 +925,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -943,7 +944,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			RBACGroups: "group1,group",
 			RBACError:  errors.New("error"),
@@ -963,7 +964,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "/absolute/path"}`),
+			Path: "/absolute/path",
 
 			RBACGroups:  "group1,group",
 			RBACAllowed: false,
@@ -983,7 +984,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte(`{"path": "relative/path"}`),
+			Path: "relative/path",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -1000,7 +1001,7 @@ func TestManagementDownloadFile(t *testing.T) {
 				Tenant:  "000000000000000000000000",
 				IsUser:  true,
 			},
-			Body: []byte("dummy"),
+			Path: "",
 
 			GetDevice: &model.Device{
 				ID:     "1234567890",
@@ -1113,12 +1114,9 @@ func TestManagementDownloadFile(t *testing.T) {
 			s := httptest.NewServer(router)
 			defer s.Close()
 
+			path := url.QueryEscape(tc.Path)
 			url := strings.Replace(APIURLManagementDeviceDownload, ":deviceId", tc.DeviceID, 1)
-			var body io.Reader
-			if tc.Body != nil {
-				body = bytes.NewReader(tc.Body)
-			}
-			req, err := http.NewRequest(http.MethodPost, "http://localhost"+url, body)
+			req, err := http.NewRequest(http.MethodGet, "http://localhost"+url+"?path="+path, nil)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}

diff --git a/api/http/router.go b/api/http/router.go
@@ -118,7 +118,7 @@ func NewRouter(
 	management := NewManagementController(app, natsClient)
 	router.GET(APIURLManagementDevice, management.GetDevice)
 	router.GET(APIURLManagementDeviceConnect, management.Connect)
-	router.POST(APIURLManagementDeviceDownload, management.DownloadFile)
+	router.GET(APIURLManagementDeviceDownload, management.DownloadFile)
 	router.POST(APIURLManagementDeviceCheckUpdate, management.CheckUpdate)
 	router.POST(APIURLManagementDeviceSendInventory, management.SendInventory)
 	router.PUT(APIURLManagementDeviceUpload, management.UploadFile)

diff --git a/docs/management_api.yml b/docs/management_api.yml
@@ -158,7 +158,7 @@ paths:
           $ref: '#/components/responses/InternalServerError'
 
   /devices/{id}/download:
-    post:
+    get:
       tags:
         - ManagementAPI
       operationId: Download
@@ -171,17 +171,12 @@ paths:
             type: string
             format: uuid
           description: ID of the device.
-      requestBody:
-        content:
-          'application/json':
-            schema:
-              type: object
-              properties:
-                path:
-                  type: string
-                  description: Path of the file on the device.
-              required:
-                - path
+        - in: query
+          name: path
+          required: true
+          schema:
+            type: string
+          description: Path of the file on the device.
       responses:
         200:
           description: The content of the file will be returned in the response body