diff --git a/backend/internal/service/ldap_service.go b/backend/internal/service/ldap_service.go index 05bd1e4c..3d59b3de 100644 --- a/backend/internal/service/ldap_service.go +++ b/backend/internal/service/ldap_service.go @@ -180,6 +180,11 @@ func (s *LdapService) fetchDesiredState(ctx context.Context, client ldapClient) return ldapDesiredState{}, err } + // Apply user admin flags from the desired group membership snapshot. + // This intentionally uses the configured group member attribute rather than + // relying on a user-side reverse-membership attribute such as memberOf. + s.applyAdminGroupMembership(users, groups) + return ldapDesiredState{ users: users, userIDs: userIDs, @@ -188,6 +193,29 @@ func (s *LdapService) fetchDesiredState(ctx context.Context, client ldapClient) }, nil } +func (s *LdapService) applyAdminGroupMembership(desiredUsers []ldapDesiredUser, desiredGroups []ldapDesiredGroup) { + dbConfig := s.appConfigService.GetDbConfig() + if dbConfig.LdapAdminGroupName.Value == "" { + return + } + + adminUsernames := make(map[string]struct{}) + for _, group := range desiredGroups { + if group.input.Name != dbConfig.LdapAdminGroupName.Value { + continue + } + + for _, username := range group.memberUsernames { + adminUsernames[username] = struct{}{} + } + } + + for i := range desiredUsers { + _, isAdmin := adminUsernames[desiredUsers[i].input.Username] + desiredUsers[i].input.IsAdmin = desiredUsers[i].input.IsAdmin || isAdmin + } +} + func (s *LdapService) fetchGroupsFromLDAP(ctx context.Context, client ldapClient, usernamesByDN map[string]string) (desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}, err error) { dbConfig := s.appConfigService.GetDbConfig() @@ -266,7 +294,6 @@ func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient) // Query LDAP for all users we want to manage searchAttrs := []string{ - "memberOf", "sn", "cn", dbConfig.LdapAttributeUserUniqueIdentifier.Value, @@ -314,15 +341,6 @@ func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient) ldapUserIDs[ldapID] = struct{}{} - // Check if user is admin by checking if they are in the admin group - isAdmin := false - for _, group := range value.GetAttributeValues("memberOf") { - if getDNProperty(dbConfig.LdapAttributeGroupName.Value, group) == dbConfig.LdapAdminGroupName.Value { - isAdmin = true - break - } - } - newUser := dto.UserCreateDto{ Username: value.GetAttributeValue(dbConfig.LdapAttributeUserUsername.Value), Email: utils.PtrOrNil(value.GetAttributeValue(dbConfig.LdapAttributeUserEmail.Value)), @@ -330,8 +348,10 @@ func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient) FirstName: value.GetAttributeValue(dbConfig.LdapAttributeUserFirstName.Value), LastName: value.GetAttributeValue(dbConfig.LdapAttributeUserLastName.Value), DisplayName: value.GetAttributeValue(dbConfig.LdapAttributeUserDisplayName.Value), - IsAdmin: isAdmin, - LdapID: ldapID, + // Admin status is computed after groups are loaded so it can use the + // configured group member attribute instead of a hard-coded memberOf. + IsAdmin: false, + LdapID: ldapID, } if newUser.DisplayName == "" { diff --git a/backend/internal/service/ldap_service_test.go b/backend/internal/service/ldap_service_test.go index 22553f8b..9722946a 100644 --- a/backend/internal/service/ldap_service_test.go +++ b/backend/internal/service/ldap_service_test.go @@ -44,7 +44,6 @@ func TestLdapServiceSyncAllReconcilesUsersAndGroups(t *testing.T) { "givenName": {"Alice"}, "sn": {"Jones"}, "displayName": {""}, - "memberOf": {"cn=admins,ou=groups,dc=example,dc=com"}, }), ldapEntry("uid=bob,ou=people,dc=example,dc=com", map[string][]string{ "entryUUID": {"u-bob"}, @@ -56,6 +55,11 @@ func TestLdapServiceSyncAllReconcilesUsersAndGroups(t *testing.T) { }), ), ldapSearchResult( + ldapEntry("cn=admins,ou=groups,dc=example,dc=com", map[string][]string{ + "entryUUID": {"g-admins"}, + "cn": {"admins"}, + "member": {"uid=alice,ou=people,dc=example,dc=com"}, + }), ldapEntry("cn=team,ou=groups,dc=example,dc=com", map[string][]string{ "entryUUID": {"g-team"}, "cn": {"team"}, @@ -188,33 +192,89 @@ func TestLdapServiceSyncAllHandlesDuplicateLDAPIDsInSingleRun(t *testing.T) { assert.ElementsMatch(t, []string{"alice"}, usernames(groups[0].Users)) } +func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) { + tests := []struct { + name string + appConfig *model.AppConfig + groupEntry *ldap.Entry + groupName string + groupLookup string + }{ + { + name: "memberOf missing on user", + appConfig: defaultTestLDAPAppConfig(), + groupEntry: ldapEntry("cn=admins,ou=groups,dc=example,dc=com", map[string][]string{ + "entryUUID": {"g-admins"}, + "cn": {"admins"}, + "member": {"uid=testadmin,ou=people,dc=example,dc=com"}, + }), + groupName: "admins", + groupLookup: "g-admins", + }, + { + name: "configured group name attribute differs from DN RDN", + appConfig: func() *model.AppConfig { + cfg := defaultTestLDAPAppConfig() + cfg.LdapAttributeGroupName = model.AppConfigVariable{Value: "displayName"} + cfg.LdapAdminGroupName = model.AppConfigVariable{Value: "pocketid.admin"} + return cfg + }(), + groupEntry: ldapEntry("cn=admins,ou=groups,dc=example,dc=com", map[string][]string{ + "entryUUID": {"g-display-admins"}, + "cn": {"admins"}, + "displayName": {"pocketid.admin"}, + "member": {"uid=testadmin,ou=people,dc=example,dc=com"}, + }), + groupName: "pocketid.admin", + groupLookup: "g-display-admins", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + service, db := newTestLdapServiceWithAppConfig(t, tt.appConfig, newFakeLDAPClient( + ldapSearchResult( + ldapEntry("uid=testadmin,ou=people,dc=example,dc=com", map[string][]string{ + "entryUUID": {"u-testadmin"}, + "uid": {"testadmin"}, + "mail": {"testadmin@example.com"}, + "givenName": {"Test"}, + "sn": {"Admin"}, + "displayName": {""}, + }), + ), + ldapSearchResult(tt.groupEntry), + )) + + require.NoError(t, service.SyncAll(t.Context())) + + var user model.User + require.NoError(t, db.First(&user, "ldap_id = ?", "u-testadmin").Error) + assert.True(t, user.IsAdmin) + + var group model.UserGroup + require.NoError(t, db.Preload("Users").First(&group, "ldap_id = ?", tt.groupLookup).Error) + assert.Equal(t, tt.groupName, group.Name) + assert.ElementsMatch(t, []string{"testadmin"}, usernames(group.Users)) + }) + } +} + func newTestLdapService(t *testing.T, client ldapClient) (*LdapService, *gorm.DB) { t.Helper() + return newTestLdapServiceWithAppConfig(t, defaultTestLDAPAppConfig(), client) +} + +func newTestLdapServiceWithAppConfig(t *testing.T, appConfigModel *model.AppConfig, client ldapClient) (*LdapService, *gorm.DB) { + t.Helper() + db := testutils.NewDatabaseForTest(t) fileStorage, err := storage.NewDatabaseStorage(db) require.NoError(t, err) - appConfig := NewTestAppConfigService(&model.AppConfig{ - RequireUserEmail: model.AppConfigVariable{Value: "false"}, - LdapEnabled: model.AppConfigVariable{Value: "true"}, - LdapBase: model.AppConfigVariable{Value: "dc=example,dc=com"}, - LdapUserSearchFilter: model.AppConfigVariable{Value: "(objectClass=person)"}, - LdapUserGroupSearchFilter: model.AppConfigVariable{Value: "(objectClass=groupOfNames)"}, - LdapAttributeUserUniqueIdentifier: model.AppConfigVariable{Value: "entryUUID"}, - LdapAttributeUserUsername: model.AppConfigVariable{Value: "uid"}, - LdapAttributeUserEmail: model.AppConfigVariable{Value: "mail"}, - LdapAttributeUserFirstName: model.AppConfigVariable{Value: "givenName"}, - LdapAttributeUserLastName: model.AppConfigVariable{Value: "sn"}, - LdapAttributeUserDisplayName: model.AppConfigVariable{Value: "displayName"}, - LdapAttributeUserProfilePicture: model.AppConfigVariable{Value: "jpegPhoto"}, - LdapAttributeGroupMember: model.AppConfigVariable{Value: "member"}, - LdapAttributeGroupUniqueIdentifier: model.AppConfigVariable{Value: "entryUUID"}, - LdapAttributeGroupName: model.AppConfigVariable{Value: "cn"}, - LdapAdminGroupName: model.AppConfigVariable{Value: "admins"}, - LdapSoftDeleteUsers: model.AppConfigVariable{Value: "true"}, - }) + appConfig := NewTestAppConfigService(appConfigModel) groupService := NewUserGroupService(db, appConfig, nil) userService := NewUserService( @@ -237,6 +297,28 @@ func newTestLdapService(t *testing.T, client ldapClient) (*LdapService, *gorm.DB return service, db } +func defaultTestLDAPAppConfig() *model.AppConfig { + return &model.AppConfig{ + RequireUserEmail: model.AppConfigVariable{Value: "false"}, + LdapEnabled: model.AppConfigVariable{Value: "true"}, + LdapBase: model.AppConfigVariable{Value: "dc=example,dc=com"}, + LdapUserSearchFilter: model.AppConfigVariable{Value: "(objectClass=person)"}, + LdapUserGroupSearchFilter: model.AppConfigVariable{Value: "(objectClass=groupOfNames)"}, + LdapAttributeUserUniqueIdentifier: model.AppConfigVariable{Value: "entryUUID"}, + LdapAttributeUserUsername: model.AppConfigVariable{Value: "uid"}, + LdapAttributeUserEmail: model.AppConfigVariable{Value: "mail"}, + LdapAttributeUserFirstName: model.AppConfigVariable{Value: "givenName"}, + LdapAttributeUserLastName: model.AppConfigVariable{Value: "sn"}, + LdapAttributeUserDisplayName: model.AppConfigVariable{Value: "displayName"}, + LdapAttributeUserProfilePicture: model.AppConfigVariable{Value: "jpegPhoto"}, + LdapAttributeGroupMember: model.AppConfigVariable{Value: "member"}, + LdapAttributeGroupUniqueIdentifier: model.AppConfigVariable{Value: "entryUUID"}, + LdapAttributeGroupName: model.AppConfigVariable{Value: "cn"}, + LdapAdminGroupName: model.AppConfigVariable{Value: "admins"}, + LdapSoftDeleteUsers: model.AppConfigVariable{Value: "true"}, + } +} + func newFakeLDAPClient(userResult, groupResult *ldap.SearchResult) ldapClient { return &fakeLDAPClient{ searchFn: func(searchRequest *ldap.SearchRequest) (*ldap.SearchResult, error) {