Custom profile attributes field endpoints (#29662)

* Adds the main Property System Architecture components

This change adds the necessary migrations for the Property Groups,
Fields and Values tables to be created, the store layer and a Property
Service that can be used from the app layer.

* Adds Custom Profile Attributes endpoints and app layer

* implement get and patch cpa values

* run i18n-extract

* Update property field type to use user instead of person

* Update PropertyFields to allow for unique nondeleted fields and remove redundant indexes

* Update PropertyValues to allow for unique nondeleted fields and remove redundant indexes

* Use StringMap instead of the map[string]any on property fields

* Add i18n strings

* Revert "Use StringMap instead of the map[string]any on property fields"

This reverts commit e2735ab0f8589d2524d636419ca0cb144575c4d6.

* Cast JSON binary data to string and add todo note for StringMap use

* Add mocks to the retrylayer tests

* Cast JSON binary data to string in property value store

* Check for binary parameter instead of casting to string for JSON data

* Fix bad merge

* Check property field type is one of the allowed ones

* Avoid reusing err variable to be explicit about the returned value

* Merge Property System Migrations into one file

* Adds NOT NULL to timestamps at the DB level

* Update stores to use tableSelectQuery instead of a slice var

* Update PropertyField model translations to be more explicit and avoid repetition

* Update PropertyValue model translations to be more explicit and avoid repetition

* Use ExecBuilder instead of ToSql&Exec

* Update property field errors to add context

* Ensure PerPage is greater than zero

* Update store errors to give more context

* Use ExecBuilder in the property stores where possible

* Add an on conflict suffix to the group register to avoid race conditions

* Remove user profile API documentation changes

* Update patchCPAValues endpoint and docs to return the updated information

* Merge two similar error conditions

* Use a route function for ListCPAValues

* Remove badly used translation string

* Remove unused get in register group method

* Adds input sanitization and validation to the CPA API endpoints

* Takes login outside of one test case to make it clear it affects multiple t.Runs

* Fix wrap error and return code when property field has been deleted

* Fix receiver name

* Adds comment to move the CPA group ID to the db cache

* Set the PerPage of CPA fields to the fields limit

* Update server/channels/app/custom_profile_attributes_test.go

Co-authored-by: Alejandro García Montoro <alejandro.garciamontoro@gmail.com>

* Standardize group ID access

* Avoid polluting the state between tests

* Use specific errors for the retrieval of CPA group

---------

Co-authored-by: Scott Bishel <scott.bishel@mattermost.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Alejandro García Montoro <alejandro.garciamontoro@gmail.com>
Этот коммит содержится в:
Miguel de la Cruz
2025-01-13 18:12:38 +01:00
коммит произвёл GitHub
родитель c6984941f1
Коммит ca34c6a03f
21 изменённых файлов: 1997 добавлений и 1 удалений

Просмотреть файл

@@ -151,6 +151,11 @@ type Routes struct {
OutgoingOAuthConnections *mux.Router // 'api/v4/oauth/outgoing_connections'
OutgoingOAuthConnection *mux.Router // 'api/v4/oauth/outgoing_connections/{outgoing_oauth_connection_id:[A-Za-z0-9]+}'
CustomProfileAttributes *mux.Router // 'api/v4/custom_profile_attributes'
CustomProfileAttributesFields *mux.Router // 'api/v4/custom_profile_attributes/fields'
CustomProfileAttributesField *mux.Router // 'api/v4/custom_profile_attributes/fields/{field_id:[A-Za-z0-9]+}'
CustomProfileAttributesValues *mux.Router // 'api/v4/custom_profile_attributes/values'
}
type API struct {
@@ -288,6 +293,11 @@ func Init(srv *app.Server) (*API, error) {
api.BaseRoutes.OutgoingOAuthConnections = api.BaseRoutes.APIRoot.PathPrefix("/oauth/outgoing_connections").Subrouter()
api.BaseRoutes.OutgoingOAuthConnection = api.BaseRoutes.OutgoingOAuthConnections.PathPrefix("/{outgoing_oauth_connection_id:[A-Za-z0-9]+}").Subrouter()
api.BaseRoutes.CustomProfileAttributes = api.BaseRoutes.APIRoot.PathPrefix("/custom_profile_attributes").Subrouter()
api.BaseRoutes.CustomProfileAttributesFields = api.BaseRoutes.CustomProfileAttributes.PathPrefix("/fields").Subrouter()
api.BaseRoutes.CustomProfileAttributesField = api.BaseRoutes.CustomProfileAttributesFields.PathPrefix("/{field_id:[A-Za-z0-9]+}").Subrouter()
api.BaseRoutes.CustomProfileAttributesValues = api.BaseRoutes.CustomProfileAttributes.PathPrefix("/values").Subrouter()
api.InitUser()
api.InitBot()
api.InitTeam()
@@ -338,6 +348,7 @@ func Init(srv *app.Server) (*API, error) {
api.InitOutgoingOAuthConnection()
api.InitClientPerformanceMetrics()
api.InitScheduledPost()
api.InitCustomProfileAttributes()
// If we allow testing then listen for manual testing URL hits
if *srv.Config().ServiceSettings.EnableTesting {
@@ -420,6 +431,11 @@ func InitLocal(srv *app.Server) *API {
api.BaseRoutes.SAML = api.BaseRoutes.APIRoot.PathPrefix("/saml").Subrouter()
api.BaseRoutes.CustomProfileAttributes = api.BaseRoutes.APIRoot.PathPrefix("/custom_profile_attributes").Subrouter()
api.BaseRoutes.CustomProfileAttributesFields = api.BaseRoutes.CustomProfileAttributes.PathPrefix("/fields").Subrouter()
api.BaseRoutes.CustomProfileAttributesField = api.BaseRoutes.CustomProfileAttributesFields.PathPrefix("/{field_id:[A-Za-z0-9]+}").Subrouter()
api.BaseRoutes.CustomProfileAttributesValues = api.BaseRoutes.CustomProfileAttributes.PathPrefix("/values").Subrouter()
api.InitUserLocal()
api.InitTeamLocal()
api.InitChannelLocal()
@@ -440,6 +456,7 @@ func InitLocal(srv *app.Server) *API {
api.InitExportLocal()
api.InitJobLocal()
api.InitSamlLocal()
api.InitCustomProfileAttributesLocal()
srv.LocalRouter.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404))

Просмотреть файл

@@ -0,0 +1,218 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package api4
import (
"encoding/json"
"net/http"
"strings"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/mlog"
"github.com/mattermost/mattermost/server/v8/channels/audit"
)
func (api *API) InitCustomProfileAttributes() {
if api.srv.Config().FeatureFlags.CustomProfileAttributes {
api.BaseRoutes.CustomProfileAttributesFields.Handle("", api.APISessionRequired(listCPAFields)).Methods(http.MethodGet)
api.BaseRoutes.CustomProfileAttributesFields.Handle("", api.APISessionRequired(createCPAField)).Methods(http.MethodPost)
api.BaseRoutes.CustomProfileAttributesField.Handle("", api.APISessionRequired(patchCPAField)).Methods(http.MethodPatch)
api.BaseRoutes.CustomProfileAttributesField.Handle("", api.APISessionRequired(deleteCPAField)).Methods(http.MethodDelete)
api.BaseRoutes.User.Handle("/custom_profile_attributes", api.APISessionRequired(listCPAValues)).Methods(http.MethodGet)
api.BaseRoutes.CustomProfileAttributesValues.Handle("", api.APISessionRequired(patchCPAValues)).Methods(http.MethodPatch)
}
}
func listCPAFields(c *Context, w http.ResponseWriter, r *http.Request) {
fields, appErr := c.App.ListCPAFields()
if appErr != nil {
c.Err = appErr
return
}
if err := json.NewEncoder(w).Encode(fields); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}
func createCPAField(c *Context, w http.ResponseWriter, r *http.Request) {
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
c.SetPermissionError(model.PermissionManageSystem)
return
}
var pf *model.PropertyField
err := json.NewDecoder(r.Body).Decode(&pf)
if err != nil || pf == nil {
c.SetInvalidParamWithErr("property_field", err)
return
}
pf.SanitizeInput()
auditRec := c.MakeAuditRecord("createCPAField", audit.Fail)
defer c.LogAuditRec(auditRec)
audit.AddEventParameterAuditable(auditRec, "property_field", pf)
createdField, appErr := c.App.CreateCPAField(pf)
if appErr != nil {
c.Err = appErr
return
}
auditRec.Success()
auditRec.AddEventResultState(createdField)
auditRec.AddEventObjectType("property_field")
w.WriteHeader(http.StatusCreated)
if err := json.NewEncoder(w).Encode(createdField); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}
func patchCPAField(c *Context, w http.ResponseWriter, r *http.Request) {
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
c.SetPermissionError(model.PermissionManageSystem)
return
}
c.RequireFieldId()
if c.Err != nil {
return
}
var patch *model.PropertyFieldPatch
err := json.NewDecoder(r.Body).Decode(&patch)
if err != nil || patch == nil {
c.SetInvalidParamWithErr("property_field_patch", err)
return
}
patch.SanitizeInput()
auditRec := c.MakeAuditRecord("patchCPAField", audit.Fail)
defer c.LogAuditRec(auditRec)
audit.AddEventParameterAuditable(auditRec, "property_field_patch", patch)
originalField, appErr := c.App.GetCPAField(c.Params.FieldId)
if appErr != nil {
c.Err = appErr
return
}
auditRec.AddEventPriorState(originalField)
patchedField, appErr := c.App.PatchCPAField(c.Params.FieldId, patch)
if appErr != nil {
c.Err = appErr
return
}
auditRec.Success()
auditRec.AddEventResultState(patchedField)
auditRec.AddEventObjectType("property_field")
if err := json.NewEncoder(w).Encode(patchedField); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}
func deleteCPAField(c *Context, w http.ResponseWriter, r *http.Request) {
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
c.SetPermissionError(model.PermissionManageSystem)
return
}
c.RequireFieldId()
if c.Err != nil {
return
}
auditRec := c.MakeAuditRecord("deleteCPAField", audit.Fail)
defer c.LogAuditRec(auditRec)
audit.AddEventParameter(auditRec, "field_id", c.Params.FieldId)
field, appErr := c.App.GetCPAField(c.Params.FieldId)
if appErr != nil {
c.Err = appErr
return
}
auditRec.AddEventPriorState(field)
if appErr := c.App.DeleteCPAField(c.Params.FieldId); appErr != nil {
c.Err = appErr
return
}
auditRec.Success()
auditRec.AddEventResultState(field)
auditRec.AddEventObjectType("property_field")
ReturnStatusOK(w)
}
func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) {
var attributeValues map[string]string
if jsonErr := json.NewDecoder(r.Body).Decode(&attributeValues); jsonErr != nil {
c.SetInvalidParamWithErr("attrs", jsonErr)
return
}
// This check is unnecessary for now
// Will be required when/if admins can patch other's values
userID := c.AppContext.Session().UserId
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), userID) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
auditRec := c.MakeAuditRecord("patchCPAValues", audit.Fail)
defer c.LogAuditRec(auditRec)
audit.AddEventParameter(auditRec, "user_id", userID)
results := make(map[string]string)
for fieldID, value := range attributeValues {
patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, strings.TrimSpace(value))
if appErr != nil {
c.Err = appErr
return
}
results[fieldID] = patchedValue.Value
}
auditRec.Success()
auditRec.AddEventObjectType("patchCPAValues")
if err := json.NewEncoder(w).Encode(results); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}
func listCPAValues(c *Context, w http.ResponseWriter, r *http.Request) {
c.RequireUserId()
if c.Err != nil {
return
}
userID := c.Params.UserId
canSee, err := c.App.UserCanSeeOtherUser(c.AppContext, c.AppContext.Session().UserId, userID)
if err != nil || !canSee {
c.SetPermissionError(model.PermissionViewMembers)
return
}
values, appErr := c.App.ListCPAValues(userID)
if appErr != nil {
c.Err = appErr
return
}
returnValue := make(map[string]string)
for _, value := range values {
returnValue[value.FieldID] = value.Value
}
if err := json.NewEncoder(w).Encode(returnValue); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}

