upstream/mattermost
1
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-05-28 04:35:04 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[MM-64320] Remove deprecated include_removed_members option in api/v4/ldap/sync (#31121)

Browse source
This commit is contained in:
Ben Schumacher 2025-07-17 12:35:08 +02:00 committed by GitHub
parent b7c2287ada
commit be0d4777ef
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 28 additions and 99 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
api/v4/source/definitions.yaml
View file

@ -1335,8 +1335,6 @@ components:
type: string
SyncIntervalMinutes:
type: integer
ReAddRemovedMembers:
type: boolean
SkipCertificateVerification:
type: boolean
PublicCertificateFile:

10
server/channels/api4/ldap.go
View file

@ -56,18 +56,10 @@ func syncLdap(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
var opts struct {
IncludeRemovedMembers *bool `json:"include_removed_members"`
}
err := json.NewDecoder(r.Body).Decode(&opts)
if err != nil {
c.Logger.LogM(mlog.MlvlLDAPInfo, "Error decoding LDAP sync options", mlog.Err(err))
}
auditRec := c.MakeAuditRecord(model.AuditEventSyncLdap, model.AuditStatusFail)
defer c.LogAuditRec(auditRec)
c.App.SyncLdap(c.AppContext, opts.IncludeRemovedMembers)
c.App.SyncLdap(c.AppContext)
auditRec.Success()
ReturnStatusOK(w)

22
server/channels/api4/ldap_test.go
View file

@ -145,33 +145,19 @@ func TestSyncLdap(t *testing.T) {
})
ldapMock := &mocks.LdapInterface{}
mockCall := ldapMock.On(
ldapMock.On(
"StartSynchronizeJob",
mock.AnythingOfType("*request.Context"),
mock.AnythingOfType("bool"),
mock.AnythingOfType("*bool"),
false,
).Return(nil, nil)
ready := make(chan bool)
reAddRemovedMembers := false
mockCall.RunFn = func(args mock.Arguments) {
reAddRemovedMembers = *args[2].(*bool)
ready <- true
}
th.App.Channels().Ldap = ldapMock
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
_, err := client.SyncLdap(context.Background(), model.NewPointer(false))
<-ready
_, err := client.SyncLdap(context.Background())
require.NoError(t, err)
require.False(t, reAddRemovedMembers)
_, err = client.SyncLdap(context.Background(), model.NewPointer(true))
<-ready
require.NoError(t, err)
require.True(t, reAddRemovedMembers)
})
resp, err := th.Client.SyncLdap(context.Background(), model.NewPointer(false))
resp, err := th.Client.SyncLdap(context.Background())
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}

6
server/channels/app/ldap.go
View file

@ -15,9 +15,7 @@ import (
)
// SyncLdap starts an LDAP sync job.
// If reAddRemovedMembers is true, then members who left or were removed from a team/channel will
// be re-added; otherwise, they will not be re-added.
func (a *App) SyncLdap(c request.CTX, reAddRemovedMembers *bool) {
func (a *App) SyncLdap(c request.CTX) {
a.Srv().Go(func() {
if license := a.Srv().License(); license != nil && *license.Features.LDAP {
if !*a.Config().LdapSettings.EnableSync {
@ -30,7 +28,7 @@ func (a *App) SyncLdap(c request.CTX, reAddRemovedMembers *bool) {
c.Logger().Error("Not executing ldap sync because ldap is not available")
return
}
if _, appErr := ldapI.StartSynchronizeJob(c, false, reAddRemovedMembers); appErr != nil {
if _, appErr := ldapI.StartSynchronizeJob(c, false); appErr != nil {
c.Logger().Error("Failed to start LDAP sync job")
}
}

2
server/cmd/mmctl/client/client.go
View file

@ -97,7 +97,7 @@ type Client interface {
PatchConfig(context.Context, *model.Config) (*model.Config, *model.Response, error)
ReloadConfig(ctx context.Context) (*model.Response, error)
MigrateConfig(ctx context.Context, from, to string) (*model.Response, error)
SyncLdap(ctx context.Context, reAddRemovedMembers *bool) (*model.Response, error)
SyncLdap(ctx context.Context) (*model.Response, error)
MigrateIdLdap(ctx context.Context, toAttribute string) (*model.Response, error)
GetUsers(ctx context.Context, page, perPage int, etag string) ([]*model.User, *model.Response, error)
UpdateUserActive(ctx context.Context, userID string, activate bool) (*model.Response, error)

15
server/cmd/mmctl/commands/ldap.go
View file

@ -30,12 +30,6 @@ func newLDAPSyncCmd() *cobra.Command {
RunE: withClient(ldapSyncCmdF),
}
cmd.Flags().Bool("include-removed-members", false, "Include members who left or were removed from a group-synced team/channel")
err := cmd.Flags().MarkDeprecated("include-removed-members", "This flag is deprecated and will be removed in a future version. Use LdapSettings.ReAddRemovedMembers instead.")
if err != nil {
panic(err)
}
return cmd
}
@ -102,14 +96,7 @@ func init() {
func ldapSyncCmdF(c client.Client, cmd *cobra.Command, args []string) error {
printer.SetSingle(true)
var resp *model.Response
var err error
if cmd.Flags().Changed("include-removed-members") {
reAddRemovedMembers, _ := cmd.Flags().GetBool("include-removed-members")
resp, err = c.SyncLdap(context.TODO(), &reAddRemovedMembers)
} else {
resp, err = c.SyncLdap(context.TODO(), nil)
}
resp, err := c.SyncLdap(context.TODO())
if err != nil {
return err
}

23
server/cmd/mmctl/commands/ldap_test.go
View file

@ -22,7 +22,7 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() {
s.client.
EXPECT().
SyncLdap(context.TODO(), nil).
SyncLdap(context.TODO()).
Return(&model.Response{StatusCode: http.StatusOK}, nil).
Times(1)
@ -39,7 +39,7 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() {
s.client.
EXPECT().
SyncLdap(context.TODO(), nil).
SyncLdap(context.TODO()).
Return(&model.Response{StatusCode: http.StatusBadRequest}, nil).
Times(1)
@ -56,7 +56,7 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() {
s.client.
EXPECT().
SyncLdap(context.TODO(), nil).
SyncLdap(context.TODO()).
Return(&model.Response{StatusCode: http.StatusBadRequest}, mockError).
Times(1)
@ -66,23 +66,6 @@ func (s *MmctlUnitTestSuite) TestLdapSyncCmd() {
s.Require().Len(printer.GetLines(), 0)
s.Require().Len(printer.GetErrorLines(), 0)
})
s.Run("Sync with deprecated includeRemoveMembers", func() {
printer.Clean()
cmd := newLDAPSyncCmd()
err := cmd.ParseFlags([]string{"--include-removed-members"})
s.Require().Nil(err)
s.client.
EXPECT().
SyncLdap(context.TODO(), model.NewPointer(true)).
Return(&model.Response{StatusCode: http.StatusOK}, nil).
Times(1)
err = ldapSyncCmdF(s.client, cmd, []string{})
s.Require().Nil(err)
})
}
func (s *MmctlUnitTestSuite) TestLdapMigrateID() {

8
server/cmd/mmctl/mocks/client_mock.go
View file

@ -2104,18 +2104,18 @@ func (mr *MockClientMockRecorder) SoftDeleteTeam(arg0, arg1 interface{}) *gomock
}
// SyncLdap mocks base method.
func (m *MockClient) SyncLdap(arg0 context.Context, arg1 *bool) (*model.Response, error) {
func (m *MockClient) SyncLdap(arg0 context.Context) (*model.Response, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "SyncLdap", arg0, arg1)
ret := m.ctrl.Call(m, "SyncLdap", arg0)
ret0, _ := ret[0].(*model.Response)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// SyncLdap indicates an expected call of SyncLdap.
func (mr *MockClientMockRecorder) SyncLdap(arg0, arg1 interface{}) *gomock.Call {
func (mr *MockClientMockRecorder) SyncLdap(arg0 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncLdap", reflect.TypeOf((*MockClient)(nil).SyncLdap), arg0, arg1)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncLdap", reflect.TypeOf((*MockClient)(nil).SyncLdap), arg0)
}
// UpdateChannelPrivacy mocks base method.

2
server/einterfaces/ldap.go
View file

@ -15,7 +15,7 @@ type LdapInterface interface {
GetUserAttributes(rctx request.CTX, id string, attributes []string) (map[string]string, *model.AppError)
CheckProviderAttributes(c request.CTX, LS *model.LdapSettings, ouser *model.User, patch *model.UserPatch) string
SwitchToLdap(c request.CTX, userID, ldapID, ldapPassword string) *model.AppError
StartSynchronizeJob(c request.CTX, waitForJobToFinish bool, reAddRemovedMembers *bool) (*model.Job, *model.AppError)
StartSynchronizeJob(c request.CTX, waitForJobToFinish bool) (*model.Job, *model.AppError)
GetAllLdapUsers(c request.CTX) ([]*model.User, *model.AppError)
MigrateIDAttribute(c request.CTX, toAttribute string) error
GetGroup(rctx request.CTX, groupUID string) (*model.Group, *model.AppError)

18
server/einterfaces/mocks/LdapInterface.go
View file

@ -309,9 +309,9 @@ func (_m *LdapInterface) MigrateIDAttribute(c request.CTX, toAttribute string) e
return r0
}
// StartSynchronizeJob provides a mock function with given fields: c, waitForJobToFinish, reAddRemovedMembers
func (_m *LdapInterface) StartSynchronizeJob(c request.CTX, waitForJobToFinish bool, reAddRemovedMembers *bool) (*model.Job, *model.AppError) {
ret := _m.Called(c, waitForJobToFinish, reAddRemovedMembers)
// StartSynchronizeJob provides a mock function with given fields: c, waitForJobToFinish
func (_m *LdapInterface) StartSynchronizeJob(c request.CTX, waitForJobToFinish bool) (*model.Job, *model.AppError) {
ret := _m.Called(c, waitForJobToFinish)
if len(ret) == 0 {
panic("no return value specified for StartSynchronizeJob")
@ -319,19 +319,19 @@ func (_m *LdapInterface) StartSynchronizeJob(c request.CTX, waitForJobToFinish b
var r0 *model.Job
var r1 *model.AppError
if rf, ok := ret.Get(0).(func(request.CTX, bool, *bool) (*model.Job, *model.AppError)); ok {
return rf(c, waitForJobToFinish, reAddRemovedMembers)
if rf, ok := ret.Get(0).(func(request.CTX, bool) (*model.Job, *model.AppError)); ok {
return rf(c, waitForJobToFinish)
}
if rf, ok := ret.Get(0).(func(request.CTX, bool, *bool) *model.Job); ok {
r0 = rf(c, waitForJobToFinish, reAddRemovedMembers)
if rf, ok := ret.Get(0).(func(request.CTX, bool) *model.Job); ok {
r0 = rf(c, waitForJobToFinish)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.Job)
}
}
if rf, ok := ret.Get(1).(func(request.CTX, bool, *bool) *model.AppError); ok {
r1 = rf(c, waitForJobToFinish, reAddRemovedMembers)
if rf, ok := ret.Get(1).(func(request.CTX, bool) *model.AppError); ok {
r1 = rf(c, waitForJobToFinish)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)

19
server/public/model/client4.go
View file

@ -5779,23 +5779,8 @@ func (c *Client4) GetClusterStatus(ctx context.Context) ([]*ClusterInfo, *Respon
// LDAP Section
// SyncLdap starts a run of the LDAP sync job.
//
// If reAddRemovedMembers is true, then group members who left or were removed from a
// synced team/channel will be re-joined; otherwise, they will be excluded.
//
// The ReAddRemovedMembers option is deprecated. Use LdapSettings.ReAddRemovedMembers instead.
func (c *Client4) SyncLdap(ctx context.Context, reAddRemovedMembers *bool) (*Response, error) {
data := map[string]any{}
if reAddRemovedMembers != nil {
data["include_removed_members"] = *reAddRemovedMembers
}
reqBody, err := json.Marshal(data)
if err != nil {
return nil, NewAppError("SyncLdap", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
r, err := c.DoAPIPostBytes(ctx, c.ldapRoute()+"/sync", reqBody)
func (c *Client4) SyncLdap(ctx context.Context) (*Response, error) {
r, err := c.DoAPIPostBytes(ctx, c.ldapRoute()+"/sync", nil)
if err != nil {
return BuildResponse(r), err
}