Merge branch 'main' into chore/depot

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,
+			// Admin status is computed after groups are loaded so it can use the
-			LdapID:        ldapID,
+			// configured group member attribute instead of a hard-coded memberOf.
+			IsAdmin: false,
+			LdapID:  ldapID,
 		}
 
 		if newUser.DisplayName == "" {

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{
+	appConfig := NewTestAppConfigService(appConfigModel)
-		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"},
-	})
 
 	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) {