Просмотреть файл

@@ -0,0 +1,17 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package api4
import "net/http"
func (api *API) InitCustomProfileAttributesLocal() {
if api.srv.Config().FeatureFlags.CustomProfileAttributes {
api.BaseRoutes.CustomProfileAttributesFields.Handle("", api.APILocal(listCPAFields)).Methods(http.MethodGet)
api.BaseRoutes.CustomProfileAttributesFields.Handle("", api.APILocal(createCPAField)).Methods(http.MethodPost)
api.BaseRoutes.CustomProfileAttributesField.Handle("", api.APILocal(patchCPAField)).Methods(http.MethodPatch)
api.BaseRoutes.CustomProfileAttributesField.Handle("", api.APILocal(deleteCPAField)).Methods(http.MethodDelete)
api.BaseRoutes.User.Handle("/custom_profile_attributes", api.APISessionRequired(listCPAValues)).Methods(http.MethodGet)
api.BaseRoutes.CustomProfileAttributesValues.Handle("", api.APISessionRequired(patchCPAValues)).Methods(http.MethodPatch)
}
}

Просмотреть файл

@@ -0,0 +1,269 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package api4
import (
"context"
"fmt"
"os"
"testing"
"github.com/mattermost/mattermost/server/public/model"
"github.com/stretchr/testify/require"
)
func TestCreateCPAField(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t)
defer th.TearDown()
t.Run("a user without admin permissions should not be able to create a field", func(t *testing.T) {
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
_, resp, err := th.Client.CreateCPAField(context.Background(), field)
CheckForbiddenStatus(t, resp)
require.Error(t, err)
})
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
field := &model.PropertyField{Name: model.NewId()}
createdField, resp, err := client.CreateCPAField(context.Background(), field)
CheckBadRequestStatus(t, resp)
require.Error(t, err)
require.Empty(t, createdField)
}, "an invalid field should be rejected")
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
name := model.NewId()
field := &model.PropertyField{
Name: fmt.Sprintf(" %s\t", name), // name should be sanitized
Type: model.PropertyFieldTypeText,
Attrs: map[string]any{"visibility": "default"},
}
createdField, resp, err := client.CreateCPAField(context.Background(), field)
CheckCreatedStatus(t, resp)
require.NoError(t, err)
require.NotZero(t, createdField.ID)
require.Equal(t, name, createdField.Name)
require.Equal(t, "default", createdField.Attrs["visibility"])
}, "a user with admin permissions should be able to create the field")
}
func TestListCPAFields(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t)
defer th.TearDown()
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: map[string]any{"visibility": "default"},
}
createdField, _, err := th.SystemAdminClient.CreateCPAField(context.Background(), field)
require.NoError(t, err)
require.NotNil(t, createdField)
t.Run("any user should be able to list fields", func(t *testing.T) {
fields, resp, err := th.Client.ListCPAFields(context.Background())
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, fields)
require.Len(t, fields, 1)
require.Equal(t, createdField.ID, fields[0].ID)
})
t.Run("the endpoint should only list non deleted fields", func(t *testing.T) {
require.Nil(t, th.App.DeleteCPAField(createdField.ID))
fields, resp, err := th.Client.ListCPAFields(context.Background())
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Empty(t, fields)
})
}
func TestPatchCPAField(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t)
defer th.TearDown()
t.Run("a user without admin permissions should not be able to patch a field", func(t *testing.T) {
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
patch := &model.PropertyFieldPatch{Name: model.NewPointer(model.NewId())}
_, resp, err := th.Client.PatchCPAField(context.Background(), createdField.ID, patch)
CheckForbiddenStatus(t, resp)
require.Error(t, err)
})
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
newName := model.NewId()
patch := &model.PropertyFieldPatch{Name: model.NewPointer(fmt.Sprintf(" %s \t ", newName))} // name should be sanitized
patchedField, resp, err := client.PatchCPAField(context.Background(), createdField.ID, patch)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Equal(t, newName, patchedField.Name)
}, "a user with admin permissions should be able to patch the field")
}
func TestDeleteCPAField(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t)
defer th.TearDown()
t.Run("a user without admin permissions should not be able to delete a field", func(t *testing.T) {
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
createdField, _, err := th.SystemAdminClient.CreateCPAField(context.Background(), field)
require.NoError(t, err)
require.NotNil(t, createdField)
resp, err := th.Client.DeleteCPAField(context.Background(), createdField.ID)
CheckForbiddenStatus(t, resp)
require.Error(t, err)
})
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
createdField, _, err := th.SystemAdminClient.CreateCPAField(context.Background(), field)
require.NoError(t, err)
require.NotNil(t, createdField)
require.Zero(t, createdField.DeleteAt)
resp, err := client.DeleteCPAField(context.Background(), createdField.ID)
CheckOKStatus(t, resp)
require.NoError(t, err)
deletedField, appErr := th.App.GetCPAField(createdField.ID)
require.Nil(t, appErr)
require.NotZero(t, deletedField.DeleteAt)
}, "a user with admin permissions should be able to delete the field")
}
func TestListCPAValues(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t).InitBasic()
defer th.TearDown()
th.RemovePermissionFromRole(model.PermissionViewMembers.Id, model.SystemUserRoleId)
defer func() {
th.AddPermissionToRole(model.PermissionViewMembers.Id, model.SystemUserRoleId)
}()
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
values := map[string]string{}
values[createdField.ID] = "Field Value"
_, _, err := th.Client.PatchCPAValues(context.Background(), values)
require.NoError(t, err)
// login with Client2 from this point on
th.LoginBasic2()
t.Run("any team member should be able to list values", func(t *testing.T) {
values, resp, err := th.Client.ListCPAValues(context.Background(), th.BasicUser.Id)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, values)
require.Len(t, values, 1)
})
t.Run("non team member should NOT be able to list values", func(t *testing.T) {
resp, err := th.SystemAdminClient.RemoveTeamMember(context.Background(), th.BasicTeam.Id, th.BasicUser2.Id)
CheckOKStatus(t, resp)
require.NoError(t, err)
_, resp, err = th.Client.ListCPAValues(context.Background(), th.BasicUser.Id)
CheckForbiddenStatus(t, resp)
require.Error(t, err)
})
}
func TestPatchCPAValues(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t).InitBasic()
defer th.TearDown()
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
t.Run("any team member should be able to create their own values", func(t *testing.T) {
values := map[string]string{}
value := "Field Value"
values[createdField.ID] = fmt.Sprintf(" %s ", value) // value should be sanitized
patchedValues, resp, err := th.Client.PatchCPAValues(context.Background(), values)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, patchedValues)
require.Len(t, patchedValues, 1)
require.Equal(t, value, patchedValues[createdField.ID])
values, resp, err = th.Client.ListCPAValues(context.Background(), th.BasicUser.Id)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, values)
require.Len(t, values, 1)
require.Equal(t, "Field Value", values[createdField.ID])
})
t.Run("any team member should be able to patch their own values", func(t *testing.T) {
values, resp, err := th.Client.ListCPAValues(context.Background(), th.BasicUser.Id)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, values)
require.Len(t, values, 1)
value := "Updated Field Value"
values[createdField.ID] = fmt.Sprintf(" %s \t", value) // value should be sanitized
patchedValues, resp, err := th.Client.PatchCPAValues(context.Background(), values)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Equal(t, value, patchedValues[createdField.ID])
values, resp, err = th.Client.ListCPAValues(context.Background(), th.BasicUser.Id)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Equal(t, value, values[createdField.ID])
})
}