[MGNLLDAP-60] Create ADUserManager which implements method getAllUsers Created: 30/Jan/12  Updated: 02/May/12  Resolved: 30/Apr/12

Status: Closed
Project: LDAP Connector
Component/s: None
Affects Version/s: None
Fix Version/s: 1.4.4

Type: New Feature Priority: Neutral
Reporter: Milan Divilek Assignee: Milan Divilek
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MGNLLDAP-67 Implement getAllUsers method for LDAP Closed
Template:
Acceptance criteria:
Empty
Date of First Response:

 Description   

ExternalUserManager hasn't implemented method getAllUser yet. LDAP module should extend this ExternalUserManager and implement this method.



 Comments   
Comment by Magnolia International [ 02/Feb/12 ]
  • Why AD-specific ?
  • Why does it look like there's so much code that's been copied from the existing AuthenticationModule classes ?
  • Why does the copyright say Copyright 2011 for a file that's been created in 2012?
  • Why is this planned for an upcoming minor release, and lastly, why is this useful (what are the use-cases), and does it relate (or not) to existing issues like MGNLLDAP-18, MGNLLDAP-20, MGNLLDAP-21.

Also note that we have (somewhere… in a sandbox, or perhaps the openid module or .. perhaps it's in core now?) an AuthModule class that relies solely on the UserManager and associated classes. Such that the current AuthModule classes would be obsoleted if we had UM impls for everything.

Given all the above, I would suggest reimplementing this using the 4.5 security API and a cleaned up AuthModule class, rather than monkey patching the existing ones, unless there is a compelling reason why this should be done before ?

Comment by Milan Divilek [ 06/Feb/12 ]

Why AD-specific ?

It's true. Class doesn't need to be AD specific. I'll make base impl for LDAP and subclass for AD.

Why does it look like there's so much code that's been copied from the existing AuthenticationModule classes ?

Because it needs same code like AuthenticationModule and the code from AuthenticationModule isn't exposed to other classes. I will make a util class for this code and rewrite both to use the util.

Why does the copyright say Copyright 2011 for a file that's been created in 2012?

I still live in year 2011 i will fix it

Why is this planned for an upcoming minor release, and lastly, why is this useful (what are the use-cases), and does it relate (or not) to existing issues like MGNLLDAP-18, MGNLLDAP-20, MGNLLDAP-21.

Please see the linked support issue. It's useful for getting all user properties (incl. groups and roles) from LDAP/AD same way like getting those for plain Magnolia users. It's use for example when you send mail to some group or in similar functionality.

Migration of the LDAP for new security is already scheduled in different ticket (MGNLLDAP-61).
Rewriting AuthenticationModule would be a great idea, but that is not focus of this ticket.

Comment by Jan Haderka [ 27/Apr/12 ]

Test for new class?
If there's no install task, there needs to be documentation describing when and how to use such custom user manager.

Comment by Milan Divilek [ 30/Apr/12 ]

Test for new class?

Test should be part of MGNLLDAP-67 and since it will be written in Mockito it won't be backported.

Generated at Mon Feb 12 02:21:11 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.