Adds the creator permission level, migration and updates the model and tests around them

This commit is contained in:
Miguel de la Cruz
2026-08-26 17:02:10 +02:00
parent c3ddf0f6cc
commit 9931c7fccb
7 changed files with 232 additions and 0 deletions
+20
View File
@@ -631,6 +631,26 @@ func TestCreatePropertyField(t *testing.T) {
})
})
t.Run("permission level=creator on a user-object field should fail", func(t *testing.T) {
// User-object fields have no entity creator, so the level is rejected
// rather than silently resolving to admin.
creatorLevel := model.PermissionLevelCreator
memberLevel := model.PermissionLevelMember
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
TargetType: "channel",
TargetID: th.BasicChannel.Id,
PermissionField: &memberLevel,
PermissionValues: &creatorLevel,
PermissionOptions: &memberLevel,
}
_, resp, err := th.SystemAdminClient.CreatePropertyField(context.Background(), group.Name, "user", field)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
})
t.Run("invalid group name should fail", func(t *testing.T) {
th.LoginBasic(t)
@@ -99,4 +99,36 @@ func TestCanonicalizeSystemObjectField(t *testing.T) {
CanonicalizeSystemObjectField(nil)
})
})
t.Run("creator is normalized to sysadmin, not rejected", func(t *testing.T) {
// System-object fields have no creator, so PropertyField.IsValid
// rejects PermissionLevelCreator on them. But canonicalization runs
// before validation on every write path (the API handler and
// App.CreatePropertyField), so a system field submitted with creator is
// silently pinned to sysadmin and never reaches the rejection. This
// test pins that precedence: if canonicalization ever moves after
// validation, the IsValid call below starts failing.
creator := model.PermissionLevelCreator
f := &model.PropertyField{
ID: model.NewId(),
GroupID: model.NewId(),
Name: "system field",
Type: model.PropertyFieldTypeText,
ObjectType: model.PropertyFieldObjectTypeSystem,
TargetType: "channel",
TargetID: "ch1",
PermissionField: &creator,
PermissionValues: &creator,
PermissionOptions: &creator,
CreateAt: model.GetMillis(),
UpdateAt: model.GetMillis(),
}
CanonicalizeSystemObjectField(f)
assert.Equal(t, model.PermissionLevelSysadmin, *f.PermissionField)
assert.Equal(t, model.PermissionLevelSysadmin, *f.PermissionValues)
assert.Equal(t, model.PermissionLevelSysadmin, *f.PermissionOptions)
assert.NoError(t, f.IsValid())
})
}
@@ -427,3 +427,5 @@ channels/db/migrations/postgres/000215_drop_channelmembers_autotranslation_colum
channels/db/migrations/postgres/000215_drop_channelmembers_autotranslation_column.up.sql
channels/db/migrations/postgres/000216_split_attribute_view_by_object_type.down.sql
channels/db/migrations/postgres/000216_split_attribute_view_by_object_type.up.sql
channels/db/migrations/postgres/000217_add_creator_to_permission_level.down.sql
channels/db/migrations/postgres/000217_add_creator_to_permission_level.up.sql
@@ -0,0 +1,18 @@
-- Postgres cannot remove a value from an existing enum in place, so rebuild the
-- type without 'creator'. Any rows currently holding 'creator' are coerced to
-- NULL first so the recreated enum can accept them.
UPDATE PropertyFields SET PermissionField = NULL WHERE PermissionField = 'creator';
UPDATE PropertyFields SET PermissionValues = NULL WHERE PermissionValues = 'creator';
UPDATE PropertyFields SET PermissionOptions = NULL WHERE PermissionOptions = 'creator';
ALTER TYPE permission_level RENAME TO permission_level_old;
CREATE TYPE permission_level AS ENUM ('none', 'sysadmin', 'member', 'admin');
ALTER TABLE PropertyFields
ALTER COLUMN PermissionField TYPE permission_level USING PermissionField::text::permission_level,
ALTER COLUMN PermissionValues TYPE permission_level USING PermissionValues::text::permission_level,
ALTER COLUMN PermissionOptions TYPE permission_level USING PermissionOptions::text::permission_level;
DROP TYPE permission_level_old;
@@ -0,0 +1 @@
ALTER TYPE permission_level ADD VALUE IF NOT EXISTS 'creator';
+47
View File
@@ -47,6 +47,16 @@ const (
// channel targets. The specific permission checked per scope is documented
// at hasPropertyFieldPermissionLevel in the app package.
PermissionLevelAdmin PermissionLevel = "admin"
// PermissionLevelCreator grants access to the creator of the entity the
// action concerns, plus everyone PermissionLevelAdmin grants. For values
// the entity is the value's target object: the post's UserId for
// post-object fields, the channel's CreatorId for channel-object fields.
// For the field and options slots the entity is the field itself, so the
// creator is its CreatedBy. Only valid on post- and channel-object fields;
// see IsValid. The specific checks per scope are documented at
// hasPropertyFieldPermissionLevel and hasPropertyFieldValuePermissionLevel
// in the app package.
PermissionLevelCreator PermissionLevel = "creator"
PropertyFieldObjectTypePost = "post"
PropertyFieldObjectTypeChannel = "channel"
@@ -63,6 +73,7 @@ var validPermissionLevels = []PermissionLevel{
PermissionLevelSysadmin,
PermissionLevelMember,
PermissionLevelAdmin,
PermissionLevelCreator,
}
// validPSAv2TargetTypes contains all valid TargetType values for PSAv2 properties.
@@ -98,6 +109,21 @@ func (t PropertyFieldType) SupportsOptions() bool {
return slices.Contains(optionFieldTypes, t)
}
// creatorPermissionObjectTypes are the object types whose target entity has an
// established creator, and therefore the only ones on which
// PermissionLevelCreator is meaningful. Fields of any other object type reject
// the level at validation rather than silently aliasing it to admin.
var creatorPermissionObjectTypes = []string{
PropertyFieldObjectTypePost,
PropertyFieldObjectTypeChannel,
}
// SupportsCreatorPermissionLevel reports whether a field's object type can
// carry PermissionLevelCreator in any of its permission slots.
func (pf *PropertyField) SupportsCreatorPermissionLevel() bool {
return slices.Contains(creatorPermissionObjectTypes, pf.ObjectType)
}
type PropertyField struct {
ID string `json:"id"`
GroupID string `json:"group_id"`
@@ -312,6 +338,27 @@ func (pf *PropertyField) IsValid() error {
return NewAppError("PropertyField.IsValid", "model.property_field.is_valid.app_error", map[string]any{"FieldName": "permission_options", "Reason": "invalid permission level"}, "id="+pf.ID, http.StatusBadRequest)
}
// PermissionLevelCreator requires an object type with an established
// creator.
//
// Runs after the membership checks above so an unrecognized level still
// reports "invalid permission level", and before the protected rules below
// so a protected field is rejected on the protected/none rule instead.
if !pf.SupportsCreatorPermissionLevel() {
for _, slot := range []struct {
name string
level *PermissionLevel
}{
{"permission_field", pf.PermissionField},
{"permission_values", pf.PermissionValues},
{"permission_options", pf.PermissionOptions},
} {
if slot.level != nil && *slot.level == PermissionLevelCreator {
return NewAppError("PropertyField.IsValid", "model.property_field.is_valid.app_error", map[string]any{"FieldName": slot.name, "Reason": "creator permission level requires a post or channel object type"}, "id="+pf.ID, http.StatusBadRequest)
}
}
}
// Cross-validation: protected fields must have field permission set to "none"
if pf.Protected {
if pf.PermissionField == nil {
+112
View File
@@ -831,6 +831,118 @@ func TestPropertyField_IsValid(t *testing.T) {
require.Error(t, pf.IsValid())
})
})
t.Run("creator permission level", func(t *testing.T) {
baseField := func(objectType string) *PropertyField {
return &PropertyField{
ID: NewId(),
GroupID: NewId(),
Name: "test field",
Type: PropertyFieldTypeText,
ObjectType: objectType,
TargetType: string(PropertyFieldTargetLevelChannel),
TargetID: NewId(),
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
}
// requireRejectedForSlot asserts IsValid rejected the field over the
// creator level and named the offending slot. Checking the name matters:
// a rule that always reported permission_field would pass a bare
// require.Error.
requireRejectedForSlot := func(t *testing.T, pf *PropertyField, slotName string) {
t.Helper()
err := pf.IsValid()
require.Error(t, err)
appErr, ok := err.(*AppError)
require.True(t, ok)
require.Equal(t, slotName, appErr.params["FieldName"])
require.Contains(t, appErr.params["Reason"].(string), "creator")
}
for _, objectType := range []string{PropertyFieldObjectTypePost, PropertyFieldObjectTypeChannel} {
t.Run("valid on "+objectType+" object type", func(t *testing.T) {
t.Run("permission_field", func(t *testing.T) {
pf := baseField(objectType)
pf.PermissionField = new(PermissionLevelCreator)
require.NoError(t, pf.IsValid())
})
t.Run("permission_values", func(t *testing.T) {
pf := baseField(objectType)
pf.PermissionValues = new(PermissionLevelCreator)
require.NoError(t, pf.IsValid())
})
t.Run("permission_options", func(t *testing.T) {
pf := baseField(objectType)
pf.PermissionOptions = new(PermissionLevelCreator)
require.NoError(t, pf.IsValid())
})
})
}
// user/session/template have no entity creator, so creator is rejected
// rather than silently aliased to admin. (system is canonicalized to
// sysadmin before validation — see TestCanonicalizeSystemObjectField.)
for _, objectType := range []string{
PropertyFieldObjectTypeUser,
PropertyFieldObjectTypeSession,
PropertyFieldObjectTypeTemplate,
} {
t.Run("rejected on "+objectType+" object type", func(t *testing.T) {
t.Run("permission_field", func(t *testing.T) {
pf := baseField(objectType)
pf.PermissionField = new(PermissionLevelCreator)
requireRejectedForSlot(t, pf, "permission_field")
})
t.Run("permission_values", func(t *testing.T) {
pf := baseField(objectType)
pf.PermissionValues = new(PermissionLevelCreator)
requireRejectedForSlot(t, pf, "permission_values")
})
t.Run("permission_options", func(t *testing.T) {
pf := baseField(objectType)
pf.PermissionOptions = new(PermissionLevelCreator)
requireRejectedForSlot(t, pf, "permission_options")
})
})
}
t.Run("protected field rejects creator on the protected rule, not the creator rule", func(t *testing.T) {
// Protected fields must have permission_field=none, and that check
// must win: otherwise the operator gets a confusing message about
// object types when the real problem is the protected flag.
pf := baseField(PropertyFieldObjectTypePost)
pf.Protected = true
pf.PermissionField = new(PermissionLevelCreator)
err := pf.IsValid()
require.Error(t, err)
appErr, ok := err.(*AppError)
require.True(t, ok)
require.Equal(t, "permission_field", appErr.params["FieldName"])
require.Contains(t, appErr.params["Reason"].(string), "none")
})
t.Run("unknown level is reported as an invalid level, not a creator problem", func(t *testing.T) {
// The membership check must run before the creator object-type
// check, so a typo'd level gets the generic message.
pf := baseField(PropertyFieldObjectTypeUser)
pf.PermissionValues = new(PermissionLevel("owner"))
err := pf.IsValid()
require.Error(t, err)
appErr, ok := err.(*AppError)
require.True(t, ok)
require.Equal(t, "invalid permission level", appErr.params["Reason"])
})
})
}
func TestPropertyFieldPatch_IsValid(t *testing.T) {