diff --git a/api/v4/source/custom_profile_attributes.yaml b/api/v4/source/custom_profile_attributes.yaml index 589ad6bdc39..8e9087c9707 100644 --- a/api/v4/source/custom_profile_attributes.yaml +++ b/api/v4/source/custom_profile_attributes.yaml @@ -83,6 +83,17 @@ saml: type: string description: "SAML attribute for syncing" + protected: + type: boolean + description: "If true, the field is read-only and cannot be modified." + source_plugin_id: + type: string + description: "The ID of the plugin that created this field. This attribute cannot be changed." + access_mode: + type: string + description: "Access mode of the field" + enum: ["", "source_only", "shared_only"] + default: "" responses: "201": description: Custom Profile Attribute field creation successful @@ -108,6 +119,9 @@ updated. The fields that can be updated are defined in the request body, all other provided fields will be ignored. + **Note:** Fields with `attrs.protected = true` cannot be + modified and will return an error. + _This endpoint is experimental._ __Minimum server version__: 10.5 @@ -167,6 +181,17 @@ saml: type: string description: "SAML attribute for syncing" + protected: + type: boolean + description: "If true, the field is read-only and cannot be modified." + source_plugin_id: + type: string + description: "The ID of the plugin that created this field. This attribute cannot be changed." + access_mode: + type: string + description: "Access mode of the field" + enum: ["", "source_only", "shared_only"] + default: "" responses: "200": description: Custom Profile Attribute field patch successful @@ -229,6 +254,9 @@ that can be updated are defined in the request body, all other provided fields will be ignored. + **Note:** Values for fields with `attrs.protected = true` cannot be + updated and will return an error. + _This endpoint is experimental._ __Minimum server version__: 10.5 @@ -356,6 +384,9 @@ description: | Update Custom Profile Attribute field values for a specific user. + **Note:** Values for fields with `attrs.protected = true` cannot be + updated and will return an error. + _This endpoint is experimental._ __Minimum server version__: 11 diff --git a/server/channels/api4/access_control.go b/server/channels/api4/access_control.go index d5c28afe4d0..c33ba50de13 100644 --- a/server/channels/api4/access_control.go +++ b/server/channels/api4/access_control.go @@ -711,7 +711,7 @@ func getFieldsAutocomplete(c *Context, w http.ResponseWriter, r *http.Request) { var ac []*model.PropertyField var appErr *model.AppError - ac, appErr = c.App.GetAccessControlFieldsAutocomplete(c.AppContext, after, limit) + ac, appErr = c.App.GetAccessControlFieldsAutocomplete(c.AppContext, after, limit, c.AppContext.Session().UserId) if appErr != nil { c.Err = appErr diff --git a/server/channels/api4/custom_profile_attributes.go b/server/channels/api4/custom_profile_attributes.go index 4cf0b4b4106..f7a2bdd077c 100644 --- a/server/channels/api4/custom_profile_attributes.go +++ b/server/channels/api4/custom_profile_attributes.go @@ -31,7 +31,8 @@ func listCPAFields(c *Context, w http.ResponseWriter, r *http.Request) { return } - fields, appErr := c.App.ListCPAFields() + callerUserID := c.AppContext.Session().UserId + fields, appErr := c.App.ListCPAFields(callerUserID) if appErr != nil { c.Err = appErr return @@ -66,7 +67,8 @@ func createCPAField(c *Context, w http.ResponseWriter, r *http.Request) { defer c.LogAuditRec(auditRec) model.AddEventParameterAuditableToAuditRec(auditRec, "property_field", pf) - createdField, appErr := c.App.CreateCPAField(pf) + callerUserID := c.AppContext.Session().UserId + createdField, appErr := c.App.CreateCPAField(callerUserID, pf) if appErr != nil { c.Err = appErr return @@ -121,7 +123,8 @@ func patchCPAField(c *Context, w http.ResponseWriter, r *http.Request) { defer c.LogAuditRec(auditRec) model.AddEventParameterAuditableToAuditRec(auditRec, "property_field_patch", patch) - originalField, appErr := c.App.GetCPAField(c.Params.FieldId) + callerUserID := c.AppContext.Session().UserId + originalField, appErr := c.App.GetCPAField(callerUserID, c.Params.FieldId) if appErr != nil { c.Err = appErr return @@ -129,7 +132,7 @@ func patchCPAField(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventPriorState(originalField) - patchedField, appErr := c.App.PatchCPAField(c.Params.FieldId, patch) + patchedField, appErr := c.App.PatchCPAField(callerUserID, c.Params.FieldId, patch) if appErr != nil { c.Err = appErr return @@ -164,14 +167,15 @@ func deleteCPAField(c *Context, w http.ResponseWriter, r *http.Request) { defer c.LogAuditRec(auditRec) model.AddEventParameterToAuditRec(auditRec, "field_id", c.Params.FieldId) - field, appErr := c.App.GetCPAField(c.Params.FieldId) + callerUserID := c.AppContext.Session().UserId + field, appErr := c.App.GetCPAField(callerUserID, c.Params.FieldId) if appErr != nil { c.Err = appErr return } auditRec.AddEventPriorState(field) - if appErr := c.App.DeleteCPAField(c.Params.FieldId); appErr != nil { + if appErr := c.App.DeleteCPAField(callerUserID, c.Params.FieldId); appErr != nil { c.Err = appErr return } @@ -224,8 +228,9 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { // if the user is not an admin, we need to check that there are no // admin-managed fields - if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { - fields, appErr := c.App.ListCPAFields() + session := *c.AppContext.Session() + if !c.App.SessionHasPermissionTo(session, model.PermissionManageSystem) { + fields, appErr := c.App.ListCPAFields(session.UserId) if appErr != nil { c.Err = appErr return @@ -242,9 +247,10 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { } } + callerUserID := c.AppContext.Session().UserId results := make(map[string]json.RawMessage, len(updates)) for fieldID, rawValue := range updates { - patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue, false) + patchedValue, appErr := c.App.PatchCPAValue(callerUserID, userID, fieldID, rawValue, false) if appErr != nil { c.Err = appErr return @@ -271,17 +277,19 @@ func listCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { return } - userID := c.Params.UserId + targetUserID := c.Params.UserId + callerUserID := c.AppContext.Session().UserId + // we check unrestricted sessions to allow local mode requests to go through if !c.AppContext.Session().IsUnrestricted() { - canSee, err := c.App.UserCanSeeOtherUser(c.AppContext, c.AppContext.Session().UserId, userID) + canSee, err := c.App.UserCanSeeOtherUser(c.AppContext, callerUserID, targetUserID) if err != nil || !canSee { c.SetPermissionError(model.PermissionViewMembers) return } } - values, appErr := c.App.ListCPAValues(userID) + values, appErr := c.App.ListCPAValues(callerUserID, targetUserID) if appErr != nil { c.Err = appErr return @@ -298,7 +306,7 @@ func listCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { func patchCPAValuesForUser(c *Context, w http.ResponseWriter, r *http.Request) { if !model.MinimumEnterpriseLicense(c.App.Channels().License()) { - c.Err = model.NewAppError("Api4.patchCPAValues", "api.custom_profile_attributes.license_error", nil, "", http.StatusForbidden) + c.Err = model.NewAppError("Api4.patchCPAValuesForUser", "api.custom_profile_attributes.license_error", nil, "", http.StatusForbidden) return } @@ -324,29 +332,35 @@ func patchCPAValuesForUser(c *Context, w http.ResponseWriter, r *http.Request) { defer c.LogAuditRec(auditRec) model.AddEventParameterToAuditRec(auditRec, "user_id", userID) - // if the user is not an admin, we need to check that there are no - // admin-managed fields - if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { - fields, appErr := c.App.ListCPAFields() + // Check for admin-managed fields + session := *c.AppContext.Session() + isAdmin := c.App.SessionHasPermissionTo(session, model.PermissionManageSystem) + if !isAdmin { + fields, appErr := c.App.ListCPAFields(session.UserId) if appErr != nil { c.Err = appErr return } - // Check if any of the fields being updated are admin-managed for _, field := range fields { - if _, isBeingUpdated := updates[field.ID]; isBeingUpdated { - if field.IsAdminManaged() { - c.Err = model.NewAppError("Api4.patchCPAValues", "app.custom_profile_attributes.property_field_is_managed.app_error", nil, "", http.StatusForbidden) - return - } + if _, isBeingUpdated := updates[field.ID]; !isBeingUpdated { + continue + } + // Check for admin-managed fields + if field.IsAdminManaged() { + c.Err = model.NewAppError("Api4.patchCPAValuesForUser", + "app.custom_profile_attributes.property_field_is_managed.app_error", + nil, "", + http.StatusForbidden) + return } } } + callerUserID := c.AppContext.Session().UserId results := make(map[string]json.RawMessage, len(updates)) for fieldID, rawValue := range updates { - patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue, false) + patchedValue, appErr := c.App.PatchCPAValue(callerUserID, userID, fieldID, rawValue, false) if appErr != nil { c.Err = appErr return diff --git a/server/channels/api4/custom_profile_attributes_test.go b/server/channels/api4/custom_profile_attributes_test.go index e51eaba847d..ab9de757f05 100644 --- a/server/channels/api4/custom_profile_attributes_test.go +++ b/server/channels/api4/custom_profile_attributes_test.go @@ -126,7 +126,7 @@ func TestListCPAFields(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -151,7 +151,7 @@ func TestListCPAFields(t *testing.T) { }) t.Run("the endpoint should only list non deleted fields", func(t *testing.T) { - require.Nil(t, th.App.DeleteCPAField(createdField.ID)) + require.Nil(t, th.App.DeleteCPAField("", createdField.ID)) fields, resp, err := th.Client.ListCPAFields(context.Background()) CheckOKStatus(t, resp) require.NoError(t, err) @@ -184,7 +184,7 @@ func TestPatchCPAField(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -203,7 +203,7 @@ func TestPatchCPAField(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -307,7 +307,7 @@ func TestPatchCPAField(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -388,7 +388,7 @@ func TestDeleteCPAField(t *testing.T) { CheckOKStatus(t, resp) require.NoError(t, err) - deletedField, appErr := th.App.GetCPAField(createdField.ID) + deletedField, appErr := th.App.GetCPAField("", createdField.ID) require.Nil(t, appErr) require.NotZero(t, deletedField.DeleteAt) @@ -430,11 +430,11 @@ func TestListCPAValues(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) - _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdField.ID, json.RawMessage(`"Field Value"`), true) + _, appErr = th.App.PatchCPAValue("", th.BasicUser.Id, createdField.ID, json.RawMessage(`"Field Value"`), true) require.Nil(t, appErr) t.Run("endpoint should not work if no valid license is present", func(t *testing.T) { @@ -474,11 +474,11 @@ func TestListCPAValues(t *testing.T) { }) require.NoError(t, err) - createdArrayField, appErr := th.App.CreateCPAField(arrayField) + createdArrayField, appErr := th.App.CreateCPAField("", arrayField) require.Nil(t, appErr) require.NotNil(t, createdArrayField) - _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdArrayField.ID, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, optionID1, optionID2)), true) + _, appErr = th.App.PatchCPAValue("", th.BasicUser.Id, createdArrayField.ID, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, optionID1, optionID2)), true) require.Nil(t, appErr) values, resp, err := th.Client.ListCPAValues(context.Background(), th.BasicUser.Id) @@ -515,7 +515,7 @@ func TestPatchCPAValues(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -618,7 +618,7 @@ func TestPatchCPAValues(t *testing.T) { }) require.NoError(t, err) - createdArrayField, appErr := th.App.CreateCPAField(arrayField) + createdArrayField, appErr := th.App.CreateCPAField("", arrayField) require.Nil(t, appErr) require.NotNil(t, createdArrayField) @@ -656,7 +656,7 @@ func TestPatchCPAValues(t *testing.T) { }) require.NoError(t, err) - createdLDAPField, appErr := th.App.CreateCPAField(ldapField) + createdLDAPField, appErr := th.App.CreateCPAField("", ldapField) require.Nil(t, appErr) require.NotNil(t, createdLDAPField) @@ -670,7 +670,7 @@ func TestPatchCPAValues(t *testing.T) { }) require.NoError(t, err) - createdSAMLField, appErr := th.App.CreateCPAField(samlField) + createdSAMLField, appErr := th.App.CreateCPAField("", samlField) require.Nil(t, appErr) require.NotNil(t, createdSAMLField) @@ -710,7 +710,7 @@ func TestPatchCPAValues(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -796,10 +796,10 @@ func TestPatchCPAValues(t *testing.T) { t.Run("batch update with managed fields fails for regular user", func(t *testing.T) { // First set some initial values to ensure we can verify they don't change // Set initial values for both fields using th.App (admins can set managed field values) - _, appErr := th.App.PatchCPAValue(th.BasicUser.Id, createdRegularField.ID, json.RawMessage(`"Initial Regular Value"`), false) + _, appErr := th.App.PatchCPAValue("", th.BasicUser.Id, createdRegularField.ID, json.RawMessage(`"Initial Regular Value"`), false) require.Nil(t, appErr) - _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Managed Value"`), true) + _, appErr = th.App.PatchCPAValue("", th.BasicUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Managed Value"`), true) require.Nil(t, appErr) // Try to batch update both managed and regular fields - this should fail @@ -814,7 +814,7 @@ func TestPatchCPAValues(t *testing.T) { CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_managed.app_error") // Verify that no values were updated when the batch operation failed - currentValues, appErr := th.App.ListCPAValues(th.BasicUser.Id) + currentValues, appErr := th.App.ListCPAValues("", th.BasicUser.Id) require.Nil(t, appErr) // Check that values remain unchanged - both fields should retain their initial values @@ -880,7 +880,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -983,7 +983,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { }) require.NoError(t, err) - createdArrayField, appErr := th.App.CreateCPAField(arrayField) + createdArrayField, appErr := th.App.CreateCPAField("", arrayField) require.Nil(t, appErr) require.NotNil(t, createdArrayField) @@ -1021,7 +1021,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { }) require.NoError(t, err) - createdLDAPField, appErr := th.App.CreateCPAField(ldapField) + createdLDAPField, appErr := th.App.CreateCPAField("", ldapField) require.Nil(t, appErr) require.NotNil(t, createdLDAPField) @@ -1035,7 +1035,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { }) require.NoError(t, err) - createdSAMLField, appErr := th.App.CreateCPAField(samlField) + createdSAMLField, appErr := th.App.CreateCPAField("", samlField) require.Nil(t, appErr) require.NotNil(t, createdSAMLField) @@ -1075,7 +1075,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField("", field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -1145,7 +1145,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { // Set initial value through the app layer that we will be replacing during the test - _, appErr := th.App.PatchCPAValue(th.SystemAdminUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Admin Value"`), true) + _, appErr := th.App.PatchCPAValue("", th.SystemAdminUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Admin Value"`), true) require.Nil(t, appErr) values := map[string]json.RawMessage{ @@ -1202,10 +1202,10 @@ func TestPatchCPAValuesForUser(t *testing.T) { t.Run("batch update with managed fields fails for regular user", func(t *testing.T) { // First set some initial values to ensure we can verify they don't change // Set initial values for both fields using th.App (admins can set managed field values) - _, appErr := th.App.PatchCPAValue(th.BasicUser.Id, createdRegularField.ID, json.RawMessage(`"Initial Regular Value"`), false) + _, appErr := th.App.PatchCPAValue("", th.BasicUser.Id, createdRegularField.ID, json.RawMessage(`"Initial Regular Value"`), false) require.Nil(t, appErr) - _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Managed Value"`), true) + _, appErr = th.App.PatchCPAValue("", th.BasicUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Managed Value"`), true) require.Nil(t, appErr) // Try to batch update both managed and regular fields - this should fail @@ -1220,7 +1220,7 @@ func TestPatchCPAValuesForUser(t *testing.T) { CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_managed.app_error") // Verify that no values were updated when the batch operation failed - currentValues, appErr := th.App.ListCPAValues(th.BasicUser.Id) + currentValues, appErr := th.App.ListCPAValues("", th.BasicUser.Id) require.Nil(t, appErr) // Check that values remain unchanged - both fields should retain their initial values diff --git a/server/channels/app/access_control.go b/server/channels/app/access_control.go index 4339029f132..890607647aa 100644 --- a/server/channels/app/access_control.go +++ b/server/channels/app/access_control.go @@ -283,14 +283,14 @@ func (a *App) GetAccessControlPolicyAttributes(rctx request.CTX, channelID strin return attributes, nil } -func (a *App) GetAccessControlFieldsAutocomplete(rctx request.CTX, after string, limit int) ([]*model.PropertyField, *model.AppError) { +func (a *App) GetAccessControlFieldsAutocomplete(rctx request.CTX, after string, limit int, callerID string) ([]*model.PropertyField, *model.AppError) { cpaGroupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("GetAccessControlAutoComplete", "app.pap.get_access_control_auto_complete.app_error", nil, err.Error(), http.StatusInternalServerError) } - fields, err := a.Srv().Store().PropertyField().SearchPropertyFields(model.PropertyFieldSearchOpts{ - GroupID: cpaGroupID, + // Use PropertyAccessService instead of direct Store access to enforce access control + fields, err := a.PropertyAccessService().SearchPropertyFields(callerID, cpaGroupID, model.PropertyFieldSearchOpts{ Cursor: model.PropertyFieldSearchCursor{ PropertyFieldID: after, CreateAt: 1, diff --git a/server/channels/app/app.go b/server/channels/app/app.go index 76334b59267..fb01afc4f0f 100644 --- a/server/channels/app/app.go +++ b/server/channels/app/app.go @@ -13,7 +13,6 @@ import ( "github.com/mattermost/mattermost/server/public/shared/httpservice" "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/public/shared/timezones" - "github.com/mattermost/mattermost/server/v8/channels/app/properties" "github.com/mattermost/mattermost/server/v8/einterfaces" "github.com/mattermost/mattermost/server/v8/platform/services/imageproxy" "github.com/mattermost/mattermost/server/v8/platform/services/searchengine" @@ -158,10 +157,6 @@ func (a *App) SetServer(srv *Server) { a.ch.srv = srv } -func (a *App) PropertyService() *properties.PropertyService { - return a.Srv().propertyService -} - func (a *App) PropertyAccessService() *PropertyAccessService { return a.Srv().propertyAccessService } diff --git a/server/channels/app/content_flagging.go b/server/channels/app/content_flagging.go index 9a8ea111ae2..03d01a4d078 100644 --- a/server/channels/app/content_flagging.go +++ b/server/channels/app/content_flagging.go @@ -157,7 +157,7 @@ func (a *App) FlagPost(rctx request.CTX, post *model.Post, teamId, reportingUser }) } - _, err = a.Srv().propertyService.CreatePropertyValues(propertyValues) + _, err = a.Srv().propertyAccessService.CreatePropertyValues(anonymousCallerId, propertyValues) if err != nil { return model.NewAppError("FlagPost", "app.content_flagging.create_property_values.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -224,7 +224,7 @@ func (a *App) setContentFlaggingPropertiesForThreadReplies(post *model.Post, con }) } - _, err = a.Srv().propertyService.CreatePropertyValues(propertyValues) + _, err = a.Srv().propertyAccessService.CreatePropertyValues(anonymousCallerId, propertyValues) if err != nil { return model.NewAppError("setContentFlaggingPropertiesForThreadReplies", "app.content_flagging.set_thread_replies_properties.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -233,7 +233,7 @@ func (a *App) setContentFlaggingPropertiesForThreadReplies(post *model.Post, con } func (a *App) ContentFlaggingGroupId() (string, *model.AppError) { - group, err := a.Srv().propertyService.GetPropertyGroup(model.ContentFlaggingGroupName) + group, err := a.Srv().propertyAccessService.GetPropertyGroup(model.ContentFlaggingGroupName) if err != nil { return "", model.NewAppError("getContentFlaggingGroupId", "app.content_flagging.get_group.error", nil, "", http.StatusInternalServerError) } @@ -246,13 +246,13 @@ func (a *App) GetPostContentFlaggingPropertyValue(postId, propertyFieldName stri return nil, appErr } - statusPropertyField, err := a.Srv().propertyService.GetPropertyFieldByName(groupId, "", propertyFieldName) + statusPropertyField, err := a.Srv().propertyAccessService.GetPropertyFieldByName(anonymousCallerId, groupId, "", propertyFieldName) if err != nil { return nil, model.NewAppError("GetPostContentFlaggingPropertyValue", "app.content_flagging.get_status_property.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } searchOptions := model.PropertyValueSearchOpts{TargetIDs: []string{postId}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: statusPropertyField.ID} - propertyValues, err := a.Srv().propertyService.SearchPropertyValues(groupId, searchOptions) + propertyValues, err := a.Srv().propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, searchOptions) if err != nil { return nil, model.NewAppError("GetPostContentFlaggingPropertyValue", "app.content_flagging.search_status_property.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -291,7 +291,7 @@ func (a *App) canFlagPost(groupId, postId, userLocal string) *model.AppError { } func (a *App) GetContentFlaggingMappedFields(groupId string) (map[string]*model.PropertyField, *model.AppError) { - fields, err := a.Srv().propertyService.SearchPropertyFields(groupId, model.PropertyFieldSearchOpts{PerPage: CONTENT_FLAGGING_MAX_PROPERTY_FIELDS}) + fields, err := a.Srv().propertyAccessService.SearchPropertyFields(anonymousCallerId, groupId, model.PropertyFieldSearchOpts{PerPage: CONTENT_FLAGGING_MAX_PROPERTY_FIELDS}) if err != nil { return nil, model.NewAppError("GetContentFlaggingMappedFields", "app.content_flagging.search_property_fields.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -365,7 +365,7 @@ func (a *App) createContentReviewPost(rctx request.CTX, flaggedPostId, teamId, r FieldID: flaggedPostIdFieldId, Value: json.RawMessage(fmt.Sprintf(`"%s"`, flaggedPostId)), } - _, err := a.Srv().propertyService.CreatePropertyValue(propertyValue) + _, err := a.Srv().propertyAccessService.CreatePropertyValue(anonymousCallerId, propertyValue) if err != nil { rctx.Logger().Error("Failed to create content review post property value in one of the channels", mlog.Err(err), mlog.String("channel_id", channel.Id), mlog.String("team_id", teamId), mlog.String("post_id", createdPost.Id)) } @@ -528,7 +528,7 @@ func (a *App) GetPostContentFlaggingPropertyValues(postId string) ([]*model.Prop return nil, appErr } - propertyValues, err := a.Srv().propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{TargetIDs: []string{postId}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES}) + propertyValues, err := a.PropertyAccessService().SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{TargetIDs: []string{postId}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES}) if err != nil { return nil, model.NewAppError("GetPostContentFlaggingPropertyValues", "app.content_flagging.search_property_values.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -654,13 +654,13 @@ func (a *App) PermanentDeleteFlaggedPost(rctx request.CTX, actionRequest *model. }, } - _, err = a.Srv().propertyService.CreatePropertyValues(propertyValues) + _, err = a.Srv().propertyAccessService.CreatePropertyValues(anonymousCallerId, propertyValues) if err != nil { return model.NewAppError("PermanentlyRemoveFlaggedPost", "app.content_flagging.create_property_values.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } status.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusRemoved)) - _, err = a.Srv().propertyService.UpdatePropertyValue(groupId, status) + _, err = a.Srv().propertyAccessService.UpdatePropertyValue(anonymousCallerId, groupId, status) if err != nil { return model.NewAppError("PermanentlyRemoveFlaggedPost", "app.content_flagging.permanently_delete.update_property_value.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -765,13 +765,13 @@ func (a *App) KeepFlaggedPost(rctx request.CTX, actionRequest *model.FlagContent }, } - _, err = a.Srv().propertyService.CreatePropertyValues(propertyValues) + _, err = a.Srv().propertyAccessService.CreatePropertyValues(anonymousCallerId, propertyValues) if err != nil { return model.NewAppError("KeepFlaggedPost", "app.content_flagging.create_property_values.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } status.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusRetained)) - _, err = a.Srv().propertyService.UpdatePropertyValue(groupId, status) + _, err = a.Srv().propertyAccessService.UpdatePropertyValue(anonymousCallerId, groupId, status) if err != nil { return model.NewAppError("KeepFlaggedPost", "app.content_flagging.keep_post.status_update.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -977,14 +977,14 @@ func (a *App) AssignFlaggedPostReviewer(rctx request.CTX, flaggedPostId, flagged Value: json.RawMessage(fmt.Sprintf(`"%s"`, reviewerId)), } - assigneePropertyValue, err := a.Srv().propertyService.UpsertPropertyValue(assigneePropertyValue) + assigneePropertyValue, err := a.Srv().propertyAccessService.UpsertPropertyValue(anonymousCallerId, assigneePropertyValue) if err != nil { return model.NewAppError("AssignFlaggedPostReviewer", "app.content_flagging.assign_reviewer.upsert_property_value.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } if status == model.ContentFlaggingStatusPending { statusPropertyValue.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusAssigned)) - statusPropertyValue, err = a.Srv().propertyService.UpdatePropertyValue(groupId, statusPropertyValue) + statusPropertyValue, err = a.Srv().propertyAccessService.UpdatePropertyValue(anonymousCallerId, groupId, statusPropertyValue) if err != nil { return model.NewAppError("AssignFlaggedPostReviewer", "app.content_flagging.assign_reviewer.update_status_property_value.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -1201,7 +1201,7 @@ func (a *App) getReviewerPostsForFlaggedPost(contentFlaggingGroupId, flaggedPost var propertyValues []*model.PropertyValue for { - batch, err := a.Srv().propertyService.SearchPropertyValues(contentFlaggingGroupId, searchOptions) + batch, err := a.Srv().propertyAccessService.SearchPropertyValues(anonymousCallerId, contentFlaggingGroupId, searchOptions) if err != nil { return nil, model.NewAppError("getReviewerPostsForFlaggedPost", "app.content_flagging.search_reviewer_posts.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/content_flagging_test.go b/server/channels/app/content_flagging_test.go index 0e60e4fb184..6fb0fb520ab 100644 --- a/server/channels/app/content_flagging_test.go +++ b/server/channels/app/content_flagging_test.go @@ -186,7 +186,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - reviewerValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + reviewerValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReviewerUserID].ID, @@ -221,7 +221,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - reviewerValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + reviewerValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReviewerUserID].ID, @@ -265,7 +265,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - reviewerValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + reviewerValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReviewerUserID].ID, @@ -295,7 +295,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - reviewerValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + reviewerValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReviewerUserID].ID, @@ -325,7 +325,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { // Set the status to Assigned statusValue.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusAssigned)) - _, err := th.App.Srv().propertyService.UpdatePropertyValue(groupId, statusValue) + _, err := th.App.PropertyAccessService().UpdatePropertyValue(anonymousCallerId, groupId, statusValue) require.NoError(t, err) appErr = th.App.AssignFlaggedPostReviewer(th.Context, post.Id, th.BasicChannel.TeamId, th.BasicUser.Id, th.SystemAdminUser.Id) @@ -337,7 +337,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { // Set the status to Removed statusValue.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusRemoved)) - _, err = th.App.Srv().propertyService.UpdatePropertyValue(groupId, statusValue) + _, err = th.App.PropertyAccessService().UpdatePropertyValue(anonymousCallerId, groupId, statusValue) require.NoError(t, err) appErr = th.App.AssignFlaggedPostReviewer(th.Context, post.Id, th.BasicChannel.TeamId, th.BasicUser.Id, th.SystemAdminUser.Id) @@ -349,7 +349,7 @@ func TestAssignFlaggedPostReviewer(t *testing.T) { // Set the status to Retained statusValue.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusRetained)) - _, err = th.App.Srv().propertyService.UpdatePropertyValue(groupId, statusValue) + _, err = th.App.PropertyAccessService().UpdatePropertyValue(anonymousCallerId, groupId, statusValue) require.NoError(t, err) appErr = th.App.AssignFlaggedPostReviewer(th.Context, post.Id, th.BasicChannel.TeamId, th.BasicUser.Id, th.SystemAdminUser.Id) @@ -988,10 +988,10 @@ func TestCanFlagPost(t *testing.T) { groupId, appErr := th.App.ContentFlaggingGroupId() require.Nil(t, appErr) - statusField, err := th.Server.propertyService.GetPropertyFieldByName(groupId, "", ContentFlaggingPropertyNameStatus) + statusField, err := th.Server.propertyAccessService.GetPropertyFieldByName(anonymousCallerId, groupId, "", ContentFlaggingPropertyNameStatus) require.NoError(t, err) - propertyValue, err := th.Server.propertyService.CreatePropertyValue(&model.PropertyValue{ + propertyValue, err := th.Server.propertyAccessService.CreatePropertyValue(anonymousCallerId, &model.PropertyValue{ TargetID: post.Id, GroupID: groupId, FieldID: statusField.ID, @@ -1007,7 +1007,7 @@ func TestCanFlagPost(t *testing.T) { // Can't fleg when post already flagged in assigned status propertyValue.Value = json.RawMessage(`"` + model.ContentFlaggingStatusAssigned + `"`) - _, err = th.Server.propertyService.UpdatePropertyValue(groupId, propertyValue) + _, err = th.Server.propertyAccessService.UpdatePropertyValue(anonymousCallerId, groupId, propertyValue) require.NoError(t, err) appErr = th.App.canFlagPost(groupId, post.Id, "en") @@ -1015,7 +1015,7 @@ func TestCanFlagPost(t *testing.T) { // Can't fleg when post already flagged in retained status propertyValue.Value = json.RawMessage(`"` + model.ContentFlaggingStatusRetained + `"`) - _, err = th.Server.propertyService.UpdatePropertyValue(groupId, propertyValue) + _, err = th.Server.propertyAccessService.UpdatePropertyValue(anonymousCallerId, groupId, propertyValue) require.NoError(t, err) appErr = th.App.canFlagPost(groupId, post.Id, "en") @@ -1023,7 +1023,7 @@ func TestCanFlagPost(t *testing.T) { // Can't fleg when post already flagged in removed status propertyValue.Value = json.RawMessage(`"` + model.ContentFlaggingStatusRemoved + `"`) - _, err = th.Server.propertyService.UpdatePropertyValue(groupId, propertyValue) + _, err = th.Server.propertyAccessService.UpdatePropertyValue(anonymousCallerId, groupId, propertyValue) require.NoError(t, err) appErr = th.App.canFlagPost(groupId, post.Id, "en") @@ -1069,7 +1069,7 @@ func TestFlagPost(t *testing.T) { require.Nil(t, appErr) // Check status property - statusValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + statusValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[ContentFlaggingPropertyNameStatus].ID, @@ -1079,7 +1079,7 @@ func TestFlagPost(t *testing.T) { require.Equal(t, `"`+model.ContentFlaggingStatusPending+`"`, string(statusValues[0].Value)) // Check reporting user property - userValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + userValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReportingUserID].ID, @@ -1089,7 +1089,7 @@ func TestFlagPost(t *testing.T) { require.Equal(t, `"`+th.BasicUser2.Id+`"`, string(userValues[0].Value)) // Check reason property - reasonValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + reasonValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReportingReason].ID, @@ -1099,7 +1099,7 @@ func TestFlagPost(t *testing.T) { require.Equal(t, `"spam"`, string(reasonValues[0].Value)) // Check comment property - commentValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + commentValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReportingComment].ID, @@ -1253,7 +1253,7 @@ func TestFlagPost(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - commentValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + commentValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReportingComment].ID, @@ -1287,7 +1287,7 @@ func TestFlagPost(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - timeValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + timeValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameReportingTime].ID, @@ -2223,7 +2223,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { require.Nil(t, appErr) // Check actor user property - actorValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + actorValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameActorUserID].ID, @@ -2233,7 +2233,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { require.Equal(t, `"`+th.SystemAdminUser.Id+`"`, string(actorValues[0].Value)) // Check actor comment property - commentValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + commentValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameActorComment].ID, @@ -2243,7 +2243,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { require.Equal(t, `"This post violates community guidelines"`, string(commentValues[0].Value)) // Check action time property - timeValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + timeValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameActionTime].ID, @@ -2309,7 +2309,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { require.Nil(t, appErr) statusValue.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusRemoved)) - _, err := th.App.Srv().propertyService.UpdatePropertyValue(groupId, statusValue) + _, err := th.App.PropertyAccessService().UpdatePropertyValue(anonymousCallerId, groupId, statusValue) require.NoError(t, err) actionRequest := &model.FlagContentActionRequest{ @@ -2333,7 +2333,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { require.Nil(t, appErr) statusValue.Value = json.RawMessage(fmt.Sprintf(`"%s"`, model.ContentFlaggingStatusRetained)) - _, err := th.App.Srv().propertyService.UpdatePropertyValue(groupId, statusValue) + _, err := th.App.PropertyAccessService().UpdatePropertyValue(anonymousCallerId, groupId, statusValue) require.NoError(t, err) actionRequest := &model.FlagContentActionRequest{ @@ -2375,7 +2375,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - commentValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + commentValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameActorComment].ID, @@ -2403,7 +2403,7 @@ func TestPermanentDeleteFlaggedPost(t *testing.T) { mappedFields, appErr := th.App.GetContentFlaggingMappedFields(groupId) require.Nil(t, appErr) - commentValues, err := th.Server.propertyService.SearchPropertyValues(groupId, model.PropertyValueSearchOpts{ + commentValues, err := th.Server.propertyAccessService.SearchPropertyValues(anonymousCallerId, groupId, model.PropertyValueSearchOpts{ TargetIDs: []string{post.Id}, PerPage: CONTENT_FLAGGING_MAX_PROPERTY_VALUES, FieldID: mappedFields[contentFlaggingPropertyNameActorComment].ID, diff --git a/server/channels/app/custom_profile_attributes.go b/server/channels/app/custom_profile_attributes.go index c4ea1e65a13..82d69f5bb73 100644 --- a/server/channels/app/custom_profile_attributes.go +++ b/server/channels/app/custom_profile_attributes.go @@ -21,14 +21,18 @@ const ( var cpaGroupID string +func (a *App) CpaGroupID() (string, error) { + return getCpaGroupID(a.Srv().propertyAccessService) +} + // ToDo: we should explore moving this to the database cache layer // instead of maintaining the ID cached at the application level -func (a *App) CpaGroupID() (string, error) { +func getCpaGroupID(service *PropertyAccessService) (string, error) { if cpaGroupID != "" { return cpaGroupID, nil } - cpaGroup, err := a.Srv().propertyService.RegisterPropertyGroup(model.CustomProfileAttributesPropertyGroupName) + cpaGroup, err := service.RegisterPropertyGroup(model.CustomProfileAttributesPropertyGroupName) if err != nil { return "", errors.Wrap(err, "cannot register Custom Profile Attributes property group") } @@ -37,13 +41,13 @@ func (a *App) CpaGroupID() (string, error) { return cpaGroupID, nil } -func (a *App) GetCPAField(fieldID string) (*model.CPAField, *model.AppError) { +func (a *App) GetCPAField(callerId, fieldID string) (*model.CPAField, *model.AppError) { groupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("GetCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - field, err := a.Srv().propertyService.GetPropertyField(groupID, fieldID) + field, err := a.Srv().propertyAccessService.GetPropertyField(callerId, groupID, fieldID) if err != nil { switch { case errors.Is(err, sql.ErrNoRows): @@ -61,7 +65,7 @@ func (a *App) GetCPAField(fieldID string) (*model.CPAField, *model.AppError) { return cpaField, nil } -func (a *App) ListCPAFields() ([]*model.CPAField, *model.AppError) { +func (a *App) ListCPAFields(callerID string) ([]*model.CPAField, *model.AppError) { groupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("ListCPAFields", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -72,7 +76,7 @@ func (a *App) ListCPAFields() ([]*model.CPAField, *model.AppError) { PerPage: CustomProfileAttributesFieldLimit, } - fields, err := a.Srv().propertyService.SearchPropertyFields(groupID, opts) + fields, err := a.Srv().propertyAccessService.SearchPropertyFields(callerID, groupID, opts) if err != nil { return nil, model.NewAppError("ListCPAFields", "app.custom_profile_attributes.search_property_fields.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -94,13 +98,13 @@ func (a *App) ListCPAFields() ([]*model.CPAField, *model.AppError) { return cpaFields, nil } -func (a *App) CreateCPAField(field *model.CPAField) (*model.CPAField, *model.AppError) { +func (a *App) CreateCPAField(callerId string, field *model.CPAField) (*model.CPAField, *model.AppError) { groupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - fieldCount, err := a.Srv().propertyService.CountActivePropertyFieldsForGroup(groupID) + fieldCount, err := a.Srv().propertyAccessService.CountActivePropertyFieldsForGroup(groupID) if err != nil { return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.count_property_fields.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -115,7 +119,7 @@ func (a *App) CreateCPAField(field *model.CPAField) (*model.CPAField, *model.App return nil, appErr } - newField, err := a.Srv().propertyService.CreatePropertyField(field.ToPropertyField()) + newField, err := a.Srv().propertyAccessService.CreatePropertyField(callerId, field.ToPropertyField()) if err != nil { var appErr *model.AppError switch { @@ -138,8 +142,8 @@ func (a *App) CreateCPAField(field *model.CPAField) (*model.CPAField, *model.App return cpaField, nil } -func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*model.CPAField, *model.AppError) { - existingField, appErr := a.GetCPAField(fieldID) +func (a *App) PatchCPAField(callerID string, fieldID string, patch *model.PropertyFieldPatch) (*model.CPAField, *model.AppError) { + existingField, appErr := a.GetCPAField(callerID, fieldID) if appErr != nil { return nil, appErr } @@ -162,7 +166,7 @@ func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*m return nil, model.NewAppError("PatchCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - patchedField, err := a.Srv().propertyService.UpdatePropertyField(groupID, existingField.ToPropertyField()) + patchedField, err := a.Srv().propertyAccessService.UpdatePropertyField(callerID, groupID, existingField.ToPropertyField()) if err != nil { var nfErr *store.ErrNotFound switch { @@ -179,7 +183,7 @@ func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*m } if shouldDeleteValues { - if dErr := a.Srv().propertyService.DeletePropertyValuesForField(groupID, cpaField.ID); dErr != nil { + if dErr := a.Srv().propertyAccessService.DeletePropertyValuesForField(callerID, groupID, cpaField.ID); dErr != nil { a.Log().Error("Error deleting property values when updating field", mlog.String("fieldID", cpaField.ID), mlog.Err(dErr), @@ -195,13 +199,13 @@ func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*m return cpaField, nil } -func (a *App) DeleteCPAField(id string) *model.AppError { +func (a *App) DeleteCPAField(callerID string, id string) *model.AppError { groupID, err := a.CpaGroupID() if err != nil { return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().propertyService.DeletePropertyField(groupID, id); err != nil { + if err := a.Srv().propertyAccessService.DeletePropertyField(callerID, groupID, id); err != nil { var nfErr *store.ErrNotFound switch { case errors.As(err, &nfErr): @@ -218,14 +222,14 @@ func (a *App) DeleteCPAField(id string) *model.AppError { return nil } -func (a *App) ListCPAValues(userID string) ([]*model.PropertyValue, *model.AppError) { +func (a *App) ListCPAValues(callerID, targetUserID string) ([]*model.PropertyValue, *model.AppError) { groupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("ListCPAValues", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - values, err := a.Srv().propertyService.SearchPropertyValues(groupID, model.PropertyValueSearchOpts{ - TargetIDs: []string{userID}, + values, err := a.Srv().propertyAccessService.SearchPropertyValues(callerID, groupID, model.PropertyValueSearchOpts{ + TargetIDs: []string{targetUserID}, PerPage: CustomProfileAttributesFieldLimit, }) if err != nil { @@ -235,13 +239,13 @@ func (a *App) ListCPAValues(userID string) ([]*model.PropertyValue, *model.AppEr return values, nil } -func (a *App) GetCPAValue(valueID string) (*model.PropertyValue, *model.AppError) { +func (a *App) GetCPAValue(callerId, valueID string) (*model.PropertyValue, *model.AppError) { groupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("GetCPAValue", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - value, err := a.Srv().propertyService.GetPropertyValue(groupID, valueID) + value, err := a.Srv().propertyAccessService.GetPropertyValue(callerId, groupID, valueID) if err != nil { return nil, model.NewAppError("GetCPAValue", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -249,8 +253,8 @@ func (a *App) GetCPAValue(valueID string) (*model.PropertyValue, *model.AppError return value, nil } -func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage, allowSynced bool) (*model.PropertyValue, *model.AppError) { - values, appErr := a.PatchCPAValues(userID, map[string]json.RawMessage{fieldID: value}, allowSynced) +func (a *App) PatchCPAValue(callerID string, userID string, fieldID string, value json.RawMessage, allowSynced bool) (*model.PropertyValue, *model.AppError) { + values, appErr := a.PatchCPAValues(callerID, userID, map[string]json.RawMessage{fieldID: value}, allowSynced) if appErr != nil { return nil, appErr } @@ -258,7 +262,7 @@ func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage return values[0], nil } -func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMessage, allowSynced bool) ([]*model.PropertyValue, *model.AppError) { +func (a *App) PatchCPAValues(callerID string, userID string, fieldValueMap map[string]json.RawMessage, allowSynced bool) ([]*model.PropertyValue, *model.AppError) { groupID, err := a.CpaGroupID() if err != nil { return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -267,7 +271,7 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes valuesToUpdate := []*model.PropertyValue{} for fieldID, rawValue := range fieldValueMap { // make sure field exists in this group - cpaField, appErr := a.GetCPAField(fieldID) + cpaField, appErr := a.GetCPAField(callerID, fieldID) if appErr != nil { return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound).Wrap(appErr) } else if cpaField.DeleteAt > 0 { @@ -293,7 +297,7 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes valuesToUpdate = append(valuesToUpdate, value) } - updatedValues, err := a.Srv().propertyService.UpsertPropertyValues(valuesToUpdate) + updatedValues, err := a.Srv().propertyAccessService.UpsertPropertyValues(callerID, valuesToUpdate) if err != nil { return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.property_value_upsert.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -311,13 +315,13 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes return updatedValues, nil } -func (a *App) DeleteCPAValues(userID string) *model.AppError { +func (a *App) DeleteCPAValues(callerId, userID string) *model.AppError { groupID, err := a.CpaGroupID() if err != nil { return model.NewAppError("DeleteCPAValues", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if err := a.Srv().propertyService.DeletePropertyValuesForTarget(groupID, "user", userID); err != nil { + if err := a.Srv().propertyAccessService.DeletePropertyValuesForTarget(callerId, groupID, "user", userID); err != nil { return model.NewAppError("DeleteCPAValues", "app.custom_profile_attributes.delete_property_values_for_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/custom_profile_attributes_test.go b/server/channels/app/custom_profile_attributes_test.go index a140656c899..45cae2f14e6 100644 --- a/server/channels/app/custom_profile_attributes_test.go +++ b/server/channels/app/custom_profile_attributes_test.go @@ -18,11 +18,11 @@ func TestGetCPAField(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) t.Run("should fail when getting a non-existent field", func(t *testing.T) { - field, appErr := th.App.GetCPAField(model.NewId()) + field, appErr := th.App.GetCPAField(anonymousCallerId, model.NewId()) require.NotNil(t, appErr) require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", appErr.Id) require.Empty(t, field) @@ -34,10 +34,10 @@ func TestGetCPAField(t *testing.T) { Name: model.NewId(), Type: model.PropertyFieldTypeText, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(field) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, field) require.NoError(t, err) - fetchedField, appErr := th.App.GetCPAField(createdField.ID) + fetchedField, appErr := th.App.GetCPAField(anonymousCallerId, createdField.ID) require.NotNil(t, appErr) require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", appErr.Id) require.Empty(t, fetchedField) @@ -45,18 +45,18 @@ func TestGetCPAField(t *testing.T) { t.Run("should get an existing CPA field", func(t *testing.T) { field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Test Field", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) require.NotEmpty(t, createdField.ID) - fetchedField, appErr := th.App.GetCPAField(createdField.ID) + fetchedField, appErr := th.App.GetCPAField(anonymousCallerId, createdField.ID) require.Nil(t, appErr) require.Equal(t, createdField.ID, fetchedField.ID) require.Equal(t, "Test Field", fetchedField.Name) @@ -66,16 +66,16 @@ func TestGetCPAField(t *testing.T) { t.Run("should initialize default attrs when field has nil Attrs", func(t *testing.T) { // Create a field with nil Attrs directly via property service (bypassing CPA validation) field := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Field with nil attrs", Type: model.PropertyFieldTypeText, Attrs: nil, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(field) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, field) require.NoError(t, err) // GetCPAField should initialize Attrs with defaults - fetchedField, appErr := th.App.GetCPAField(createdField.ID) + fetchedField, appErr := th.App.GetCPAField(anonymousCallerId, createdField.ID) require.Nil(t, appErr) require.Equal(t, model.CustomProfileAttributesVisibilityDefault, fetchedField.Attrs.Visibility) require.Equal(t, float64(0), fetchedField.Attrs.SortOrder) @@ -84,16 +84,16 @@ func TestGetCPAField(t *testing.T) { t.Run("should initialize default attrs when field has empty Attrs", func(t *testing.T) { // Create a field with empty Attrs directly via property service field := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Field with empty attrs", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{}, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(field) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, field) require.NoError(t, err) // GetCPAField should add missing default attrs - fetchedField, appErr := th.App.GetCPAField(createdField.ID) + fetchedField, appErr := th.App.GetCPAField(anonymousCallerId, createdField.ID) require.Nil(t, appErr) require.Equal(t, model.CustomProfileAttributesVisibilityDefault, fetchedField.Attrs.Visibility) require.Equal(t, float64(0), fetchedField.Attrs.SortOrder) @@ -102,7 +102,7 @@ func TestGetCPAField(t *testing.T) { t.Run("should validate LDAP/SAML synced fields", func(t *testing.T) { // Create LDAP synced field ldapField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "LDAP Field", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{ @@ -110,12 +110,12 @@ func TestGetCPAField(t *testing.T) { }, }) require.NoError(t, err) - createdLDAPField, appErr := th.App.CreateCPAField(ldapField) + createdLDAPField, appErr := th.App.CreateCPAField(anonymousCallerId, ldapField) require.Nil(t, appErr) // Create SAML synced field samlField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "SAML Field", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{ @@ -123,31 +123,31 @@ func TestGetCPAField(t *testing.T) { }, }) require.NoError(t, err) - createdSAMLField, appErr := th.App.CreateCPAField(samlField) + createdSAMLField, appErr := th.App.CreateCPAField(anonymousCallerId, samlField) require.Nil(t, appErr) // Test with allowSynced=false userID := model.NewId() // Test LDAP field - _, appErr = th.App.PatchCPAValue(userID, createdLDAPField.ID, json.RawMessage(`"test value"`), false) + _, appErr = th.App.PatchCPAValue(anonymousCallerId, userID, createdLDAPField.ID, json.RawMessage(`"test value"`), false) require.NotNil(t, appErr) require.Equal(t, "app.custom_profile_attributes.property_field_is_synced.app_error", appErr.Id) // Test SAML field - _, appErr = th.App.PatchCPAValue(userID, createdSAMLField.ID, json.RawMessage(`"test value"`), false) + _, appErr = th.App.PatchCPAValue(anonymousCallerId, userID, createdSAMLField.ID, json.RawMessage(`"test value"`), false) require.NotNil(t, appErr) require.Equal(t, "app.custom_profile_attributes.property_field_is_synced.app_error", appErr.Id) // Test with allowSynced=true // LDAP field should work - patchedValue, appErr := th.App.PatchCPAValue(userID, createdLDAPField.ID, json.RawMessage(`"test value"`), true) + patchedValue, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdLDAPField.ID, json.RawMessage(`"test value"`), true) require.Nil(t, appErr) require.NotNil(t, patchedValue) require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value) // SAML field should work - patchedValue, appErr = th.App.PatchCPAValue(userID, createdSAMLField.ID, json.RawMessage(`"test value"`), true) + patchedValue, appErr = th.App.PatchCPAValue(anonymousCallerId, userID, createdSAMLField.ID, json.RawMessage(`"test value"`), true) require.Nil(t, appErr) require.NotNil(t, patchedValue) require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value) @@ -158,18 +158,18 @@ func TestListCPAFields(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) t.Run("should list the CPA property fields", func(t *testing.T) { field1 := model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Field 1", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsSortOrder: 1}, } - _, err := th.App.Srv().propertyService.CreatePropertyField(&field1) + _, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, &field1) require.NoError(t, err) field2 := &model.PropertyField{ @@ -177,19 +177,19 @@ func TestListCPAFields(t *testing.T) { Name: "Field 2", Type: model.PropertyFieldTypeText, } - _, err = th.App.Srv().propertyService.CreatePropertyField(field2) + _, err = th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, field2) require.NoError(t, err) field3 := model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Field 3", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsSortOrder: 0}, } - _, err = th.App.Srv().propertyService.CreatePropertyField(&field3) + _, err = th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, &field3) require.NoError(t, err) - fields, appErr := th.App.ListCPAFields() + fields, appErr := th.App.ListCPAFields(anonymousCallerId) require.Nil(t, appErr) require.Len(t, fields, 2) require.Equal(t, "Field 3", fields[0].Name) @@ -199,26 +199,26 @@ func TestListCPAFields(t *testing.T) { t.Run("should initialize default attrs for fields with nil or empty Attrs", func(t *testing.T) { // Create a field with nil Attrs fieldWithNilAttrs := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Field with nil attrs", Type: model.PropertyFieldTypeText, Attrs: nil, } - _, err := th.App.Srv().propertyService.CreatePropertyField(fieldWithNilAttrs) + _, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, fieldWithNilAttrs) require.NoError(t, err) // Create a field with empty Attrs fieldWithEmptyAttrs := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Field with empty attrs", Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{}, } - _, err = th.App.Srv().propertyService.CreatePropertyField(fieldWithEmptyAttrs) + _, err = th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, fieldWithEmptyAttrs) require.NoError(t, err) // ListCPAFields should initialize Attrs with defaults - fields, appErr := th.App.ListCPAFields() + fields, appErr := th.App.ListCPAFields(anonymousCallerId) require.Nil(t, appErr) require.NotEmpty(t, fields) @@ -239,7 +239,7 @@ func TestListCPAFields(t *testing.T) { Attrs: model.StringInterface{}, // Empty attrs - no visibility or sort_order }) require.NoError(t, err) - createdFieldMinimal, appErr := th.App.CreateCPAField(fieldMinimal) + createdFieldMinimal, appErr := th.App.CreateCPAField(anonymousCallerId, fieldMinimal) require.Nil(t, appErr) require.NotNil(t, createdFieldMinimal) @@ -253,12 +253,12 @@ func TestListCPAFields(t *testing.T) { }, }) require.NoError(t, err) - createdFieldNormal, appErr := th.App.CreateCPAField(fieldNormal) + createdFieldNormal, appErr := th.App.CreateCPAField(anonymousCallerId, fieldNormal) require.Nil(t, appErr) require.NotNil(t, createdFieldNormal) // List all fields - fields, appErr := th.App.ListCPAFields() + fields, appErr := th.App.ListCPAFields(anonymousCallerId) require.Nil(t, appErr) require.NotEmpty(t, fields) @@ -288,14 +288,14 @@ func TestCreateCPAField(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) t.Run("should fail if the field is not valid", func(t *testing.T) { field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{Name: model.NewId()}) require.NoError(t, err) - createdField, err := th.App.CreateCPAField(field) + createdField, err := th.App.CreateCPAField(anonymousCallerId, field) require.Error(t, err) require.Empty(t, createdField) }) @@ -308,27 +308,27 @@ func TestCreateCPAField(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) - require.Equal(t, cpaGroupID, createdField.GroupID) + require.Equal(t, cpaID, createdField.GroupID) }) t.Run("should correctly create a CPA field", func(t *testing.T) { field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) require.NotZero(t, createdField.ID) - require.Equal(t, cpaGroupID, createdField.GroupID) + require.Equal(t, cpaID, createdField.GroupID) require.Equal(t, model.CustomProfileAttributesVisibilityHidden, createdField.Attrs.Visibility) - fetchedField, gErr := th.App.Srv().propertyService.GetPropertyField("", createdField.ID) + fetchedField, gErr := th.App.PropertyAccessService().GetPropertyField(anonymousCallerId, "", createdField.ID) require.NoError(t, gErr) require.Equal(t, field.Name, fetchedField.Name) require.NotZero(t, fetchedField.CreateAt) @@ -338,7 +338,7 @@ func TestCreateCPAField(t *testing.T) { t.Run("should create CPA field with DeleteAt set to 0 even if input has non-zero DeleteAt", func(t *testing.T) { // Create a CPAField with DeleteAt != 0 field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, @@ -349,16 +349,16 @@ func TestCreateCPAField(t *testing.T) { field.DeleteAt = time.Now().UnixMilli() require.NotZero(t, field.DeleteAt, "Pre-condition: field should have non-zero DeleteAt") - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) require.NotZero(t, createdField.ID) - require.Equal(t, cpaGroupID, createdField.GroupID) + require.Equal(t, cpaID, createdField.GroupID) // Verify that DeleteAt has been reset to 0 require.Zero(t, createdField.DeleteAt, "DeleteAt should be 0 after creation") // Double-check by fetching the field from the database - fetchedField, gErr := th.App.Srv().propertyService.GetPropertyField("", createdField.ID) + fetchedField, gErr := th.App.PropertyAccessService().GetPropertyField(anonymousCallerId, "", createdField.ID) require.NoError(t, gErr) require.Zero(t, fetchedField.DeleteAt, "DeleteAt should be 0 in database") }) @@ -375,7 +375,7 @@ func TestCreateCPAField(t *testing.T) { }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) require.NotZero(t, createdField.ID) } @@ -387,7 +387,7 @@ func TestCreateCPAField(t *testing.T) { Type: model.PropertyFieldTypeText, }, } - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.NotNil(t, appErr) require.Equal(t, http.StatusUnprocessableEntity, appErr.StatusCode) require.Zero(t, createdField) @@ -395,12 +395,12 @@ func TestCreateCPAField(t *testing.T) { t.Run("deleted fields should not count for the limit", func(t *testing.T) { // we retrieve the list of fields and check we've reached the limit - fields, appErr := th.App.ListCPAFields() + fields, appErr := th.App.ListCPAFields(anonymousCallerId) require.Nil(t, appErr) require.Len(t, fields, CustomProfileAttributesFieldLimit) // then we delete one field - require.Nil(t, th.App.DeleteCPAField(fields[0].ID)) + require.Nil(t, th.App.DeleteCPAField(anonymousCallerId, fields[0].ID)) // creating a new one should work now field := &model.CPAField{ @@ -409,7 +409,7 @@ func TestCreateCPAField(t *testing.T) { Type: model.PropertyFieldTypeText, }, } - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) require.NotZero(t, createdField.ID) }) @@ -420,18 +420,18 @@ func TestPatchCPAField(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) newField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeText, Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(newField) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, newField) require.Nil(t, appErr) patch := &model.PropertyFieldPatch{ @@ -442,7 +442,7 @@ func TestPatchCPAField(t *testing.T) { } t.Run("should fail if the field doesn't exist", func(t *testing.T) { - updatedField, appErr := th.App.PatchCPAField(model.NewId(), patch) + updatedField, appErr := th.App.PatchCPAField(anonymousCallerId, model.NewId(), patch) require.NotNil(t, appErr) require.Empty(t, updatedField) }) @@ -454,10 +454,10 @@ func TestPatchCPAField(t *testing.T) { Type: model.PropertyFieldTypeText, } - field, err := th.App.Srv().propertyService.CreatePropertyField(newField) + field, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, newField) require.NoError(t, err) - updatedField, uErr := th.App.PatchCPAField(field.ID, patch) + updatedField, uErr := th.App.PatchCPAField(anonymousCallerId, field.ID, patch) require.NotNil(t, uErr) require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", uErr.Id) require.Empty(t, updatedField) @@ -466,7 +466,7 @@ func TestPatchCPAField(t *testing.T) { t.Run("should correctly patch the CPA property field", func(t *testing.T) { time.Sleep(10 * time.Millisecond) // ensure the UpdateAt is different than CreateAt - updatedField, appErr := th.App.PatchCPAField(createdField.ID, patch) + updatedField, appErr := th.App.PatchCPAField(anonymousCallerId, createdField.ID, patch) require.Nil(t, appErr) require.Equal(t, createdField.ID, updatedField.ID) require.Equal(t, "Patched name", updatedField.Name) @@ -479,7 +479,7 @@ func TestPatchCPAField(t *testing.T) { t.Run("should preserve option IDs when patching select field options", func(t *testing.T) { // Create a select field with options selectField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Select Field", Type: model.PropertyFieldTypeSelect, Attrs: map[string]any{ @@ -497,7 +497,7 @@ func TestPatchCPAField(t *testing.T) { }) require.NoError(t, err) - createdSelectField, appErr := th.App.CreateCPAField(selectField) + createdSelectField, appErr := th.App.CreateCPAField(anonymousCallerId, selectField) require.Nil(t, appErr) // Get the original option IDs @@ -530,7 +530,7 @@ func TestPatchCPAField(t *testing.T) { }), } - updatedSelectField, appErr := th.App.PatchCPAField(createdSelectField.ID, selectPatch) + updatedSelectField, appErr := th.App.PatchCPAField(anonymousCallerId, createdSelectField.ID, selectPatch) require.Nil(t, appErr) updatedOptions := updatedSelectField.Attrs.Options @@ -552,7 +552,7 @@ func TestPatchCPAField(t *testing.T) { t.Run("Should not delete the values of a field after patching it if the type has not changed", func(t *testing.T) { // Create a select field with options field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Select Field with values", Type: model.PropertyFieldTypeSelect, Attrs: model.StringInterface{ @@ -569,7 +569,7 @@ func TestPatchCPAField(t *testing.T) { }, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) // Get the option IDs @@ -580,7 +580,7 @@ func TestPatchCPAField(t *testing.T) { // Create values for this field using the first option userID := model.NewId() - value, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"%s"`, optionID)), false) + value, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"%s"`, optionID)), false) require.Nil(t, appErr) require.NotNil(t, value) @@ -605,13 +605,13 @@ func TestPatchCPAField(t *testing.T) { }, }), } - updatedField, appErr := th.App.PatchCPAField(createdField.ID, patch) + updatedField, appErr := th.App.PatchCPAField(anonymousCallerId, createdField.ID, patch) require.Nil(t, appErr) require.Equal(t, "Updated select field name", updatedField.Name) require.Equal(t, model.PropertyFieldTypeSelect, updatedField.Type) // Verify values still exist - values, appErr := th.App.ListCPAValues(userID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Len(t, values, 1) require.Equal(t, json.RawMessage(fmt.Sprintf(`"%s"`, optionID)), values[0].Value) @@ -620,7 +620,7 @@ func TestPatchCPAField(t *testing.T) { t.Run("Should delete the values of a field after patching it if the type has changed", func(t *testing.T) { // Create a select field with options field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: "Select Field with type change", Type: model.PropertyFieldTypeSelect, Attrs: model.StringInterface{ @@ -637,7 +637,7 @@ func TestPatchCPAField(t *testing.T) { }, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) // Get the option IDs @@ -648,12 +648,12 @@ func TestPatchCPAField(t *testing.T) { // Create values for this field userID := model.NewId() - value, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"%s"`, optionID)), false) + value, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"%s"`, optionID)), false) require.Nil(t, appErr) require.NotNil(t, value) // Verify value exists before type change - values, appErr := th.App.ListCPAValues(userID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Len(t, values, 1) @@ -661,12 +661,12 @@ func TestPatchCPAField(t *testing.T) { patch := &model.PropertyFieldPatch{ Type: model.NewPointer(model.PropertyFieldTypeText), } - updatedField, appErr := th.App.PatchCPAField(createdField.ID, patch) + updatedField, appErr := th.App.PatchCPAField(anonymousCallerId, createdField.ID, patch) require.Nil(t, appErr) require.Equal(t, model.PropertyFieldTypeText, updatedField.Type) // Verify values have been deleted - values, appErr = th.App.ListCPAValues(userID) + values, appErr = th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Empty(t, values) }) @@ -676,34 +676,34 @@ func TestDeleteCPAField(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) newField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeText, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(newField) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, newField) require.Nil(t, appErr) for i := range 3 { newValue := &model.PropertyValue{ TargetID: model.NewId(), TargetType: model.PropertyValueTargetTypeUser, - GroupID: cpaGroupID, + GroupID: cpaID, FieldID: createdField.ID, Value: json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), } - value, err := th.App.Srv().propertyService.CreatePropertyValue(newValue) + value, err := th.App.PropertyAccessService().CreatePropertyValue(anonymousCallerId, newValue) require.NoError(t, err) require.NotZero(t, value.ID) } t.Run("should fail if the field doesn't exist", func(t *testing.T) { - err := th.App.DeleteCPAField(model.NewId()) + err := th.App.DeleteCPAField(anonymousCallerId, model.NewId()) require.NotNil(t, err) require.Equal(t, "app.custom_profile_attributes.property_field_delete.app_error", err.Id) }) @@ -714,10 +714,10 @@ func TestDeleteCPAField(t *testing.T) { Name: model.NewId(), Type: model.PropertyFieldTypeText, } - field, err := th.App.Srv().propertyService.CreatePropertyField(newField) + field, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, newField) require.NoError(t, err) - dErr := th.App.DeleteCPAField(field.ID) + dErr := th.App.DeleteCPAField(anonymousCallerId, field.ID) require.NotNil(t, dErr) require.Equal(t, "app.custom_profile_attributes.property_field_delete.app_error", dErr.Id) }) @@ -725,25 +725,25 @@ func TestDeleteCPAField(t *testing.T) { t.Run("should correctly delete the field", func(t *testing.T) { // check that we have the associated values to the field prior deletion opts := model.PropertyValueSearchOpts{PerPage: 10, FieldID: createdField.ID} - values, err := th.App.Srv().propertyService.SearchPropertyValues(cpaGroupID, opts) + values, err := th.App.PropertyAccessService().SearchPropertyValues(anonymousCallerId, cpaID, opts) require.NoError(t, err) require.Len(t, values, 3) // delete the field - require.Nil(t, th.App.DeleteCPAField(createdField.ID)) + require.Nil(t, th.App.DeleteCPAField(anonymousCallerId, createdField.ID)) // check that it is marked as deleted - fetchedField, err := th.App.Srv().propertyService.GetPropertyField("", createdField.ID) + fetchedField, err := th.App.PropertyAccessService().GetPropertyField(anonymousCallerId, "", createdField.ID) require.NoError(t, err) require.NotZero(t, fetchedField.DeleteAt) // ensure that the associated fields have been marked as deleted too - values, err = th.App.Srv().propertyService.SearchPropertyValues(cpaGroupID, opts) + values, err = th.App.PropertyAccessService().SearchPropertyValues(anonymousCallerId, cpaID, opts) require.NoError(t, err) require.Len(t, values, 0) opts.IncludeDeleted = true - values, err = th.App.Srv().propertyService.SearchPropertyValues(cpaGroupID, opts) + values, err = th.App.PropertyAccessService().SearchPropertyValues(anonymousCallerId, cpaID, opts) require.NoError(t, err) require.Len(t, values, 3) for _, value := range values { @@ -756,13 +756,20 @@ func TestGetCPAValue(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) - fieldID := model.NewId() + field := &model.PropertyField{ + GroupID: cpaID, + Name: model.NewId(), + Type: model.PropertyFieldTypeText, + } + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, field) + require.NoError(t, err) + fieldID := createdField.ID t.Run("should fail if the value doesn't exist", func(t *testing.T) { - pv, appErr := th.App.GetCPAValue(model.NewId()) + pv, appErr := th.App.GetCPAValue(anonymousCallerId, model.NewId()) require.NotNil(t, appErr) require.Nil(t, pv) }) @@ -775,10 +782,11 @@ func TestGetCPAValue(t *testing.T) { FieldID: fieldID, Value: json.RawMessage(`"Value"`), } - propertyValue, err := th.App.Srv().propertyService.CreatePropertyValue(propertyValue) + created, err := th.App.PropertyAccessService().CreatePropertyValue(anonymousCallerId, propertyValue) require.NoError(t, err) + require.NotNil(t, created) - pv, appErr := th.App.GetCPAValue(propertyValue.ID) + pv, appErr := th.App.GetCPAValue(anonymousCallerId, created.ID) require.NotNil(t, appErr) require.Nil(t, pv) }) @@ -787,38 +795,38 @@ func TestGetCPAValue(t *testing.T) { propertyValue := &model.PropertyValue{ TargetID: model.NewId(), TargetType: model.PropertyValueTargetTypeUser, - GroupID: cpaGroupID, + GroupID: cpaID, FieldID: fieldID, Value: json.RawMessage(`"Value"`), } - propertyValue, err := th.App.Srv().propertyService.CreatePropertyValue(propertyValue) + propertyValue, err := th.App.PropertyAccessService().CreatePropertyValue(anonymousCallerId, propertyValue) require.NoError(t, err) - pv, appErr := th.App.GetCPAValue(propertyValue.ID) + pv, appErr := th.App.GetCPAValue(anonymousCallerId, propertyValue.ID) require.Nil(t, appErr) require.NotNil(t, pv) }) t.Run("should handle array values correctly", func(t *testing.T) { arrayField := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeMultiselect, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(arrayField) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, arrayField) require.NoError(t, err) propertyValue := &model.PropertyValue{ TargetID: model.NewId(), TargetType: model.PropertyValueTargetTypeUser, - GroupID: cpaGroupID, + GroupID: cpaID, FieldID: createdField.ID, Value: json.RawMessage(`["option1", "option2", "option3"]`), } - propertyValue, err = th.App.Srv().propertyService.CreatePropertyValue(propertyValue) + propertyValue, err = th.App.PropertyAccessService().CreatePropertyValue(anonymousCallerId, propertyValue) require.NoError(t, err) - pv, appErr := th.App.GetCPAValue(propertyValue.ID) + pv, appErr := th.App.GetCPAValue(anonymousCallerId, propertyValue.ID) require.Nil(t, appErr) require.NotNil(t, pv) var arrayValues []string @@ -833,13 +841,13 @@ func TestListCPAValues(t *testing.T) { cfg.FeatureFlags.CustomProfileAttributes = true }).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) userID := model.NewId() t.Run("should return empty list when user has no values", func(t *testing.T) { - values, appErr := th.App.ListCPAValues(userID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Empty(t, values) }) @@ -849,27 +857,27 @@ func TestListCPAValues(t *testing.T) { for i := 1; i <= CustomProfileAttributesFieldLimit; i++ { field := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: fmt.Sprintf("Field %d", i), Type: model.PropertyFieldTypeText, } - _, err := th.App.Srv().propertyService.CreatePropertyField(field) + _, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, field) require.NoError(t, err) value := &model.PropertyValue{ TargetID: userID, TargetType: model.PropertyValueTargetTypeUser, - GroupID: cpaGroupID, + GroupID: cpaID, FieldID: field.ID, Value: json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), } - _, err = th.App.Srv().propertyService.CreatePropertyValue(value) + _, err = th.App.PropertyAccessService().CreatePropertyValue(anonymousCallerId, value) require.NoError(t, err) expectedValues = append(expectedValues, value.Value) } // List values for original user - values, appErr := th.App.ListCPAValues(userID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Len(t, values, CustomProfileAttributesFieldLimit) @@ -877,7 +885,7 @@ func TestListCPAValues(t *testing.T) { for i, value := range values { require.Equal(t, userID, value.TargetID) require.Equal(t, "user", value.TargetType) - require.Equal(t, cpaGroupID, value.GroupID) + require.Equal(t, cpaID, value.GroupID) actualValues[i] = value.Value } require.ElementsMatch(t, expectedValues, actualValues) @@ -888,33 +896,33 @@ func TestPatchCPAValue(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) t.Run("should fail if the field doesn't exist", func(t *testing.T) { invalidFieldID := model.NewId() - _, appErr := th.App.PatchCPAValue(model.NewId(), invalidFieldID, json.RawMessage(`"fieldValue"`), true) + _, appErr := th.App.PatchCPAValue(anonymousCallerId, model.NewId(), invalidFieldID, json.RawMessage(`"fieldValue"`), true) require.NotNil(t, appErr) }) t.Run("should create value if new field value", func(t *testing.T) { newField := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeText, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(newField) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, newField) require.NoError(t, err) userID := model.NewId() - patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"test value"`), true) + patchedValue, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(`"test value"`), true) require.Nil(t, appErr) require.NotNil(t, patchedValue) require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value) require.Equal(t, userID, patchedValue.TargetID) t.Run("should correctly patch the CPA property value", func(t *testing.T) { - patch2, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"new patched value"`), true) + patch2, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(`"new patched value"`), true) require.Nil(t, appErr) require.NotNil(t, patch2) require.Equal(t, patchedValue.ID, patch2.ID) @@ -925,17 +933,17 @@ func TestPatchCPAValue(t *testing.T) { t.Run("should fail if field is deleted", func(t *testing.T) { newField := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeText, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(newField) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, newField) require.NoError(t, err) - err = th.App.Srv().propertyService.DeletePropertyField(cpaGroupID, createdField.ID) + err = th.App.PropertyAccessService().DeletePropertyField(anonymousCallerId, cpaID, createdField.ID) require.NoError(t, err) userID := model.NewId() - patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"test value"`), true) + patchedValue, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(`"test value"`), true) require.NotNil(t, appErr) require.Nil(t, patchedValue) }) @@ -943,7 +951,7 @@ func TestPatchCPAValue(t *testing.T) { t.Run("should handle array values correctly", func(t *testing.T) { optionsID := []string{model.NewId(), model.NewId(), model.NewId(), model.NewId()} arrayField := &model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: model.NewId(), Type: model.PropertyFieldTypeMultiselect, Attrs: model.StringInterface{ @@ -955,14 +963,14 @@ func TestPatchCPAValue(t *testing.T) { }, }, } - createdField, err := th.App.Srv().propertyService.CreatePropertyField(arrayField) + createdField, err := th.App.PropertyAccessService().CreatePropertyField(anonymousCallerId, arrayField) require.NoError(t, err) // Create a JSON array with option IDs (not names) optionJSON := fmt.Sprintf(`["%s", "%s", "%s"]`, optionsID[0], optionsID[1], optionsID[2]) userID := model.NewId() - patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(optionJSON), true) + patchedValue, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(optionJSON), true) require.Nil(t, appErr) require.NotNil(t, patchedValue) var arrayValues []string @@ -972,7 +980,7 @@ func TestPatchCPAValue(t *testing.T) { // Update array values with valid option IDs updatedOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[1], optionsID[3]) - updatedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(updatedOptionJSON), true) + updatedValue, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(updatedOptionJSON), true) require.Nil(t, appErr) require.NotNil(t, updatedValue) require.Equal(t, patchedValue.ID, updatedValue.ID) @@ -986,21 +994,21 @@ func TestPatchCPAValue(t *testing.T) { invalidID := model.NewId() invalidOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[0], invalidID) - invalidValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidOptionJSON), true) + invalidValue, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(invalidOptionJSON), true) require.NotNil(t, appErr) require.Nil(t, invalidValue) require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) // Test with completely invalid JSON format invalidJSON := `[not valid json]` - invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidJSON), true) + invalidValue, appErr = th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(invalidJSON), true) require.NotNil(t, appErr) require.Nil(t, invalidValue) require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) // Test with wrong data type (sending string instead of array) wrongTypeJSON := `"not an array"` - invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(wrongTypeJSON), true) + invalidValue, appErr = th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(wrongTypeJSON), true) require.NotNil(t, appErr) require.Nil(t, invalidValue) require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) @@ -1014,7 +1022,7 @@ func TestDeleteCPAValues(t *testing.T) { cfg.FeatureFlags.CustomProfileAttributes = true }).InitBasic(t) - cpaGroupID, cErr := th.App.CpaGroupID() + cpaID, cErr := th.App.CpaGroupID() require.NoError(t, cErr) userID := model.NewId() @@ -1024,56 +1032,56 @@ func TestDeleteCPAValues(t *testing.T) { var createdFields []*model.CPAField for i := 1; i <= 3; i++ { field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ - GroupID: cpaGroupID, + GroupID: cpaID, Name: fmt.Sprintf("Field %d", i), Type: model.PropertyFieldTypeText, }) require.NoError(t, err) - createdField, appErr := th.App.CreateCPAField(field) + createdField, appErr := th.App.CreateCPAField(anonymousCallerId, field) require.Nil(t, appErr) createdFields = append(createdFields, createdField) // Create a value for this field - value, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), false) + value, appErr := th.App.PatchCPAValue(anonymousCallerId, userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), false) require.Nil(t, appErr) require.NotNil(t, value) } // Verify values exist before deletion - values, appErr := th.App.ListCPAValues(userID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Len(t, values, 3) // Test deleting values for user t.Run("should delete all values for a user", func(t *testing.T) { - appErr := th.App.DeleteCPAValues(userID) + appErr := th.App.DeleteCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) // Verify values are gone - values, appErr := th.App.ListCPAValues(userID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) require.Empty(t, values) }) t.Run("should handle deleting values for a user with no values", func(t *testing.T) { - appErr := th.App.DeleteCPAValues(otherUserID) + appErr := th.App.DeleteCPAValues(anonymousCallerId, otherUserID) require.Nil(t, appErr) }) t.Run("should not affect values for other users", func(t *testing.T) { // Create values for another user for _, field := range createdFields { - value, appErr := th.App.PatchCPAValue(otherUserID, field.ID, json.RawMessage(`"Other user value"`), false) + value, appErr := th.App.PatchCPAValue(anonymousCallerId, otherUserID, field.ID, json.RawMessage(`"Other user value"`), false) require.Nil(t, appErr) require.NotNil(t, value) } // Delete values for original user - appErr := th.App.DeleteCPAValues(userID) + appErr := th.App.DeleteCPAValues(anonymousCallerId, userID) require.Nil(t, appErr) // Verify other user's values still exist - values, appErr := th.App.ListCPAValues(otherUserID) + values, appErr := th.App.ListCPAValues(anonymousCallerId, otherUserID) require.Nil(t, appErr) require.Len(t, values, 3) }) diff --git a/server/channels/app/migrations.go b/server/channels/app/migrations.go index 850744c0b06..2a3a092c0cb 100644 --- a/server/channels/app/migrations.go +++ b/server/channels/app/migrations.go @@ -617,14 +617,14 @@ func (s *Server) doSetupContentFlaggingProperties() error { } // RegisterPropertyGroup is idempotent, so no need to check if group is already registered - group, err := s.propertyService.RegisterPropertyGroup(model.ContentFlaggingGroupName) + group, err := s.propertyAccessService.RegisterPropertyGroup(model.ContentFlaggingGroupName) if err != nil { return fmt.Errorf("failed to register Content Flagging group: %w", err) } // Using page size of 100 and not iterating through all pages because the // number of fields are static and defined here and not expected to be more than 100 for now. - existingProperties, appErr := s.propertyService.SearchPropertyFields(group.ID, model.PropertyFieldSearchOpts{PerPage: 100}) + existingProperties, appErr := s.propertyAccessService.SearchPropertyFields(anonymousCallerId, group.ID, model.PropertyFieldSearchOpts{PerPage: 100}) if appErr != nil { return fmt.Errorf("failed to search for existing content flagging properties: %w", appErr) } @@ -718,13 +718,13 @@ func (s *Server) doSetupContentFlaggingProperties() error { } for _, property := range propertiesToCreate { - if _, err := s.propertyService.CreatePropertyField(property); err != nil { + if _, err := s.propertyAccessService.CreatePropertyField(anonymousCallerId, property); err != nil { return fmt.Errorf("failed to create content flagging property: %q, error: %w", property.Name, err) } } if len(propertiesToUpdate) > 0 { - if _, err := s.propertyService.UpdatePropertyFields(group.ID, propertiesToUpdate); err != nil { + if _, err := s.propertyAccessService.UpdatePropertyFields(anonymousCallerId, group.ID, propertiesToUpdate); err != nil { return fmt.Errorf("failed to update content flagging property fields: %w", err) } } diff --git a/server/channels/app/migrations_test.go b/server/channels/app/migrations_test.go index e981e0c0263..8af00f4c3b5 100644 --- a/server/channels/app/migrations_test.go +++ b/server/channels/app/migrations_test.go @@ -18,12 +18,12 @@ func TestDoSetupContentFlaggingProperties(t *testing.T) { //settings, setting up the store and initializing services used in store such as property services. th := Setup(t) - group, err := th.Server.propertyService.GetPropertyGroup(model.ContentFlaggingGroupName) + group, err := th.Server.propertyAccessService.GetPropertyGroup(model.ContentFlaggingGroupName) require.NoError(t, err) require.NotNil(t, group) require.Equal(t, model.ContentFlaggingGroupName, group.Name) - propertyFields, err := th.Server.propertyService.SearchPropertyFields(group.ID, model.PropertyFieldSearchOpts{PerPage: 100}) + propertyFields, err := th.Server.propertyAccessService.SearchPropertyFields(anonymousCallerId, group.ID, model.PropertyFieldSearchOpts{PerPage: 100}) require.NoError(t, err) require.Len(t, propertyFields, 11) @@ -43,11 +43,11 @@ func TestDoSetupContentFlaggingProperties(t *testing.T) { err = th.Server.doSetupContentFlaggingProperties() require.NoError(t, err) - group, err := th.Server.propertyService.GetPropertyGroup(model.ContentFlaggingGroupName) + group, err := th.Server.propertyAccessService.GetPropertyGroup(model.ContentFlaggingGroupName) require.NoError(t, err) require.Equal(t, model.ContentFlaggingGroupName, group.Name) - propertyFields, err := th.Server.propertyService.SearchPropertyFields(group.ID, model.PropertyFieldSearchOpts{PerPage: 100}) + propertyFields, err := th.Server.propertyAccessService.SearchPropertyFields(anonymousCallerId, group.ID, model.PropertyFieldSearchOpts{PerPage: 100}) require.NoError(t, err) require.Len(t, propertyFields, 11) diff --git a/server/channels/app/plugin_api.go b/server/channels/app/plugin_api.go index fbaa15c0041..5373c1e08a6 100644 --- a/server/channels/app/plugin_api.go +++ b/server/channels/app/plugin_api.go @@ -1533,100 +1533,99 @@ func (api *PluginAPI) CreatePropertyField(field *model.PropertyField) (*model.Pr if field == nil { return nil, fmt.Errorf("invalid input: property field parameter is required") } - - return api.app.PropertyService().CreatePropertyField(field) + return api.app.PropertyAccessService().CreatePropertyFieldForPlugin(api.manifest.Id, field) } func (api *PluginAPI) GetPropertyField(groupID, fieldID string) (*model.PropertyField, error) { - return api.app.PropertyService().GetPropertyField(groupID, fieldID) + return api.app.PropertyAccessService().GetPropertyField(api.manifest.Id, groupID, fieldID) } func (api *PluginAPI) GetPropertyFields(groupID string, ids []string) ([]*model.PropertyField, error) { - return api.app.PropertyService().GetPropertyFields(groupID, ids) + return api.app.PropertyAccessService().GetPropertyFields(api.manifest.Id, groupID, ids) } func (api *PluginAPI) UpdatePropertyField(groupID string, field *model.PropertyField) (*model.PropertyField, error) { - return api.app.PropertyService().UpdatePropertyField(groupID, field) + return api.app.PropertyAccessService().UpdatePropertyField(api.manifest.Id, groupID, field) } func (api *PluginAPI) DeletePropertyField(groupID, fieldID string) error { - return api.app.PropertyService().DeletePropertyField(groupID, fieldID) + return api.app.PropertyAccessService().DeletePropertyField(api.manifest.Id, groupID, fieldID) } func (api *PluginAPI) SearchPropertyFields(groupID string, opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) { - return api.app.PropertyService().SearchPropertyFields(groupID, opts) + return api.app.PropertyAccessService().SearchPropertyFields(api.manifest.Id, groupID, opts) } func (api *PluginAPI) CountPropertyFields(groupID string, includeDeleted bool) (int64, error) { if includeDeleted { - return api.app.PropertyService().CountAllPropertyFieldsForGroup(groupID) + return api.app.PropertyAccessService().CountAllPropertyFieldsForGroup(groupID) } - return api.app.PropertyService().CountActivePropertyFieldsForGroup(groupID) + return api.app.PropertyAccessService().CountActivePropertyFieldsForGroup(groupID) } func (api *PluginAPI) CountPropertyFieldsForTarget(groupID, targetType, targetID string, includeDeleted bool) (int64, error) { if includeDeleted { - return api.app.PropertyService().CountAllPropertyFieldsForTarget(groupID, targetType, targetID) + return api.app.PropertyAccessService().CountAllPropertyFieldsForTarget(groupID, targetType, targetID) } - return api.app.PropertyService().CountActivePropertyFieldsForTarget(groupID, targetType, targetID) + return api.app.PropertyAccessService().CountActivePropertyFieldsForTarget(groupID, targetType, targetID) } func (api *PluginAPI) CreatePropertyValue(value *model.PropertyValue) (*model.PropertyValue, error) { - return api.app.PropertyService().CreatePropertyValue(value) + return api.app.PropertyAccessService().CreatePropertyValue(api.manifest.Id, value) } func (api *PluginAPI) GetPropertyValue(groupID, valueID string) (*model.PropertyValue, error) { - return api.app.PropertyService().GetPropertyValue(groupID, valueID) + return api.app.PropertyAccessService().GetPropertyValue(api.manifest.Id, groupID, valueID) } func (api *PluginAPI) GetPropertyValues(groupID string, ids []string) ([]*model.PropertyValue, error) { - return api.app.PropertyService().GetPropertyValues(groupID, ids) + return api.app.PropertyAccessService().GetPropertyValues(api.manifest.Id, groupID, ids) } func (api *PluginAPI) UpdatePropertyValue(groupID string, value *model.PropertyValue) (*model.PropertyValue, error) { - return api.app.PropertyService().UpdatePropertyValue(groupID, value) + return api.app.PropertyAccessService().UpdatePropertyValue(api.manifest.Id, groupID, value) } func (api *PluginAPI) UpsertPropertyValue(value *model.PropertyValue) (*model.PropertyValue, error) { - return api.app.PropertyService().UpsertPropertyValue(value) + return api.app.PropertyAccessService().UpsertPropertyValue(api.manifest.Id, value) } func (api *PluginAPI) DeletePropertyValue(groupID, valueID string) error { - return api.app.PropertyService().DeletePropertyValue(groupID, valueID) + return api.app.PropertyAccessService().DeletePropertyValue(api.manifest.Id, groupID, valueID) } func (api *PluginAPI) SearchPropertyValues(groupID string, opts model.PropertyValueSearchOpts) ([]*model.PropertyValue, error) { - return api.app.PropertyService().SearchPropertyValues(groupID, opts) + return api.app.PropertyAccessService().SearchPropertyValues(api.manifest.Id, groupID, opts) } func (api *PluginAPI) RegisterPropertyGroup(name string) (*model.PropertyGroup, error) { - return api.app.PropertyService().RegisterPropertyGroup(name) + return api.app.PropertyAccessService().RegisterPropertyGroup(name) } func (api *PluginAPI) GetPropertyGroup(name string) (*model.PropertyGroup, error) { - return api.app.PropertyService().GetPropertyGroup(name) + return api.app.PropertyAccessService().GetPropertyGroup(name) } func (api *PluginAPI) GetPropertyFieldByName(groupID, targetID, name string) (*model.PropertyField, error) { - return api.app.PropertyService().GetPropertyFieldByName(groupID, targetID, name) + return api.app.PropertyAccessService().GetPropertyFieldByName(api.manifest.Id, groupID, targetID, name) } func (api *PluginAPI) UpdatePropertyFields(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) { - return api.app.PropertyService().UpdatePropertyFields(groupID, fields) + return api.app.PropertyAccessService().UpdatePropertyFields(api.manifest.Id, groupID, fields) } func (api *PluginAPI) UpdatePropertyValues(groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { - return api.app.PropertyService().UpdatePropertyValues(groupID, values) + return api.app.PropertyAccessService().UpdatePropertyValues(api.manifest.Id, groupID, values) } func (api *PluginAPI) UpsertPropertyValues(values []*model.PropertyValue) ([]*model.PropertyValue, error) { - return api.app.PropertyService().UpsertPropertyValues(values) + return api.app.PropertyAccessService().UpsertPropertyValues(api.manifest.Id, values) } func (api *PluginAPI) DeletePropertyValuesForTarget(groupID, targetType, targetID string) error { - return api.app.PropertyService().DeletePropertyValuesForTarget(groupID, targetType, targetID) + return api.app.PropertyAccessService().DeletePropertyValuesForTarget(api.manifest.Id, groupID, targetType, targetID) } func (api *PluginAPI) DeletePropertyValuesForField(groupID, fieldID string) error { - return api.app.PropertyService().DeletePropertyValuesForField(groupID, fieldID) + return api.app.PropertyAccessService().DeletePropertyValuesForField(api.manifest.Id, groupID, fieldID) } diff --git a/server/channels/app/plugin_properties_test.go b/server/channels/app/plugin_properties_test.go index e8b9485fc51..75bf28be202 100644 --- a/server/channels/app/plugin_properties_test.go +++ b/server/channels/app/plugin_properties_test.go @@ -434,4 +434,682 @@ func TestPluginProperties(t *testing.T) { appErr := th.App.ch.RemovePlugin(pluginIDs[0]) require.Nil(t, appErr) }) + + t.Run("test plugin-created CPA field gets source_plugin_id", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + tearDown, pluginIDs, activationErrors := SetAppEnvironmentWithPlugins(t, []string{` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Create a CPA field + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "CPA Test Field", + Type: model.PropertyFieldTypeText, + } + + createdField, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create CPA field: %w", err) + } + + // Verify source_plugin_id was automatically set + if createdField.Attrs == nil { + return fmt.Errorf("field attrs is nil") + } + + sourcePluginID, ok := createdField.Attrs["source_plugin_id"].(string) + if !ok { + return fmt.Errorf("source_plugin_id not found in attrs") + } + + if sourcePluginID != p.API.GetPluginID() { + return fmt.Errorf("source_plugin_id mismatch: expected %s, got %s", p.API.GetPluginID(), sourcePluginID) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `}, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 1) + require.NoError(t, activationErrors[0]) + + // Clean up + err2 := th.App.DisablePlugin(pluginIDs[0]) + require.Nil(t, err2) + appErr := th.App.ch.RemovePlugin(pluginIDs[0]) + require.Nil(t, appErr) + }) + + t.Run("test plugin can update its own protected field", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + tearDown, pluginIDs, activationErrors := SetAppEnvironmentWithPlugins(t, []string{` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Create a protected CPA field + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Protected Field", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "protected": true, + }, + } + + createdField, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create protected field: %w", err) + } + + // Try to update the protected field (should succeed since we created it) + createdField.Name = "Updated Protected Field" + updatedField, err := p.API.UpdatePropertyField("` + cpaID + `", createdField) + if err != nil { + return fmt.Errorf("failed to update own protected field: %w", err) + } + + if updatedField.Name != "Updated Protected Field" { + return fmt.Errorf("field name not updated correctly") + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `}, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 1) + require.NoError(t, activationErrors[0]) + + // Clean up + err2 := th.App.DisablePlugin(pluginIDs[0]) + require.Nil(t, err2) + appErr := th.App.ch.RemovePlugin(pluginIDs[0]) + require.Nil(t, appErr) + }) + + t.Run("test plugin cannot update another plugin's protected field", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + // Both plugins in same environment + tearDown, _, activationErrors := SetAppEnvironmentWithPlugins(t, []string{ + // Plugin 1: creates a protected field + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Create a protected CPA field + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Plugin1 Protected Field", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "protected": true, + }, + } + + _, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create protected field: %w", err) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + // Plugin 2: tries to update plugin1's field + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Search for plugin1's protected field + fields, err := p.API.SearchPropertyFields("` + cpaID + `", model.PropertyFieldSearchOpts{PerPage: 100}) + if err != nil { + return fmt.Errorf("failed to search fields: %w", err) + } + + var plugin1Field *model.PropertyField + for _, field := range fields { + if field.Name == "Plugin1 Protected Field" { + plugin1Field = field + break + } + } + + if plugin1Field == nil { + return fmt.Errorf("plugin1 field not found") + } + + // Attempt to update it (should fail) + plugin1Field.Name = "Hacked By Plugin2" + _, err = p.API.UpdatePropertyField("` + cpaID + `", plugin1Field) + if err == nil { + return fmt.Errorf("expected error when updating another plugin's protected field, but got none") + } + + // Error is expected + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + }, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 2) + require.NoError(t, activationErrors[0]) + require.NoError(t, activationErrors[1]) + }) + + t.Run("test plugin can delete its own protected field", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + tearDown, pluginIDs, activationErrors := SetAppEnvironmentWithPlugins(t, []string{` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Create a protected CPA field + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Field To Delete", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "protected": true, + }, + } + + createdField, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create protected field: %w", err) + } + + // Try to delete the protected field (should succeed since we created it) + err = p.API.DeletePropertyField("` + cpaID + `", createdField.ID) + if err != nil { + return fmt.Errorf("failed to delete own protected field: %w", err) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `}, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 1) + require.NoError(t, activationErrors[0]) + + // Clean up + err2 := th.App.DisablePlugin(pluginIDs[0]) + require.Nil(t, err2) + appErr := th.App.ch.RemovePlugin(pluginIDs[0]) + require.Nil(t, appErr) + }) + + t.Run("test plugin cannot delete another plugin's protected field", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + // Both plugins in same environment + tearDown, _, activationErrors := SetAppEnvironmentWithPlugins(t, []string{ + // Plugin 1: creates a protected field + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Plugin1 Field To Keep", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "protected": true, + }, + } + + _, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create protected field: %w", err) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + // Plugin 2: tries to delete plugin1's field + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Search for plugin1's protected field + fields, err := p.API.SearchPropertyFields("` + cpaID + `", model.PropertyFieldSearchOpts{PerPage: 100}) + if err != nil { + return fmt.Errorf("failed to search fields: %w", err) + } + + var plugin1Field *model.PropertyField + for _, field := range fields { + if field.Name == "Plugin1 Field To Keep" { + plugin1Field = field + break + } + } + + if plugin1Field == nil { + return fmt.Errorf("plugin1 field not found") + } + + // Attempt to delete it (should fail) + err = p.API.DeletePropertyField("` + cpaID + `", plugin1Field.ID) + if err == nil { + return fmt.Errorf("expected error when deleting another plugin's protected field, but got none") + } + + // Error is expected + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + }, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 2) + require.NoError(t, activationErrors[0]) + require.NoError(t, activationErrors[1]) + }) + + t.Run("test plugin can update values for its own protected field", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + tearDown, pluginIDs, activationErrors := SetAppEnvironmentWithPlugins(t, []string{` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Create a protected CPA field + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Protected Field With Values", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "protected": true, + }, + } + + createdField, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create protected field: %w", err) + } + + // Create a value for this field + targetID := model.NewId() + value := &model.PropertyValue{ + GroupID: "` + cpaID + `", + FieldID: createdField.ID, + TargetID: targetID, + TargetType: "user", + Value: []byte("\"initial value\""), + } + + createdValue, err := p.API.CreatePropertyValue(value) + if err != nil { + return fmt.Errorf("failed to create value: %w", err) + } + + // Update the value (should succeed) + createdValue.Value = []byte("\"updated value\"") + updatedValue, err := p.API.UpdatePropertyValue("` + cpaID + `", createdValue) + if err != nil { + return fmt.Errorf("failed to update value for own protected field: %w", err) + } + + if string(updatedValue.Value) != "\"updated value\"" { + return fmt.Errorf("value not updated correctly") + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `}, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 1) + require.NoError(t, activationErrors[0]) + + // Clean up + err2 := th.App.DisablePlugin(pluginIDs[0]) + require.Nil(t, err2) + appErr := th.App.ch.RemovePlugin(pluginIDs[0]) + require.Nil(t, appErr) + }) + + t.Run("test plugin cannot update values for another plugin's protected field", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + testTargetID := model.NewId() + + // Both plugins in same environment + tearDown, _, activationErrors := SetAppEnvironmentWithPlugins(t, []string{ + // Plugin 1: creates a protected field with a value + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Plugin1 Field With Protected Values", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "protected": true, + }, + } + + createdField, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create protected field: %w", err) + } + + // Create a value + value := &model.PropertyValue{ + GroupID: "` + cpaID + `", + FieldID: createdField.ID, + TargetID: "` + testTargetID + `", + TargetType: "user", + Value: []byte("\"plugin1 value\""), + } + + _, err = p.API.CreatePropertyValue(value) + if err != nil { + return fmt.Errorf("failed to create value: %w", err) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + // Plugin 2: tries to update plugin1's field value + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Search for plugin1's protected field + fields, err := p.API.SearchPropertyFields("` + cpaID + `", model.PropertyFieldSearchOpts{PerPage: 100}) + if err != nil { + return fmt.Errorf("failed to search fields: %w", err) + } + + var plugin1Field *model.PropertyField + for _, field := range fields { + if field.Name == "Plugin1 Field With Protected Values" { + plugin1Field = field + break + } + } + + if plugin1Field == nil { + return fmt.Errorf("plugin1 field not found") + } + + // Try to update the value (should fail) + value := &model.PropertyValue{ + GroupID: "` + cpaID + `", + FieldID: plugin1Field.ID, + TargetID: "` + testTargetID + `", + TargetType: "user", + Value: []byte("\"hacked by plugin2\""), + } + + _, err = p.API.UpsertPropertyValue(value) + if err == nil { + return fmt.Errorf("expected error when updating another plugin's protected field value, but got none") + } + + // Error is expected + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + }, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 2) + require.NoError(t, activationErrors[0]) + require.NoError(t, activationErrors[1]) + }) + + t.Run("test plugin can modify non-protected CPA fields from other plugins", func(t *testing.T) { + cpaID, err := th.App.CpaGroupID() + require.NoError(t, err) + + // Both plugins in same environment + tearDown, _, activationErrors := SetAppEnvironmentWithPlugins(t, []string{ + // Plugin 1: creates a NON-protected field + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + field := &model.PropertyField{ + GroupID: "` + cpaID + `", + Name: "Non-Protected Field", + Type: model.PropertyFieldTypeText, + // Note: protected is not set + } + + _, err := p.API.CreatePropertyField(field) + if err != nil { + return fmt.Errorf("failed to create non-protected field: %w", err) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + // Plugin 2: modifies plugin1's non-protected field + ` + package main + + import ( + "fmt" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + // Search for plugin1's non-protected field + fields, err := p.API.SearchPropertyFields("` + cpaID + `", model.PropertyFieldSearchOpts{PerPage: 100}) + if err != nil { + return fmt.Errorf("failed to search fields: %w", err) + } + + var plugin1Field *model.PropertyField + for _, field := range fields { + if field.Name == "Non-Protected Field" { + plugin1Field = field + break + } + } + + if plugin1Field == nil { + return fmt.Errorf("plugin1 field not found") + } + + // Update it (should succeed since it's not protected) + plugin1Field.Name = "Modified By Plugin2" + _, err = p.API.UpdatePropertyField("` + cpaID + `", plugin1Field) + if err != nil { + return fmt.Errorf("failed to update non-protected field: %w", err) + } + + return nil + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + }, th.App, th.NewPluginAPI) + defer tearDown() + require.Len(t, activationErrors, 2) + require.NoError(t, activationErrors[0]) + require.NoError(t, activationErrors[1]) + + // Verify the field was actually updated + updatedFields, appErr := th.App.ListCPAFields("") + require.Nil(t, appErr) + var fieldWasUpdated bool + for _, field := range updatedFields { + if field.Name == "Modified By Plugin2" { + fieldWasUpdated = true + break + } + } + require.True(t, fieldWasUpdated, "Non-protected field should have been updated by plugin2") + }) } diff --git a/server/channels/app/property_access.go b/server/channels/app/property_access.go index 0622125b408..96523428f40 100644 --- a/server/channels/app/property_access.go +++ b/server/channels/app/property_access.go @@ -32,22 +32,37 @@ const ( propertyAccessPaginationPageSize = 100 // propertyAccessMaxPaginationIterations is the maximum number of pagination iterations before returning an error propertyAccessMaxPaginationIterations = 10 + // anonymousCallerId can be used for calls to the service that aren't tied to a specific entity + // These calls will not be able to access any data that has access control restrictions. + anonymousCallerId = "" ) +// PluginChecker is a function type that checks if a plugin is installed. +// Returns true if the plugin exists and is installed, false otherwise. +type PluginChecker func(pluginID string) bool + // PropertyAccessService is a decorator around PropertyService that enforces // access control based on caller identity. All property operations go through // this service to ensure consistent access control enforcement. type PropertyAccessService struct { propertyService *properties.PropertyService + pluginChecker PluginChecker } // NewPropertyAccessService creates a new PropertyAccessService wrapping the given PropertyService. -func NewPropertyAccessService(ps *properties.PropertyService) *PropertyAccessService { +// The pluginChecker function is used to verify plugin installation status when checking access +// to protected fields. Pass nil if plugin checking is not needed (e.g., in tests). +func NewPropertyAccessService(ps *properties.PropertyService, pluginChecker PluginChecker) *PropertyAccessService { return &PropertyAccessService{ propertyService: ps, + pluginChecker: pluginChecker, } } +func (pas *PropertyAccessService) setPluginCheckerForTests(pluginChecker PluginChecker) { + pas.pluginChecker = pluginChecker +} + // Property Group Methods // RegisterPropertyGroup registers a new property group. @@ -65,6 +80,12 @@ func (pas *PropertyAccessService) GetPropertyGroup(name string) (*model.Property // CreatePropertyField creates a new property field. // This method rejects any attempt to set source_plugin_id - only plugins can set this via CreatePropertyFieldForPlugin. func (pas *PropertyAccessService) CreatePropertyField(callerID string, field *model.PropertyField) (*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(field.GroupID); err != nil { + return nil, fmt.Errorf("CreatePropertyField: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.CreatePropertyField(field) + } + // Reject if source_plugin_id is set to a non-empty value - only plugins can set this via CreatePropertyFieldForPlugin if pas.getSourcePluginID(field) != "" { return nil, fmt.Errorf("CreatePropertyField: source_plugin_id cannot be set directly, it is only set automatically for plugin-created fields") @@ -91,6 +112,12 @@ func (pas *PropertyAccessService) CreatePropertyField(callerID string, field *mo // This method automatically sets the source_plugin_id to the provided pluginID. // Only use this method when creating fields through the Plugin API. func (pas *PropertyAccessService) CreatePropertyFieldForPlugin(pluginID string, field *model.PropertyField) (*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(field.GroupID); err != nil { + return nil, fmt.Errorf("CreatePropertyFieldForPlugin: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.CreatePropertyField(field) + } + if pluginID == "" { return nil, fmt.Errorf("CreatePropertyFieldForPlugin: pluginID is required") } @@ -118,6 +145,12 @@ func (pas *PropertyAccessService) CreatePropertyFieldForPlugin(pluginID string, // GetPropertyField retrieves a property field by group and field ID. // Field details are filtered based on the caller's access permissions. func (pas *PropertyAccessService) GetPropertyField(callerID string, groupID, id string) (*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("GetPropertyField: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.GetPropertyField(groupID, id) + } + field, err := pas.propertyService.GetPropertyField(groupID, id) if err != nil { return nil, fmt.Errorf("GetPropertyField: %w", err) @@ -129,6 +162,12 @@ func (pas *PropertyAccessService) GetPropertyField(callerID string, groupID, id // GetPropertyFields retrieves multiple property fields by their IDs. // Field details are filtered based on the caller's access permissions. func (pas *PropertyAccessService) GetPropertyFields(callerID string, groupID string, ids []string) ([]*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("GetPropertyFields: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.GetPropertyFields(groupID, ids) + } + fields, err := pas.propertyService.GetPropertyFields(groupID, ids) if err != nil { return nil, fmt.Errorf("GetPropertyFields: %w", err) @@ -140,6 +179,12 @@ func (pas *PropertyAccessService) GetPropertyFields(callerID string, groupID str // GetPropertyFieldByName retrieves a property field by name. // Field details are filtered based on the caller's access permissions. func (pas *PropertyAccessService) GetPropertyFieldByName(callerID string, groupID, targetID, name string) (*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("GetPropertyFieldByName: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.GetPropertyFieldByName(groupID, targetID, name) + } + field, err := pas.propertyService.GetPropertyFieldByName(groupID, targetID, name) if err != nil { return nil, fmt.Errorf("GetPropertyFieldByName: %w", err) @@ -171,6 +216,12 @@ func (pas *PropertyAccessService) CountAllPropertyFieldsForTarget(groupID, targe // SearchPropertyFields searches for property fields based on the given options. // Field details are filtered based on the caller's access permissions. func (pas *PropertyAccessService) SearchPropertyFields(callerID string, groupID string, opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("SearchPropertyFields: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.SearchPropertyFields(groupID, opts) + } + fields, err := pas.propertyService.SearchPropertyFields(groupID, opts) if err != nil { return nil, fmt.Errorf("SearchPropertyFields: %w", err) @@ -182,6 +233,12 @@ func (pas *PropertyAccessService) SearchPropertyFields(callerID string, groupID // UpdatePropertyField updates a property field. // Checks write access and ensures source_plugin_id is not changed. func (pas *PropertyAccessService) UpdatePropertyField(callerID string, groupID string, field *model.PropertyField) (*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("UpdatePropertyField: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.UpdatePropertyField(groupID, field) + } + // Get existing field to check access existingField, existsErr := pas.propertyService.GetPropertyField(groupID, field.ID) if existsErr != nil { @@ -213,6 +270,12 @@ func (pas *PropertyAccessService) UpdatePropertyField(callerID string, groupID s // UpdatePropertyFields updates multiple property fields. // Checks write access for all fields atomically before updating any. func (pas *PropertyAccessService) UpdatePropertyFields(callerID string, groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("UpdatePropertyFields: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.UpdatePropertyFields(groupID, fields) + } + if len(fields) == 0 { return fields, nil } @@ -267,16 +330,22 @@ func (pas *PropertyAccessService) UpdatePropertyFields(callerID string, groupID } // DeletePropertyField deletes a property field and all its values. -// Checks write access before allowing deletion. +// Checks delete access before allowing deletion. func (pas *PropertyAccessService) DeletePropertyField(callerID string, groupID, id string) error { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return fmt.Errorf("DeletePropertyField: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.DeletePropertyField(groupID, id) + } + // Get existing field to check access existingField, err := pas.propertyService.GetPropertyField(groupID, id) if err != nil { return fmt.Errorf("DeletePropertyField: %w", err) } - // Check write access - if err := pas.checkFieldWriteAccess(existingField, callerID); err != nil { + // Check delete access + if err := pas.checkFieldDeleteAccess(existingField, callerID); err != nil { return fmt.Errorf("DeletePropertyField: %w", err) } @@ -291,6 +360,12 @@ func (pas *PropertyAccessService) DeletePropertyField(callerID string, groupID, // CreatePropertyValue creates a new property value. // Checks write access before allowing the creation. func (pas *PropertyAccessService) CreatePropertyValue(callerID string, value *model.PropertyValue) (*model.PropertyValue, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(value.GroupID); err != nil { + return nil, fmt.Errorf("CreatePropertyValue: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.CreatePropertyValue(value) + } + // Get the associated field to check access field, err := pas.propertyService.GetPropertyField(value.GroupID, value.FieldID) if err != nil { @@ -312,6 +387,19 @@ func (pas *PropertyAccessService) CreatePropertyValue(callerID string, value *mo // CreatePropertyValues creates multiple property values. // Checks write access for all fields atomically before creating any values. func (pas *PropertyAccessService) CreatePropertyValues(callerID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { + shouldApplyAccessControl := false + for _, value := range values { + if hasRestrictions, err := pas.groupHasAccessRestrictions(value.GroupID); err != nil { + return nil, fmt.Errorf("CreatePropertyValues: cannot determine access restrictions: %w", err) + } else if hasRestrictions { + shouldApplyAccessControl = true + break + } + } + if !shouldApplyAccessControl { + return pas.propertyService.CreatePropertyValues(values) + } + fieldMap, err := pas.getFieldsForValues(values) if err != nil { return nil, fmt.Errorf("CreatePropertyValues: %w", err) @@ -340,6 +428,12 @@ func (pas *PropertyAccessService) CreatePropertyValues(callerID string, values [ // GetPropertyValue retrieves a property value by ID. // Returns (nil, nil) if the value exists but the caller doesn't have access. func (pas *PropertyAccessService) GetPropertyValue(callerID string, groupID, id string) (*model.PropertyValue, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("GetPropertyValue: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.GetPropertyValue(groupID, id) + } + value, err := pas.propertyService.GetPropertyValue(groupID, id) if err != nil { return nil, fmt.Errorf("GetPropertyValue: %w", err) @@ -362,6 +456,12 @@ func (pas *PropertyAccessService) GetPropertyValue(callerID string, groupID, id // GetPropertyValues retrieves multiple property values by their IDs. // Values the caller doesn't have access to are silently filtered out. func (pas *PropertyAccessService) GetPropertyValues(callerID string, groupID string, ids []string) ([]*model.PropertyValue, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("GetPropertyValues: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.GetPropertyValues(groupID, ids) + } + values, err := pas.propertyService.GetPropertyValues(groupID, ids) if err != nil { return nil, fmt.Errorf("GetPropertyValues: %w", err) @@ -378,6 +478,12 @@ func (pas *PropertyAccessService) GetPropertyValues(callerID string, groupID str // SearchPropertyValues searches for property values based on the given options. // Values the caller doesn't have access to are silently filtered out. func (pas *PropertyAccessService) SearchPropertyValues(callerID string, groupID string, opts model.PropertyValueSearchOpts) ([]*model.PropertyValue, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("SearchPropertyValues: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.SearchPropertyValues(groupID, opts) + } + values, err := pas.propertyService.SearchPropertyValues(groupID, opts) if err != nil { return nil, fmt.Errorf("SearchPropertyValues: %w", err) @@ -394,6 +500,12 @@ func (pas *PropertyAccessService) SearchPropertyValues(callerID string, groupID // UpdatePropertyValue updates a property value. // Checks write access before allowing the update. func (pas *PropertyAccessService) UpdatePropertyValue(callerID string, groupID string, value *model.PropertyValue) (*model.PropertyValue, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return nil, fmt.Errorf("UpdatePropertyValue: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.UpdatePropertyValue(groupID, value) + } + // Get the associated field to check access field, err := pas.propertyService.GetPropertyField(groupID, value.FieldID) if err != nil { @@ -415,6 +527,19 @@ func (pas *PropertyAccessService) UpdatePropertyValue(callerID string, groupID s // UpdatePropertyValues updates multiple property values. // Checks write access for all fields atomically before updating any values. func (pas *PropertyAccessService) UpdatePropertyValues(callerID string, groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { + shouldApplyAccessControl := false + for _, value := range values { + if hasRestrictions, err := pas.groupHasAccessRestrictions(value.GroupID); err != nil { + return nil, fmt.Errorf("UpdatePropertyValues: cannot determine access restrictions: %w", err) + } else if hasRestrictions { + shouldApplyAccessControl = true + break + } + } + if !shouldApplyAccessControl { + return pas.propertyService.UpdatePropertyValues(groupID, values) + } + if len(values) == 0 { return values, nil } @@ -447,6 +572,12 @@ func (pas *PropertyAccessService) UpdatePropertyValues(callerID string, groupID // UpsertPropertyValue creates or updates a property value. // Checks write access before allowing the upsert. func (pas *PropertyAccessService) UpsertPropertyValue(callerID string, value *model.PropertyValue) (*model.PropertyValue, error) { + if hasRestrictions, err := pas.groupHasAccessRestrictions(value.GroupID); err != nil { + return nil, fmt.Errorf("UpsertPropertyValue cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.UpsertPropertyValue(value) + } + // Get the associated field to check access field, err := pas.propertyService.GetPropertyField(value.GroupID, value.FieldID) if err != nil { @@ -468,6 +599,19 @@ func (pas *PropertyAccessService) UpsertPropertyValue(callerID string, value *mo // UpsertPropertyValues creates or updates multiple property values. // Checks write access for all fields atomically before upserting any values. func (pas *PropertyAccessService) UpsertPropertyValues(callerID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { + shouldApplyAccessControl := false + for _, value := range values { + if hasRestrictions, err := pas.groupHasAccessRestrictions(value.GroupID); err != nil { + return nil, fmt.Errorf("UpsertPropertyValues: cannot determine access restrictions: %w", err) + } else if hasRestrictions { + shouldApplyAccessControl = true + break + } + } + if !shouldApplyAccessControl { + return pas.propertyService.UpsertPropertyValues(values) + } + if len(values) == 0 { return values, nil } @@ -500,6 +644,12 @@ func (pas *PropertyAccessService) UpsertPropertyValues(callerID string, values [ // DeletePropertyValue deletes a property value. // Checks write access before allowing deletion. func (pas *PropertyAccessService) DeletePropertyValue(callerID string, groupID, id string) error { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return fmt.Errorf("DeletePropertyValue: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.DeletePropertyValue(groupID, id) + } + // Get the value to find its field ID value, err := pas.propertyService.GetPropertyValue(groupID, id) if err != nil { @@ -527,6 +677,12 @@ func (pas *PropertyAccessService) DeletePropertyValue(callerID string, groupID, // DeletePropertyValuesForTarget deletes all property values for a specific target. // Checks write access for all affected fields atomically before deleting. func (pas *PropertyAccessService) DeletePropertyValuesForTarget(callerID string, groupID string, targetType string, targetID string) error { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return fmt.Errorf("DeletePropertyValuesForTarget: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.DeletePropertyValuesForTarget(groupID, targetType, targetID) + } + // Collect unique field IDs across all values without loading all values into memory fieldIDs := make(map[string]struct{}) var cursor model.PropertyValueSearchCursor @@ -605,6 +761,12 @@ func (pas *PropertyAccessService) DeletePropertyValuesForTarget(callerID string, // DeletePropertyValuesForField deletes all property values for a specific field. // Checks write access before allowing deletion. func (pas *PropertyAccessService) DeletePropertyValuesForField(callerID string, groupID, fieldID string) error { + if hasRestrictions, err := pas.groupHasAccessRestrictions(groupID); err != nil { + return fmt.Errorf("DeletePropertyValuesForField: cannot determine access restrictions: %w", err) + } else if !hasRestrictions { + return pas.propertyService.DeletePropertyValuesForField(groupID, fieldID) + } + // Get the field to check access field, err := pas.propertyService.GetPropertyField(groupID, fieldID) if err != nil { @@ -705,6 +867,35 @@ func (pas *PropertyAccessService) checkFieldWriteAccess(field *model.PropertyFie return nil } +// checkFieldDeleteAccess checks if the given caller can delete a PropertyField. +// IMPORTANT: Always pass the existing field fetched from the database, not a field provided by the caller. +// Returns nil if deletion is allowed, or an error if denied. +func (pas *PropertyAccessService) checkFieldDeleteAccess(field *model.PropertyField, callerID string) error { + // Check if field is protected + if !model.IsPropertyFieldProtected(field) { + return nil + } + + // Protected fields can only be deleted by the source plugin + sourcePluginID := pas.getSourcePluginID(field) + if sourcePluginID == "" { + // Protected field with no source plugin - allow deletion + return nil + } + + // Check if the source plugin is still installed + if pas.pluginChecker != nil && !pas.pluginChecker(sourcePluginID) { + // Plugin has been uninstalled - allow deletion of orphaned field + return nil + } + + if sourcePluginID != callerID { + return fmt.Errorf("field %s is protected and can only be modified by source plugin '%s'", field.ID, sourcePluginID) + } + + return nil +} + // getCallerValuesForField retrieves all property values for the caller on a specific field. // This is used internally for shared_only filtering. // Returns an empty slice if callerID is empty or if there are no values. @@ -1076,3 +1267,11 @@ func (pas *PropertyAccessService) applyValueReadAccessControl(values []*model.Pr return filtered, nil } + +func (pas *PropertyAccessService) groupHasAccessRestrictions(groupId string) (bool, error) { + cpaID, err := getCpaGroupID(pas) + if err != nil { + return false, err + } + return groupId == cpaID, nil +} diff --git a/server/channels/app/property_access_test.go b/server/channels/app/property_access_test.go index 8af9e5cbfa1..4144a42d94d 100644 --- a/server/channels/app/property_access_test.go +++ b/server/channels/app/property_access_test.go @@ -17,9 +17,10 @@ func TestGetPropertyFieldReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, err := pas.RegisterPropertyGroup("test-group") + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") require.NoError(t, err) + cpaGroupID = group.ID pluginID1 := "plugin-1" pluginID2 := "plugin-2" @@ -326,6 +327,42 @@ func TestGetPropertyFieldReadAccess(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "invalid access mode") }) + + t.Run("non-CPA group source_only field - everyone sees all options", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-read") + require.NoError(t, err) + + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "source-only-non-cpa", + Type: model.PropertyFieldTypeSelect, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + model.PropertyFieldAttributeOptions: []any{ + map[string]any{"id": "opt1", "value": "Secret Option 1"}, + map[string]any{"id": "opt2", "value": "Secret Option 2"}, + }, + }, + } + created, err := pas.CreatePropertyField("", field) + require.NoError(t, err) + + // Non-source plugin sees all options (no filtering) + retrieved, err := pas.GetPropertyField(pluginID2, nonCpaGroup.ID, created.ID) + require.NoError(t, err) + assert.Equal(t, created.ID, retrieved.ID) + assert.Len(t, retrieved.Attrs[model.PropertyFieldAttributeOptions].([]any), 2) + + // User sees all options (no filtering) + retrieved, err = pas.GetPropertyField(userID, nonCpaGroup.ID, created.ID) + require.NoError(t, err) + assert.Equal(t, created.ID, retrieved.ID) + assert.Len(t, retrieved.Attrs[model.PropertyFieldAttributeOptions].([]any), 2) + }) } func TestGetPropertyFieldsReadAccess(t *testing.T) { @@ -333,9 +370,10 @@ func TestGetPropertyFieldsReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, err := pas.RegisterPropertyGroup("test-group-batch") + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") require.NoError(t, err) + cpaGroupID = group.ID pluginID := "plugin-1" userID := model.NewId() @@ -435,9 +473,10 @@ func TestSearchPropertyFieldsReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, err := pas.RegisterPropertyGroup("test-group-search") + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") require.NoError(t, err) + cpaGroupID = group.ID pluginID := "plugin-1" userID := model.NewId() @@ -541,9 +580,10 @@ func TestGetPropertyFieldByNameReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, err := pas.RegisterPropertyGroup("test-group-byname") + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") require.NoError(t, err) + cpaGroupID = group.ID pluginID := "plugin-1" userID := model.NewId() @@ -583,7 +623,14 @@ func TestGetPropertyFieldByNameReadAccess(t *testing.T) { func TestCreatePropertyField_SourcePluginIDValidation(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows field creation without source_plugin_id", func(t *testing.T) { field := &model.PropertyField{ @@ -680,13 +727,62 @@ func TestCreatePropertyField_SourcePluginIDValidation(t *testing.T) { assert.Nil(t, created) assert.Contains(t, err.Error(), "protected can only be set by plugins") }) + + t.Run("non-CPA group allows protected attribute", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group") + require.NoError(t, err) + + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: model.NewId(), + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + }, + } + + // Should succeed because access control doesn't apply to non-CPA groups + created, err := th.App.PropertyAccessService().CreatePropertyField("user1", field) + require.NoError(t, err) + assert.NotNil(t, created) + assert.True(t, created.Attrs[model.PropertyAttrsProtected].(bool)) + }) + + t.Run("non-CPA group allows source_plugin_id to be set", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-2") + require.NoError(t, err) + + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: model.NewId(), + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsSourcePluginID: "some-plugin", + }, + } + + // Should succeed because access control doesn't apply to non-CPA groups + created, err := th.App.PropertyAccessService().CreatePropertyField("user1", field) + require.NoError(t, err) + assert.NotNil(t, created) + assert.Equal(t, "some-plugin", created.Attrs[model.PropertyAttrsSourcePluginID]) + }) } // TestCreatePropertyFieldForPlugin tests the plugin-specific field creation method func TestCreatePropertyFieldForPlugin(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("automatically sets source_plugin_id", func(t *testing.T) { field := &model.PropertyField{ @@ -759,7 +855,14 @@ func TestCreatePropertyFieldForPlugin(t *testing.T) { func TestUpdatePropertyField_WriteAccessControl(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows update of unprotected field", func(t *testing.T) { field := &model.PropertyField{ @@ -855,13 +958,45 @@ func TestUpdatePropertyField_WriteAccessControl(t *testing.T) { assert.Nil(t, updated) assert.Contains(t, err.Error(), "immutable") }) + + t.Run("non-CPA group allows anyone to update protected field", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-update") + require.NoError(t, err) + + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "Protected Field Non-CPA", + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: "plugin1", + }, + } + + created, err := th.App.PropertyAccessService().CreatePropertyField("", field) + require.NoError(t, err) + + created.Name = "Updated by Different Plugin" + // Should succeed - plugin2 can update plugin1's protected field in non-CPA group + updated, err := th.App.PropertyAccessService().UpdatePropertyField("plugin2", nonCpaGroup.ID, created) + require.NoError(t, err) + assert.Equal(t, "Updated by Different Plugin", updated.Name) + }) } // TestUpdatePropertyFields_BulkWriteAccessControl tests bulk field updates with atomic access checking func TestUpdatePropertyFields_BulkWriteAccessControl(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows bulk update of unprotected fields", func(t *testing.T) { field1 := &model.PropertyField{GroupID: groupID, Name: "Field1", Type: model.PropertyFieldTypeText} @@ -922,7 +1057,14 @@ func TestUpdatePropertyFields_BulkWriteAccessControl(t *testing.T) { func TestDeletePropertyField_WriteAccessControl(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows deletion of unprotected field", func(t *testing.T) { field := &model.PropertyField{GroupID: groupID, Name: "Unprotected", Type: model.PropertyFieldTypeText} @@ -950,6 +1092,10 @@ func TestDeletePropertyField_WriteAccessControl(t *testing.T) { }) t.Run("denies non-source plugin deleting protected field", func(t *testing.T) { + pas.setPluginCheckerForTests(func(pluginID string) bool { + return pluginID == "plugin1" + }) + field := &model.PropertyField{ GroupID: groupID, Name: "Protected", @@ -958,20 +1104,49 @@ func TestDeletePropertyField_WriteAccessControl(t *testing.T) { model.PropertyAttrsProtected: true, }, } - created, err := th.App.PropertyAccessService().CreatePropertyFieldForPlugin("plugin1", field) + created, err := pas.CreatePropertyFieldForPlugin("plugin1", field) require.NoError(t, err) - err = th.App.PropertyAccessService().DeletePropertyField("plugin2", groupID, created.ID) + err = pas.DeletePropertyField("plugin2", groupID, created.ID) require.Error(t, err) assert.Contains(t, err.Error(), "protected") }) + + t.Run("non-CPA group allows anyone to delete protected field", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-delete") + require.NoError(t, err) + + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "Protected Non-CPA", + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: "plugin1", + }, + } + created, err := th.App.PropertyAccessService().CreatePropertyField("", field) + require.NoError(t, err) + + // Should succeed - plugin2 can delete plugin1's protected field in non-CPA group + err = th.App.PropertyAccessService().DeletePropertyField("plugin2", nonCpaGroup.ID, created.ID) + require.NoError(t, err) + }) } // TestCreatePropertyValue_WriteAccessControl tests write access control for value creation func TestCreatePropertyValue_WriteAccessControl(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows creating value for public field", func(t *testing.T) { field := &model.PropertyField{GroupID: groupID, Name: "Public", Type: model.PropertyFieldTypeText} @@ -1042,13 +1217,51 @@ func TestCreatePropertyValue_WriteAccessControl(t *testing.T) { assert.Nil(t, createdValue) assert.Contains(t, err.Error(), "protected") }) + + t.Run("non-CPA group allows anyone to create value for protected field", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-value-create") + require.NoError(t, err) + + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "Protected Non-CPA", + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: "plugin1", + }, + } + created, err := th.App.PropertyAccessService().CreatePropertyField("", field) + require.NoError(t, err) + + value := &model.PropertyValue{ + GroupID: nonCpaGroup.ID, + FieldID: created.ID, + TargetType: "user", + TargetID: model.NewId(), + Value: json.RawMessage(`"value from plugin2"`), + } + + // Should succeed - plugin2 can create value on plugin1's protected field in non-CPA group + createdValue, err := th.App.PropertyAccessService().CreatePropertyValue("plugin2", value) + require.NoError(t, err) + assert.NotNil(t, createdValue) + }) } // TestDeletePropertyValue_WriteAccessControl tests write access control for value deletion func TestDeletePropertyValue_WriteAccessControl(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows deleting value for public field", func(t *testing.T) { field := &model.PropertyField{GroupID: groupID, Name: "Public", Type: model.PropertyFieldTypeText} @@ -1101,7 +1314,14 @@ func TestDeletePropertyValue_WriteAccessControl(t *testing.T) { func TestDeletePropertyValuesForTarget_WriteAccessControl(t *testing.T) { th := Setup(t) - groupID := model.NewId() + pas := th.App.PropertyAccessService() + + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") + require.NoError(t, err) + cpaGroupID = group.ID + + groupID := cpaGroupID t.Run("allows deleting all values when caller has write access to all fields", func(t *testing.T) { field1 := &model.PropertyField{GroupID: groupID, Name: "Field1", Type: model.PropertyFieldTypeText} @@ -1172,9 +1392,10 @@ func TestGetPropertyValueReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, rerr := pas.RegisterPropertyGroup("test-group-values") + // Register the CPA group + group, rerr := pas.RegisterPropertyGroup("cpa") require.NoError(t, rerr) + cpaGroupID = group.ID pluginID1 := "plugin-1" pluginID2 := "plugin-2" @@ -1503,6 +1724,54 @@ func TestGetPropertyValueReadAccess(t *testing.T) { require.NoError(t, err) assert.Nil(t, retrieved) }) + + t.Run("non-CPA group source_only value - everyone can read", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-value-read") + require.NoError(t, err) + + // Create source_only field + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "source-only-non-cpa", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + }, + } + field, err = pas.CreatePropertyField("", field) + require.NoError(t, err) + + // Create value + textValue, err := json.Marshal("secret value") + require.NoError(t, err) + value := &model.PropertyValue{ + GroupID: nonCpaGroup.ID, + FieldID: field.ID, + TargetType: "user", + TargetID: userID1, + Value: textValue, + } + value, err = pas.CreatePropertyValue("", value) + require.NoError(t, err) + + // Non-source plugin can read (no filtering) + retrieved, err := pas.GetPropertyValue(pluginID2, nonCpaGroup.ID, value.ID) + require.NoError(t, err) + require.NotNil(t, retrieved) + assert.Equal(t, value.ID, retrieved.ID) + assert.Equal(t, json.RawMessage(textValue), retrieved.Value) + + // User can read (no filtering) + retrieved, err = pas.GetPropertyValue(userID2, nonCpaGroup.ID, value.ID) + require.NoError(t, err) + require.NotNil(t, retrieved) + assert.Equal(t, value.ID, retrieved.ID) + assert.Equal(t, json.RawMessage(textValue), retrieved.Value) + }) } func TestGetPropertyValuesReadAccess(t *testing.T) { @@ -1510,9 +1779,10 @@ func TestGetPropertyValuesReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, rerr := pas.RegisterPropertyGroup("test-group-bulk-values") + // Register the CPA group + group, rerr := pas.RegisterPropertyGroup("cpa") require.NoError(t, rerr) + cpaGroupID = group.ID pluginID1 := "plugin-1" pluginID2 := "plugin-2" @@ -1595,9 +1865,10 @@ func TestSearchPropertyValuesReadAccess(t *testing.T) { pas := th.App.PropertyAccessService() - // Register a test group - group, rerr := pas.RegisterPropertyGroup("test-group-search-values") + // Register the CPA group + group, rerr := pas.RegisterPropertyGroup("cpa") require.NoError(t, rerr) + cpaGroupID = group.ID pluginID1 := "plugin-1" pluginID2 := "plugin-2" @@ -1740,8 +2011,10 @@ func TestCreatePropertyValues_WriteAccessControl(t *testing.T) { pas := th.App.PropertyAccessService() - group, err := pas.RegisterPropertyGroup("test-group-create-values") + // Register the CPA group + group, err := pas.RegisterPropertyGroup("cpa") require.NoError(t, err) + cpaGroupID = group.ID pluginID1 := "plugin-1" pluginID2 := "plugin-2" @@ -2063,4 +2336,278 @@ func TestCreatePropertyValues_WriteAccessControl(t *testing.T) { require.NoError(t, err) assert.Empty(t, results3) }) + + t.Run("non-CPA group allows bulk creation of values for protected fields", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-bulk-create") + require.NoError(t, err) + + // Create protected fields + field1 := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "protected-bulk-1", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + }, + } + field1, err = pas.CreatePropertyField("", field1) + require.NoError(t, err) + + field2 := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "protected-bulk-2", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + }, + } + field2, err = pas.CreatePropertyField("", field2) + require.NoError(t, err) + + targetID := model.NewId() + value1, err := json.Marshal("data1") + require.NoError(t, err) + value2, err := json.Marshal("data2") + require.NoError(t, err) + + values := []*model.PropertyValue{ + { + GroupID: nonCpaGroup.ID, + FieldID: field1.ID, + TargetType: "user", + TargetID: targetID, + Value: value1, + }, + { + GroupID: nonCpaGroup.ID, + FieldID: field2.ID, + TargetType: "user", + TargetID: targetID, + Value: value2, + }, + } + + // Should succeed - plugin2 can create values on plugin1's protected fields in non-CPA group + created, err := pas.CreatePropertyValues(pluginID2, values) + require.NoError(t, err) + assert.Len(t, created, 2) + }) + + t.Run("non-CPA group allows bulk upsert of values for protected fields", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-bulk-upsert") + require.NoError(t, err) + + // Create protected field + field := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "protected-upsert", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + }, + } + field, err = pas.CreatePropertyField("", field) + require.NoError(t, err) + + targetID := model.NewId() + value1, err := json.Marshal("initial value") + require.NoError(t, err) + + values := []*model.PropertyValue{ + { + GroupID: nonCpaGroup.ID, + FieldID: field.ID, + TargetType: "user", + TargetID: targetID, + Value: value1, + }, + } + + // Should succeed - plugin2 can upsert values on plugin1's protected field in non-CPA group + created, err := pas.UpsertPropertyValues(pluginID2, values) + require.NoError(t, err) + assert.Len(t, created, 1) + + // Update the value + value2, err := json.Marshal("updated value") + require.NoError(t, err) + values[0].Value = value2 + + // Should succeed again + updated, err := pas.UpsertPropertyValues(pluginID2, values) + require.NoError(t, err) + assert.Len(t, updated, 1) + + var retrievedValue string + err = json.Unmarshal(updated[0].Value, &retrievedValue) + require.NoError(t, err) + assert.Equal(t, "updated value", retrievedValue) + }) + + t.Run("mixed CPA and non-CPA groups - enforces access control only on CPA group", func(t *testing.T) { + // Register a non-CPA group + nonCpaGroup, err := pas.RegisterPropertyGroup("other-group-mixed") + require.NoError(t, err) + + // Create protected field in CPA group + cpaField := &model.PropertyField{ + GroupID: group.ID, + Name: "cpa-protected-mixed", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + }, + } + cpaField, err = pas.CreatePropertyFieldForPlugin(pluginID1, cpaField) + require.NoError(t, err) + + // Create protected field in non-CPA group + nonCpaField := &model.PropertyField{ + GroupID: nonCpaGroup.ID, + Name: "non-cpa-protected-mixed", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + model.PropertyAttrsAccessMode: model.PropertyAccessModeSourceOnly, + model.PropertyAttrsProtected: true, + model.PropertyAttrsSourcePluginID: pluginID1, + }, + } + nonCpaField, err = pas.CreatePropertyField("", nonCpaField) + require.NoError(t, err) + + targetID := model.NewId() + cpaValue, err := json.Marshal("cpa data") + require.NoError(t, err) + nonCpaValue, err := json.Marshal("non-cpa data") + require.NoError(t, err) + + values := []*model.PropertyValue{ + { + GroupID: group.ID, + FieldID: cpaField.ID, + TargetType: "user", + TargetID: targetID, + Value: cpaValue, + }, + { + GroupID: nonCpaGroup.ID, + FieldID: nonCpaField.ID, + TargetType: "user", + TargetID: targetID, + Value: nonCpaValue, + }, + } + + // Should fail - plugin2 cannot create value on CPA group protected field + created, err := pas.CreatePropertyValues(pluginID2, values) + require.Error(t, err) + assert.Nil(t, created) + assert.Contains(t, err.Error(), "protected") + + // Verify no values were created (atomic failure) + results, err := pas.SearchPropertyValues(pluginID1, group.ID, model.PropertyValueSearchOpts{ + TargetIDs: []string{targetID}, + PerPage: 100, + }) + require.NoError(t, err) + assert.Empty(t, results) + + results, err = pas.SearchPropertyValues(pluginID1, nonCpaGroup.ID, model.PropertyValueSearchOpts{ + TargetIDs: []string{targetID}, + PerPage: 100, + }) + require.NoError(t, err) + assert.Empty(t, results) + }) +} + +func TestDeletePropertyField_OrphanedFieldDeletion(t *testing.T) { + th := Setup(t) + + groupID, err := th.App.CpaGroupID() + require.NoError(t, err) + pas := th.App.PropertyAccessService() + + t.Run("allows deletion of orphaned protected field when plugin is uninstalled", func(t *testing.T) { + pas.setPluginCheckerForTests(func(pluginID string) bool { + return false + }) + + field := &model.PropertyField{ + GroupID: groupID, + Name: "Orphaned Protected Field", + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + }, + } + created, err := pas.CreatePropertyFieldForPlugin("removed-plugin", field) + require.NoError(t, err) + + err = pas.DeletePropertyField("admin-user", groupID, created.ID) + require.NoError(t, err) + }) + + t.Run("blocks deletion of protected field when plugin is still installed", func(t *testing.T) { + pas.setPluginCheckerForTests(func(pluginID string) bool { + return pluginID == "installed-plugin" + }) + + field := &model.PropertyField{ + GroupID: groupID, + Name: "Active Protected Field", + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + }, + } + created, err := pas.CreatePropertyFieldForPlugin("installed-plugin", field) + require.NoError(t, err) + + err = pas.DeletePropertyField("admin-user", groupID, created.ID) + require.Error(t, err) + assert.Contains(t, err.Error(), "protected") + assert.Contains(t, err.Error(), "installed-plugin") + + err = pas.DeletePropertyField("installed-plugin", groupID, created.ID) + require.NoError(t, err) + }) + + t.Run("blocks update of orphaned protected field even when plugin is uninstalled", func(t *testing.T) { + pas.setPluginCheckerForTests(func(pluginID string) bool { + return false + }) + + field := &model.PropertyField{ + GroupID: groupID, + Name: "Orphaned Field For Update", + Type: model.PropertyFieldTypeText, + Attrs: model.StringInterface{ + model.PropertyAttrsProtected: true, + }, + } + created, err := pas.CreatePropertyFieldForPlugin("removed-plugin", field) + require.NoError(t, err) + + created.Name = "Updated Orphaned Field" + updated, err := pas.UpdatePropertyField("admin-user", groupID, created) + require.Error(t, err) + assert.Nil(t, updated) + assert.Contains(t, err.Error(), "protected") + }) } diff --git a/server/channels/app/server.go b/server/channels/app/server.go index 4e037b42a36..bb6abb9386c 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -130,7 +130,6 @@ type Server struct { telemetryService *telemetry.TelemetryService userService *users.UserService teamService *teams.TeamService - propertyService *properties.PropertyService propertyAccessService *PropertyAccessService serviceMux sync.RWMutex @@ -235,7 +234,7 @@ func NewServer(options ...Option) (*Server, error) { return nil, errors.Wrapf(err, "unable to create teams service") } - s.propertyService, err = properties.New(properties.ServiceConfig{ + propertyService, err := properties.New(properties.ServiceConfig{ PropertyGroupStore: s.Store().PropertyGroup(), PropertyFieldStore: s.Store().PropertyField(), PropertyValueStore: s.Store().PropertyValue(), @@ -245,7 +244,10 @@ func NewServer(options ...Option) (*Server, error) { } // Wrap PropertyService with access control layer to enforce caller-based permissions - s.propertyAccessService = NewPropertyAccessService(s.propertyService) + s.propertyAccessService = NewPropertyAccessService(propertyService, func(pluginID string) bool { + _, err := s.ch.GetPluginStatus(pluginID) + return err == nil + }) // It is important to initialize the hub only after the global logger is set // to avoid race conditions while logging from inside the hub. diff --git a/server/channels/testlib/store.go b/server/channels/testlib/store.go index b4064c298c5..845b13abca3 100644 --- a/server/channels/testlib/store.go +++ b/server/channels/testlib/store.go @@ -134,6 +134,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { propertyValueStore := mocks.PropertyValueStore{} propertyGroupStore.On("Register", model.ContentFlaggingGroupName).Return(&model.PropertyGroup{ID: model.NewId(), Name: model.ContentFlaggingGroupName}, nil) + propertyGroupStore.On("Register", model.CustomProfileAttributesPropertyGroupName).Return(&model.PropertyGroup{ID: model.NewId(), Name: model.CustomProfileAttributesPropertyGroupName}, nil) propertyFieldStore.On("SearchPropertyFields", mock.Anything).Return([]*model.PropertyField{}, nil) propertyFieldStore.On("CreatePropertyField", mock.Anything).Return(&model.PropertyField{}, nil) diff --git a/server/cmd/mmctl/commands/user_attributes_field_e2e_test.go b/server/cmd/mmctl/commands/user_attributes_field_e2e_test.go index 64b453b8150..ce93e69f40c 100644 --- a/server/cmd/mmctl/commands/user_attributes_field_e2e_test.go +++ b/server/cmd/mmctl/commands/user_attributes_field_e2e_test.go @@ -13,10 +13,10 @@ import ( // cleanCPAFields removes all existing CPA fields to ensure clean test state func (s *MmctlE2ETestSuite) cleanCPAFields() { - existingFields, appErr := s.th.App.ListCPAFields() + existingFields, appErr := s.th.App.ListCPAFields("") s.Require().Nil(appErr) for _, field := range existingFields { - appErr := s.th.App.DeleteCPAField(field.ID) + appErr := s.th.App.DeleteCPAField("", field.ID) s.Require().Nil(appErr) } } @@ -66,11 +66,11 @@ func (s *MmctlE2ETestSuite) TestCPAFieldListCmd() { }, } - createdTextField, appErr := s.th.App.CreateCPAField(textField) + createdTextField, appErr := s.th.App.CreateCPAField("", textField) s.Require().Nil(appErr) s.Require().NotNil(createdTextField) - createdSelectField, appErr := s.th.App.CreateCPAField(selectField) + createdSelectField, appErr := s.th.App.CreateCPAField("", selectField) s.Require().Nil(appErr) s.Require().NotNil(createdSelectField) @@ -114,7 +114,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldCreateCmd() { s.Require().Contains(output, "Field Department correctly created") // Verify field was actually created in the database - fields, appErr := s.th.App.ListCPAFields() + fields, appErr := s.th.App.ListCPAFields("") s.Require().Nil(appErr) s.Require().Len(fields, 1) s.Require().Equal("Department", fields[0].Name) @@ -150,7 +150,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldCreateCmd() { s.Require().Contains(output, "Field Skills correctly created") // Verify field was actually created in the database with correct options - fields, appErr := s.th.App.ListCPAFields() + fields, appErr := s.th.App.ListCPAFields("") s.Require().Nil(appErr) s.Require().Len(fields, 1) s.Require().Equal("Skills", fields[0].Name) @@ -210,7 +210,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { }, } - createdField, appErr := s.th.App.CreateCPAField(field) + createdField, appErr := s.th.App.CreateCPAField("", field) s.Require().Nil(appErr) // Now edit the field @@ -237,7 +237,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { s.Require().Contains(output, "Field Programming Languages successfully updated") // Verify field was actually updated - updatedField, appErr := s.th.App.GetCPAField(createdField.ID) + updatedField, appErr := s.th.App.GetCPAField("", createdField.ID) s.Require().Nil(appErr) s.Require().Equal("Programming Languages", updatedField.Name) @@ -268,7 +268,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { }, } - createdField, appErr := s.th.App.CreateCPAField(field) + createdField, appErr := s.th.App.CreateCPAField("", field) s.Require().Nil(appErr) // Now edit the field with --managed flag @@ -287,7 +287,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { s.Require().Len(printer.GetErrorLines(), 0) // Verify field was actually updated - updatedField, appErr := s.th.App.GetCPAField(createdField.ID) + updatedField, appErr := s.th.App.GetCPAField("", createdField.ID) s.Require().Nil(appErr) // Verify that managed flag was set correctly @@ -310,7 +310,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { }, } - createdField, appErr := s.th.App.CreateCPAField(field) + createdField, appErr := s.th.App.CreateCPAField("", field) s.Require().Nil(appErr) // Now edit the field using its name instead of ID @@ -336,7 +336,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { s.Require().Contains(output, "Field Team successfully updated") // Verify field was actually updated by retrieving it - updatedField, appErr := s.th.App.GetCPAField(createdField.ID) + updatedField, appErr := s.th.App.GetCPAField("", createdField.ID) s.Require().Nil(appErr) s.Require().Equal("Team", updatedField.Name) @@ -363,7 +363,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { }, } - createdField, appErr := s.th.App.CreateCPAField(field) + createdField, appErr := s.th.App.CreateCPAField("", field) s.Require().Nil(appErr) // Get the original option IDs to verify they are preserved @@ -406,7 +406,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { s.Require().Contains(output, "Field Programming Languages successfully updated") // Verify field was actually updated and options are preserved correctly - updatedField, appErr := s.th.App.GetCPAField(createdField.ID) + updatedField, appErr := s.th.App.GetCPAField("", createdField.ID) s.Require().Nil(appErr) // Check options @@ -456,7 +456,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { }, } - createdField, appErr := s.th.App.CreateCPAField(field) + createdField, appErr := s.th.App.CreateCPAField("", field) s.Require().Nil(appErr) cmd := &cobra.Command{} @@ -475,7 +475,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { s.Require().Contains(output, "Successfully deleted CPA field") // Verify field was actually deleted by checking if it exists in the list - fields, appErr := s.th.App.ListCPAFields() + fields, appErr := s.th.App.ListCPAFields("") s.Require().Nil(appErr) // Field should not be in the list anymore @@ -502,7 +502,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { }, } - createdField, appErr := s.th.App.CreateCPAField(field) + createdField, appErr := s.th.App.CreateCPAField("", field) s.Require().Nil(appErr) cmd := &cobra.Command{} @@ -522,7 +522,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { s.Require().Contains(output, "Successfully deleted CPA field: Department") // Verify field was actually deleted by checking if it exists in the list - fields, appErr := s.th.App.ListCPAFields() + fields, appErr := s.th.App.ListCPAFields("") s.Require().Nil(appErr) // Field should not be in the list anymore diff --git a/server/cmd/mmctl/commands/user_attributes_value_e2e_test.go b/server/cmd/mmctl/commands/user_attributes_value_e2e_test.go index 62284667ee4..64a47000128 100644 --- a/server/cmd/mmctl/commands/user_attributes_value_e2e_test.go +++ b/server/cmd/mmctl/commands/user_attributes_value_e2e_test.go @@ -14,7 +14,7 @@ import ( // cleanCPAValuesForUser removes all CPA values for a user func (s *MmctlE2ETestSuite) cleanCPAValuesForUser(userID string) { - existingValues, appErr := s.th.App.ListCPAValues(userID) + existingValues, appErr := s.th.App.ListCPAValues("", userID) s.Require().Nil(appErr) // Clear all existing values by setting them to null @@ -24,7 +24,7 @@ func (s *MmctlE2ETestSuite) cleanCPAValuesForUser(userID string) { } if len(updates) > 0 { - _, appErr = s.th.App.PatchCPAValues(userID, updates, false) + _, appErr = s.th.App.PatchCPAValues("", userID, updates, false) s.Require().Nil(appErr) } } @@ -64,14 +64,14 @@ func (s *MmctlE2ETestSuite) TestCPAValueList() { }, } - createdField, appErr := s.th.App.CreateCPAField(textField) + createdField, appErr := s.th.App.CreateCPAField("", textField) s.Require().Nil(appErr) // Set a text value using the app layer updates := map[string]json.RawMessage{ createdField.ID: json.RawMessage(`"Engineering"`), } - _, appErr = s.th.App.PatchCPAValues(s.th.BasicUser.Id, updates, false) + _, appErr = s.th.App.PatchCPAValues("", s.th.BasicUser.Id, updates, false) s.Require().Nil(appErr) // Test listing the values with plain format (human-readable) @@ -122,7 +122,7 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { }, } - createdField, appErr := s.th.App.CreateCPAField(textField) + createdField, appErr := s.th.App.CreateCPAField("", textField) s.Require().Nil(appErr) // Set a text value @@ -135,7 +135,8 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { s.Require().Nil(err) // Verify the value was set - values, appErr := s.th.App.ListCPAValues(s.th.BasicUser.Id) + + values, appErr := s.th.App.ListCPAValues("", s.th.BasicUser.Id) s.Require().Nil(appErr) s.Require().Len(values, 1) s.Require().Equal(createdField.ID, values[0].FieldID) @@ -165,7 +166,7 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { }, } - createdField, appErr := s.th.App.CreateCPAField(selectField) + createdField, appErr := s.th.App.CreateCPAField("", selectField) s.Require().Nil(appErr) // Set a select value using the option name @@ -178,7 +179,8 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { s.Require().Nil(err) // Verify the value was set (should be stored as option ID) - values, appErr := s.th.App.ListCPAValues(s.th.BasicUser.Id) + + values, appErr := s.th.App.ListCPAValues("", s.th.BasicUser.Id) s.Require().Nil(appErr) s.Require().Len(values, 1) s.Require().Equal(createdField.ID, values[0].FieldID) @@ -218,7 +220,7 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { }, } - createdField, appErr := s.th.App.CreateCPAField(multiselectField) + createdField, appErr := s.th.App.CreateCPAField("", multiselectField) s.Require().Nil(appErr) // Set multiple values using option names @@ -236,7 +238,8 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { s.Require().Nil(err) // Verify the values were set (should be stored as option IDs) - values, appErr := s.th.App.ListCPAValues(s.th.BasicUser.Id) + + values, appErr := s.th.App.ListCPAValues("", s.th.BasicUser.Id) s.Require().Nil(appErr) s.Require().Len(values, 1) s.Require().Equal(createdField.ID, values[0].FieldID) @@ -285,7 +288,7 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { }, } - createdField, appErr := s.th.App.CreateCPAField(multiselectField) + createdField, appErr := s.th.App.CreateCPAField("", multiselectField) s.Require().Nil(appErr) // Set a single value using option name @@ -299,7 +302,8 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { s.Require().Nil(err) // Verify the value was set (should be stored as an array with single option ID) - values, appErr := s.th.App.ListCPAValues(s.th.BasicUser.Id) + + values, appErr := s.th.App.ListCPAValues("", s.th.BasicUser.Id) s.Require().Nil(appErr) s.Require().Len(values, 1) s.Require().Equal(createdField.ID, values[0].FieldID) @@ -345,7 +349,7 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { }, } - createdField, appErr := s.th.App.CreateCPAField(userField) + createdField, appErr := s.th.App.CreateCPAField("", userField) s.Require().Nil(appErr) // Set a user value using the system admin user ID @@ -358,7 +362,8 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { s.Require().Nil(err) // Verify the value was set - values, appErr := s.th.App.ListCPAValues(s.th.BasicUser.Id) + + values, appErr := s.th.App.ListCPAValues("", s.th.BasicUser.Id) s.Require().Nil(appErr) s.Require().Len(values, 1) s.Require().Equal(createdField.ID, values[0].FieldID) diff --git a/server/public/model/custom_profile_attributes.go b/server/public/model/custom_profile_attributes.go index 1aba78bc50a..14feef692de 100644 --- a/server/public/model/custom_profile_attributes.go +++ b/server/public/model/custom_profile_attributes.go @@ -112,13 +112,16 @@ type CPAField struct { } type CPAAttrs struct { - Visibility string `json:"visibility"` - SortOrder float64 `json:"sort_order"` - Options PropertyOptions[*CustomProfileAttributesSelectOption] `json:"options"` - ValueType string `json:"value_type"` - LDAP string `json:"ldap"` - SAML string `json:"saml"` - Managed string `json:"managed"` + Visibility string `json:"visibility"` + SortOrder float64 `json:"sort_order"` + Options PropertyOptions[*CustomProfileAttributesSelectOption] `json:"options"` + ValueType string `json:"value_type"` + LDAP string `json:"ldap"` + SAML string `json:"saml"` + Managed string `json:"managed"` + Protected bool `json:"protected"` + SourcePluginID string `json:"source_plugin_id"` + AccessMode string `json:"access_mode"` } func (c *CPAField) IsSynced() bool { @@ -173,6 +176,9 @@ func (c *CPAField) ToPropertyField() *PropertyField { CustomProfileAttributesPropertyAttrsLDAP: c.Attrs.LDAP, CustomProfileAttributesPropertyAttrsSAML: c.Attrs.SAML, CustomProfileAttributesPropertyAttrsManaged: c.Attrs.Managed, + PropertyAttrsProtected: c.Attrs.Protected, + PropertyAttrsSourcePluginID: c.Attrs.SourcePluginID, + PropertyAttrsAccessMode: c.Attrs.AccessMode, } return &pf diff --git a/webapp/platform/types/src/properties.ts b/webapp/platform/types/src/properties.ts index 66af1dfba89..97f9b2712f0 100644 --- a/webapp/platform/types/src/properties.ts +++ b/webapp/platform/types/src/properties.ts @@ -67,6 +67,9 @@ export type UserPropertyField = PropertyField & { ldap?: string; saml?: string; managed?: string; + protected?: boolean; + source_plugin_id?: string; + access_mode?: '' | 'source_only' | 'shared_only'; }; };