-
Notifications
You must be signed in to change notification settings - Fork 95
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
ldapadmin - administrators can modify users' uid #1109
Conversation
Evolution funded by CIGALsace (https://github.com/camptocamp/georchestra-cigalsace-configuration/issues/531) Enables the ability for LDAP administrators (members of MOD_LDAPADMIN) to modify the user id (uid) of the users LDAP entries. The uid field is not like the others attributes and need extra work to implement the ability to modify it, since it is part of the DN qualification ; it requires to "move" the LDAP entry, which is not the same as updating a regular attribute. It also requires to get the users groups and update the member entry. Tests: Utests + tests against a real ldap adapted / OK Runtime tests OK
Please do not merge yet |
- Adding synchronized on CRUD operations from AccountDaoImpl to avoid race conditions (Note: might not the best way to do, but this is what was already done onto GroupDao). - Adding a test to cover modifyUser(Account, Account), tested against a real ldap.
Should be good to go now. |
@@ -75,5 +84,36 @@ public void testBlankFields_issues_1086_1096() throws Exception { | |||
assertTrue("Found a 'o' attribute, expeceted none", noOrgAnymore); | |||
|
|||
} | |||
|
|||
@Test | |||
public void testUpdateAcountAccount() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testUpdateAcountAccount ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the method signature is update(Account old, Account new)
, and I did not have any imagination for a test name :)
OK, LGTM, thanks Pierre, please merge. |
Missing the "send a mail feature", I will work on it before merging. |
Tests: - Mainly tested at runtime, expected mail provided to the mailserver - Testsuite OK
good to go. |
LGTM, please merge ! |
ldapadmin - administrators can modify users' uid
Looks like there's a small side-effect ...
|
@@ -61,6 +61,7 @@ shared.download_form.pdfurl= | |||
shared.ldapadmin.contextpath=/ldapadmin | |||
shared.ldapadmin.db=georchestra | |||
shared.ldapadmin.jdbcurl=jdbc:postgresql://@shared.psql.host@:@shared.psql.port@/@shared.ldapadmin.db@ | |||
shared.ldapadmin.warnifuidmodified=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake, this is not a real shared maven filter. It only belongs to ldapadmin.
=> should not be set in config/shared.maven.filters but in build_support/GenerateConfig.groovy
Evolution funded by CIGALsace (https://github.com/camptocamp/georchestra-cigalsace-configuration/issues/531).
Enables the ability for LDAP administrators (members of MOD_LDAPADMIN) to modify the user id (uid) of the users LDAP entries.
The uid field is not like the others attributes and need extra work to implement the ability to modify it, since it is part of the DN qualification ; it requires to "move" the LDAP entry, which is not the same as updating a regular attribute.
It also requires to get the users groups and update the member entry (server-side but also client-side in the js objects).
Tests: