From 59767281443d1b869e9da4747c2efe0bb582a68c Mon Sep 17 00:00:00 2001
From: "Ariel Shaqed (Scolnicov)" <ariels@treeverse.io>
Date: Tue, 17 Nov 2020 16:08:36 +0200
Subject: [PATCH] [CR] Validate nonempty access key ID, secrets in Swagger

Also roll back unrelated merged-in change to swagger.yml.
---
 api/api_controller_test.go | 58 +++++++++++++++++++++++++++-----------
 auth/setup.go              |  4 +--
 docs/assets/js/swagger.yml |  2 ++
 swagger.yml                |  2 ++
 4 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/api/api_controller_test.go b/api/api_controller_test.go
index f8c4d61295d..0a3ee278908 100644
--- a/api/api_controller_test.go
+++ b/api/api_controller_test.go
@@ -1446,8 +1446,9 @@ func TestHandler_ContinuousExportHandlers(t *testing.T) {
 func Test_setupLakeFSHandler(t *testing.T) {
 	name := "admin"
 	cases := []struct {
-		name string
-		user models.Setup
+		name               string
+		user               models.Setup
+		expectedStatusCode int
 	}{
 		{name: "simple", user: models.Setup{Username: &name}},
 		{
@@ -1460,6 +1461,23 @@ func Test_setupLakeFSHandler(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "emptyAccessKeyId",
+			user: models.Setup{
+				Username: &name,
+				Key:      &models.SetupKey{SecretAccessKey: swag.String("cetec astronomy")},
+			},
+			expectedStatusCode: 422,
+		},
+		{
+			name: "emptySecretKey", user: models.Setup{
+				Username: &name,
+				Key: &models.SetupKey{
+					AccessKeyID: swag.String("IKEAsneakers"),
+				},
+			},
+			expectedStatusCode: 422,
+		},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -1483,10 +1501,16 @@ func Test_setupLakeFSHandler(t *testing.T) {
 					_ = res.Body.Close()
 				}()
 
-				const expectedStatusCode = http.StatusOK
+				expectedStatusCode := http.StatusOK
+				if c.expectedStatusCode != 0 {
+					expectedStatusCode = c.expectedStatusCode
+				}
 				if res.StatusCode != expectedStatusCode {
 					t.Fatalf("setup request returned %d status, expected %d", res.StatusCode, expectedStatusCode)
 				}
+				if res.StatusCode != http.StatusOK {
+					return
+				}
 
 				// read response
 				var credKeys *models.CredentialsWithSecret
@@ -1520,19 +1544,21 @@ func Test_setupLakeFSHandler(t *testing.T) {
 				}
 			})
 
-			// now we ask again - should get status conflict
-			t.Run("existing setup", func(t *testing.T) {
-				// request to setup
-				res := mustSetup(t, reqURI, contentType, req)
-				defer func() {
-					_ = res.Body.Close()
-				}()
-
-				const expectedStatusCode = http.StatusConflict
-				if res.StatusCode != expectedStatusCode {
-					t.Fatalf("setup request returned %d status, expected %d", res.StatusCode, expectedStatusCode)
-				}
-			})
+			if c.expectedStatusCode == 0 {
+				// now we ask again - should get status conflict
+				t.Run("existing setup", func(t *testing.T) {
+					// request to setup
+					res := mustSetup(t, reqURI, contentType, req)
+					defer func() {
+						_ = res.Body.Close()
+					}()
+
+					const expectedStatusCode = http.StatusConflict
+					if res.StatusCode != expectedStatusCode {
+						t.Fatalf("setup request returned %d status, expected %d", res.StatusCode, expectedStatusCode)
+					}
+				})
+			}
 		})
 	}
 }
diff --git a/auth/setup.go b/auth/setup.go
index 92c243f6c68..5043d73a5b0 100644
--- a/auth/setup.go
+++ b/auth/setup.go
@@ -234,12 +234,12 @@ func AddAdminUser(authService Service, user *model.SuperuserConfiguration) (*mod
 		// Generate and return a key pair
 		creds, err = authService.CreateCredentials(user.Username)
 		if err != nil {
-			return nil, fmt.Errorf("create credentials for %s %w", user.Username, err)
+			return nil, fmt.Errorf("create credentials for %s: %w", user.Username, err)
 		}
 	} else {
 		creds, err = authService.AddCredentials(user.Username, user.AccessKeyID, user.SecretAccessKey)
 		if err != nil {
-			return nil, fmt.Errorf("add credentials for %s %w", user.Username, err)
+			return nil, fmt.Errorf("add credentials for %s: %w", user.Username, err)
 		}
 	}
 	return creds, nil
diff --git a/docs/assets/js/swagger.yml b/docs/assets/js/swagger.yml
index 18af13010c3..80bacfb50e8 100644
--- a/docs/assets/js/swagger.yml
+++ b/docs/assets/js/swagger.yml
@@ -244,10 +244,12 @@ definitions:
             description: access key ID to set for user for use in integration testing.
             example: AKIAIOSFODNN7EXAMPLE
             type: string
+            minLength: 1
           secret_access_key:
             description: secret access key to set for user for use in integration testing.
             example: wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY
             type: string
+            minLength: 1
         required:
           - access_key_id
           - secret_access_key
diff --git a/swagger.yml b/swagger.yml
index 18af13010c3..80bacfb50e8 100644
--- a/swagger.yml
+++ b/swagger.yml
@@ -244,10 +244,12 @@ definitions:
             description: access key ID to set for user for use in integration testing.
             example: AKIAIOSFODNN7EXAMPLE
             type: string
+            minLength: 1
           secret_access_key:
             description: secret access key to set for user for use in integration testing.
             example: wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY
             type: string
+            minLength: 1
         required:
           - access_key_id
           - secret_access_key