diff --git a/internal/controller/api_service_accounts.go b/internal/controller/api_service_accounts.go index 346ed77..10479de 100644 --- a/internal/controller/api_service_accounts.go +++ b/internal/controller/api_service_accounts.go @@ -82,14 +82,24 @@ 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 userServiceAccount.Name == "" { + if name == "" { return responder.JSON(http.StatusPreconditionFailed, NewErrorResponse("service account name is empty")) - } else if err := simplename.Validate(userServiceAccount.Name); err != nil { + } 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")) + } + if userServiceAccount.Name != name { + return responder.JSON(http.StatusPreconditionFailed, + NewErrorResponse("service account name does not match URL path")) + } // Validate roles for _, role := range userServiceAccount.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) } diff --git a/internal/controller/api_service_accounts_test.go b/internal/controller/api_service_accounts_test.go new file mode 100644 index 0000000..47537d5 --- /dev/null +++ b/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 +}