Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider implementation of Typo3UserRepository::setUserGroups to retain be_users.usergroup ordering #184

Open
jpmschuler opened this issue Oct 5, 2023 · 2 comments

Comments

@jpmschuler
Copy link

While for fe_users the group orders make no difference, this is not the case for be_users, as groups can not only contain permissions, but also tree mounts. We e.g. got a lot of editors who have access to multiple site and these sites have a priority, e.g.:

  • a big major site and smaller sites
  • a site with many daily changes vs. some basically read-only sites

In that cases we tend to "sort" the groups in the field be_users.usergroup accordingly, so that the first tree mount is the most relevant one and up on top.

However the implementation for the usergroup check and override at

$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
->getQueryBuilderForTable($table);
$rows = $queryBuilder
->select('uid')
->from($table)
->where(
$queryBuilder->expr()->in('uid', $usergroup),
$queryBuilder->expr()->eq('tx_igldapssoauth_dn', $queryBuilder->createNamedParameter('', \PDO::PARAM_STR))
)
->execute()
->fetchAllAssociative();
foreach ($rows as $row) {
$localUserGroups[] = $row['uid'];
}
doesn't filter the current groups by valid ones, but rather replaces them with the SQL result used to find valid ones. While using SQL here is quite elegant in that regard, it doesn't offer the option to retain the order, so the groups are re-ordered on every login (even if group sync is deactivated and keep groups is on).

So I propose to either do that filtering in PHP instead (foreach through current groups and compare with SQL result) or add an static orderBy statement (in many SQL dialects orderBy can not only be asc/desc, but also accept a CSV, however I don't know if doctrine supports that).

@xperseguers
Copy link
Owner

Hello. I see. Would you mind create such a PR (based on PHP filtering, I find it more robust than trying to do that with Doctrine but without being able to properly test on all DBMS).

@jpmschuler
Copy link
Author

I agree completely with that approach.
Will give the PR a go as soon as time comes up, put a blocker for 2nd Nov for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants