Merge 92efb01366 into 461af8de47

internal/controller/api_service_accounts.go

@@ -82,13 +82,23 @@ func (controller *Controller) updateServiceAccount(ctx *gin.Context) responder.R
 		return responder.JSON(http.StatusBadRequest, NewErrorResponse("invalid JSON was provided"))
 	}
 
+	name := ctx.Param("name")
+
 	// Validate service account name
+	if name == "" {
+		return responder.JSON(http.StatusPreconditionFailed,
+			NewErrorResponse("service account name is empty"))
+	} else if err := simplename.Validate(name); err != nil {
+		return responder.JSON(http.StatusPreconditionFailed,
+			NewErrorResponse("service account %v", err))
+	}
 	if userServiceAccount.Name == "" {
 		return responder.JSON(http.StatusPreconditionFailed,
 			NewErrorResponse("service account name is empty"))
-	} else if err := simplename.Validate(userServiceAccount.Name); err != nil {
+	}
+	if userServiceAccount.Name != name {
 		return responder.JSON(http.StatusPreconditionFailed,
-			NewErrorResponse("service account %v", err))
+			NewErrorResponse("service account name does not match URL path"))
 	}
 
 	// Validate roles
@@ -105,7 +115,7 @@ func (controller *Controller) updateServiceAccount(ctx *gin.Context) responder.R
 	}
 
 	return controller.storeUpdate(func(txn storepkg.Transaction) responder.Responder {
-		dbServiceAccount, err := txn.GetServiceAccount(userServiceAccount.Name)
+		dbServiceAccount, err := txn.GetServiceAccount(name)
 		if err != nil {
 			return responder.Error(err)
 		}

internal/controller/api_service_accounts_test.go

@@ -0,0 +1,116 @@
+//nolint:testpackage // we need to test unexported API handlers
+package controller
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	storepkg "github.com/cirruslabs/orchard/internal/controller/store"
+	"github.com/cirruslabs/orchard/internal/controller/store/badger"
+	v1pkg "github.com/cirruslabs/orchard/pkg/resource/v1"
+	"github.com/gin-gonic/gin"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func TestUpdateServiceAccountRejectsBodyNameMismatch(t *testing.T) {
+	controller := newServiceAccountTestController(t)
+
+	require.NoError(t, controller.EnsureServiceAccount(&v1pkg.ServiceAccount{
+		Meta:  v1pkg.Meta{Name: "alice"},
+		Token: "alice-token",
+		Roles: []v1pkg.ServiceAccountRole{v1pkg.ServiceAccountRoleAdminRead},
+	}))
+	require.NoError(t, controller.EnsureServiceAccount(&v1pkg.ServiceAccount{
+		Meta:  v1pkg.Meta{Name: "bob"},
+		Token: "bob-token",
+		Roles: []v1pkg.ServiceAccountRole{v1pkg.ServiceAccountRoleAdminRead},
+	}))
+
+	ctx, recorder := serviceAccountUpdateContext(t, "alice", v1pkg.ServiceAccount{
+		Meta:  v1pkg.Meta{Name: "bob"},
+		Token: "new-token",
+		Roles: []v1pkg.ServiceAccountRole{v1pkg.ServiceAccountRoleAdminWrite},
+	})
+
+	controller.updateServiceAccount(ctx).Respond(ctx)
+
+	require.Equal(t, http.StatusPreconditionFailed, recorder.Code)
+	require.Equal(t, "alice-token", getServiceAccount(t, controller, "alice").Token)
+	require.Equal(t, "bob-token", getServiceAccount(t, controller, "bob").Token)
+}
+
+func TestUpdateServiceAccountUsesPathName(t *testing.T) {
+	controller := newServiceAccountTestController(t)
+
+	require.NoError(t, controller.EnsureServiceAccount(&v1pkg.ServiceAccount{
+		Meta:  v1pkg.Meta{Name: "alice"},
+		Token: "old-token",
+		Roles: []v1pkg.ServiceAccountRole{v1pkg.ServiceAccountRoleAdminRead},
+	}))
+
+	ctx, recorder := serviceAccountUpdateContext(t, "alice", v1pkg.ServiceAccount{
+		Meta:  v1pkg.Meta{Name: "alice"},
+		Token: "new-token",
+		Roles: []v1pkg.ServiceAccountRole{v1pkg.ServiceAccountRoleAdminWrite},
+	})
+
+	controller.updateServiceAccount(ctx).Respond(ctx)
+
+	require.Equal(t, http.StatusOK, recorder.Code)
+
+	serviceAccount := getServiceAccount(t, controller, "alice")
+	require.Equal(t, "new-token", serviceAccount.Token)
+	require.Equal(t, []v1pkg.ServiceAccountRole{v1pkg.ServiceAccountRoleAdminWrite}, serviceAccount.Roles)
+}
+
+func newServiceAccountTestController(t *testing.T) *Controller {
+	t.Helper()
+
+	logger := zap.NewNop().Sugar()
+	store, err := badger.NewBadgerStore(t.TempDir(), true, logger)
+	require.NoError(t, err)
+
+	return &Controller{
+		store:                store,
+		logger:               logger,
+		insecureAuthDisabled: true,
+	}
+}
+
+func serviceAccountUpdateContext(
+	t *testing.T,
+	name string,
+	serviceAccount v1pkg.ServiceAccount,
+) (*gin.Context, *httptest.ResponseRecorder) {
+	t.Helper()
+
+	body, err := json.Marshal(serviceAccount)
+	require.NoError(t, err)
+
+	recorder := httptest.NewRecorder()
+	ctx, _ := gin.CreateTestContext(recorder)
+	ctx.Params = gin.Params{{Key: "name", Value: name}}
+	ctx.Request = httptest.NewRequest(http.MethodPut, "/service-accounts/"+name, bytes.NewReader(body))
+	ctx.Request.Header.Set("Content-Type", "application/json")
+
+	return ctx, recorder
+}
+
+func getServiceAccount(t *testing.T, controller *Controller, name string) *v1pkg.ServiceAccount {
+	t.Helper()
+
+	var serviceAccount *v1pkg.ServiceAccount
+	err := controller.store.View(func(txn storepkg.Transaction) error {
+		var err error
+		serviceAccount, err = txn.GetServiceAccount(name)
+
+		return err
+	})
+	require.NoError(t, err)
+
+	return serviceAccount
+}