From c951e5fa4bd57a19dafba8c6077eceb6d42251e4 Mon Sep 17 00:00:00 2001 From: Noooste <83548733+Noooste@users.noreply.github.com> Date: Sun, 19 Apr 2026 23:15:27 +0200 Subject: [PATCH] feat: implement permission denial functionality and enhance permission saving state in AccessControl component Signed-off-by: Noooste <83548733+Noooste@users.noreply.github.com> --- backend/internal/handlers/buckets.go | 67 ++++++++++++++----- backend/internal/services/interfaces.go | 1 + backend/internal/services/mocks/admin_mock.go | 9 +++ backend/internal/services/mocks/mocks_test.go | 1 + frontend/src/pages/AccessControl.tsx | 9 ++- 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/backend/internal/handlers/buckets.go b/backend/internal/handlers/buckets.go index 1765594..8963bce 100644 --- a/backend/internal/handlers/buckets.go +++ b/backend/internal/handlers/buckets.go @@ -287,23 +287,60 @@ func (h *BucketHandler) GrantBucketPermission(c fiber.Ctx) error { ) } - // Build the permission request for Garage Admin API - permRequest := models.BucketKeyPermRequest{ - BucketID: bucketInfo.ID, - AccessKeyID: req.AccessKeyID, - Permissions: models.BucketKeyPermission{ - Read: req.Permissions.Read, - Write: req.Permissions.Write, - Owner: req.Permissions.Owner, - }, + // Garage's AllowBucketKey is additive — false values are no-ops, not revokes. + // To make this endpoint a true "set permissions" operation, split into Allow + // for the requested-true perms and Deny for the requested-false perms. + allow := models.BucketKeyPermission{ + Read: req.Permissions.Read, + Write: req.Permissions.Write, + Owner: req.Permissions.Owner, + } + deny := models.BucketKeyPermission{ + Read: !req.Permissions.Read, + Write: !req.Permissions.Write, + Owner: !req.Permissions.Owner, } - // Grant permissions using Garage Admin API - result, err := h.adminService.AllowBucketKey(ctx, permRequest) - if err != nil { - return c.Status(fiber.StatusInternalServerError).JSON( - models.ErrorResponse(models.ErrCodeInternalError, "Failed to grant permissions: "+err.Error()), - ) + var result *models.GarageBucketInfo + + if allow.Read || allow.Write || allow.Owner { + r, err := h.adminService.AllowBucketKey(ctx, models.BucketKeyPermRequest{ + BucketID: bucketInfo.ID, + AccessKeyID: req.AccessKeyID, + Permissions: allow, + }) + if err != nil { + return c.Status(fiber.StatusInternalServerError).JSON( + models.ErrorResponse(models.ErrCodeInternalError, "Failed to grant permissions: "+err.Error()), + ) + } + result = r + } + + if deny.Read || deny.Write || deny.Owner { + r, err := h.adminService.DenyBucketKey(ctx, models.BucketKeyPermRequest{ + BucketID: bucketInfo.ID, + AccessKeyID: req.AccessKeyID, + Permissions: deny, + }) + if err != nil { + return c.Status(fiber.StatusInternalServerError).JSON( + models.ErrorResponse(models.ErrCodeInternalError, "Failed to revoke permissions: "+err.Error()), + ) + } + result = r + } + + if result == nil { + // Caller passed all-false on a key with no existing perms — nothing to do. + // Fetch current bucket state to return a consistent response. + r, err := h.adminService.GetBucketInfo(ctx, bucketInfo.ID) + if err != nil { + return c.Status(fiber.StatusInternalServerError).JSON( + models.ErrorResponse(models.ErrCodeInternalError, "Failed to fetch bucket info: "+err.Error()), + ) + } + result = r } return c.JSON(models.SuccessResponse(result)) diff --git a/backend/internal/services/interfaces.go b/backend/internal/services/interfaces.go index 8785369..974506f 100644 --- a/backend/internal/services/interfaces.go +++ b/backend/internal/services/interfaces.go @@ -28,6 +28,7 @@ type AdminService interface { UpdateBucket(ctx context.Context, bucketID string, req models.UpdateBucketRequest) (*models.GarageBucketInfo, error) DeleteBucket(ctx context.Context, bucketID string) error AllowBucketKey(ctx context.Context, req models.BucketKeyPermRequest) (*models.GarageBucketInfo, error) + DenyBucketKey(ctx context.Context, req models.BucketKeyPermRequest) (*models.GarageBucketInfo, error) // Cluster GetClusterHealth(ctx context.Context) (*models.ClusterHealth, error) diff --git a/backend/internal/services/mocks/admin_mock.go b/backend/internal/services/mocks/admin_mock.go index 3e05b16..c7bd641 100644 --- a/backend/internal/services/mocks/admin_mock.go +++ b/backend/internal/services/mocks/admin_mock.go @@ -41,6 +41,7 @@ type AdminMock struct { UpdateBucketFn func(ctx context.Context, bucketID string, req models.UpdateBucketRequest) (*models.GarageBucketInfo, error) DeleteBucketFn func(ctx context.Context, bucketID string) error AllowBucketKeyFn func(ctx context.Context, req models.BucketKeyPermRequest) (*models.GarageBucketInfo, error) + DenyBucketKeyFn func(ctx context.Context, req models.BucketKeyPermRequest) (*models.GarageBucketInfo, error) // Cluster GetClusterHealthFn func(ctx context.Context) (*models.ClusterHealth, error) @@ -168,6 +169,14 @@ func (m *AdminMock) AllowBucketKey(ctx context.Context, req models.BucketKeyPerm return m.AllowBucketKeyFn(ctx, req) } +func (m *AdminMock) DenyBucketKey(ctx context.Context, req models.BucketKeyPermRequest) (*models.GarageBucketInfo, error) { + m.record("DenyBucketKey", req) + if m.DenyBucketKeyFn == nil { + return nil, errNotConfigured("DenyBucketKey") + } + return m.DenyBucketKeyFn(ctx, req) +} + // --- Cluster --- func (m *AdminMock) GetClusterHealth(ctx context.Context) (*models.ClusterHealth, error) { diff --git a/backend/internal/services/mocks/mocks_test.go b/backend/internal/services/mocks/mocks_test.go index 5f90b16..db34ecc 100644 --- a/backend/internal/services/mocks/mocks_test.go +++ b/backend/internal/services/mocks/mocks_test.go @@ -38,6 +38,7 @@ func TestAdminMock_UnconfiguredMethodsReturnSentinel(t *testing.T) { {"UpdateBucket", func() error { _, e := m.UpdateBucket(ctx, "b", models.UpdateBucketRequest{}); return e }}, {"DeleteBucket", func() error { return m.DeleteBucket(ctx, "b") }}, {"AllowBucketKey", func() error { _, e := m.AllowBucketKey(ctx, models.BucketKeyPermRequest{}); return e }}, + {"DenyBucketKey", func() error { _, e := m.DenyBucketKey(ctx, models.BucketKeyPermRequest{}); return e }}, {"GetClusterHealth", func() error { _, e := m.GetClusterHealth(ctx); return e }}, {"GetClusterStatus", func() error { _, e := m.GetClusterStatus(ctx); return e }}, {"GetClusterStatistics", func() error { _, e := m.GetClusterStatistics(ctx); return e }}, diff --git a/frontend/src/pages/AccessControl.tsx b/frontend/src/pages/AccessControl.tsx index 2bed898..2b1df53 100644 --- a/frontend/src/pages/AccessControl.tsx +++ b/frontend/src/pages/AccessControl.tsx @@ -132,6 +132,7 @@ export function AccessControl() { const [permissionRead, setPermissionRead] = useState(false); const [permissionWrite, setPermissionWrite] = useState(false); const [permissionOwner, setPermissionOwner] = useState(false); + const [savingPermissions, setSavingPermissions] = useState(false); // Key settings state (activation/expiration) const [settingsDialogOpen, setSettingsDialogOpen] = useState(false); @@ -384,6 +385,7 @@ export function AccessControl() { return; } + setSavingPermissions(true); try { // Call backend API to grant bucket permissions await bucketsApi.grantPermission(selectedBucket, editingKey.accessKeyId, { @@ -405,6 +407,8 @@ export function AccessControl() { } catch (error) { // Error toast is handled by API interceptor console.error('Grant permission error:', error); + } finally { + setSavingPermissions(false); } }; @@ -1176,8 +1180,9 @@ export function AccessControl() { -