? Pending

User tests: Successful: Unsuccessful:

avatar travisrisner
travisrisner
13 Jun 2019

Pull Request for Issue #25180 .

Summary of Changes

Ensure the username and email fields are using the 0 array key returned from the LDAP Symfony Package, otherwise. I tested this against an LDAP server I had setup.

I am unsure at this time how to write a test for this, but am reading up on it. If there's a specific team that needs to handle that, just let me know.

Testing Instructions

See Issue #25180 .

About how to set up the plugin to use the public ldap server ldap.forumsys.com for a test see this comment.

avatar travisrisner travisrisner - open - 13 Jun 2019
avatar travisrisner travisrisner - change - 13 Jun 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jun 2019
Category Front End Plugins
avatar travisrisner travisrisner - change - 13 Jun 2019
Labels Added: ?
avatar travisrisner
travisrisner - comment - 13 Jun 2019

@richard67 what about something like that?

avatar richard67
richard67 - comment - 13 Jun 2019

@travisrisner Looks good to me, elegant solution with the "&&" operator. Works for J4 because minimum PHP version for J4 is 7.something.

Unfortunately I need to set up an Ldap server before I can test, so it might be weekend then. Do you know if there is some kind of public Ldap test server web site available for testing?

avatar richard67
richard67 - comment - 13 Jun 2019

@HLeithner Is that PR sufficient, or does it also need to implement some unit test?

As far as I could see (am not 100% sure if I found all), there is no unit test yet for the ldap plugin, so it would need to write some kind of a stub for the ldap server, as long as the automated testing team has not developed a solution for testing against a real Ldap server.

As this PR really fixes a bug, we should not hold it back just because it does not implement a test which was not implemented before.

@mbabker What do you think about that?

avatar richard67
richard67 - comment - 13 Jun 2019
avatar mbabker
mbabker - comment - 13 Jun 2019

There's a reason I said the test team needs to work on that and not "this issue needs an accompanying test case"...

avatar richard67
richard67 - comment - 13 Jun 2019

@mbabker I know, and I understood. I asked in order to be sure that it is ok because @HLeithner wrote about a test in the original issue #25180 . So this PR is ok?

avatar richard67
richard67 - comment - 13 Jun 2019

Hmm, it seems the ldap test server I found still uses protocol version1, so it can't be used.

Edit: Or I am making some mistake in ldap plugin settings.

avatar richard67
richard67 - comment - 15 Jun 2019

I have tested this item successfully on 33c619c

Finally I've managed to configure the ldap plugin to use the public ldap test server ldap.forumsys.com.

Was a bit tricky, description for other testers will follow in a comment below.

Then I was able to reproduce the error from issue #25180 and verify that this PR here solves it.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25191.

avatar richard67
richard67 - comment - 15 Jun 2019

I have tested this item successfully on 33c619c

Finally I've managed to configure the ldap plugin to use the public ldap test server ldap.forumsys.com.

Was a bit tricky, description for other testers will follow in a comment below.

Then I was able to reproduce the error from issue #25180 and verify that this PR here solves it.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25191.

avatar richard67 richard67 - test_item - 15 Jun 2019 - Tested successfully
avatar richard67
richard67 - comment - 15 Jun 2019

I've used following settings in the plugin for the test:

Host: ldap.forumsys.com
Port: 389 (= default)
LDAP V3: Yes
Negotiate TLS: No
Follow Referrals: No
Authorisation Method: Bind and Search
Base DN: dc=example,dc=com
Search String: uid=[search]
User's DN: uid=[username],dc=example,dc=com
Connect Username: rieman
Connect Password: see https://www.forumsys.com/tutorials/integration-how-to/ldap/online-ldap-test-server/
Map: Full Name: cn
Map: Email: mail
Map: User ID: uid

The User's DN parameter I had to fill in even if the help text says it is only used for direct bind. Same problem I had with J3 (staging). Maybe a bug, I will investigate later.

The direct bind to user method I did not get working, so maybe the J4 plugin has other errors beside the one handled with this PR. With J3 (staging) I got both methods (direct bind to user and search) working.

User riemann does not exist in Joomla and is used only for ldap bind and lookup.

User gauss or any other user mentioned on https://www.forumsys.com/tutorials/integration-how-to/ldap/online-ldap-test-server/ can be used to log in. Passwords see on that web site. For this user, an exquivalent Joomla user exists already, or it will be autmatically created with group=Registered when login in at frontend for the first time.

avatar richard67 richard67 - change - 15 Jun 2019
The description was changed
avatar joomla-cms-bot joomla-cms-bot - edited - 15 Jun 2019
avatar joomla-cms-bot joomla-cms-bot - edited - 15 Jun 2019
avatar richard67
richard67 - comment - 15 Jun 2019

@travisrisner I've updated the description by some testing instructions in order to find some 2nd tester more easily.

avatar richard67 richard67 - change - 15 Jun 2019
The description was changed
avatar joomla-cms-bot joomla-cms-bot - edited - 15 Jun 2019
avatar joomla-cms-bot joomla-cms-bot - edited - 15 Jun 2019
avatar wilsonge wilsonge - close - 15 Jun 2019
avatar wilsonge wilsonge - merge - 15 Jun 2019
avatar wilsonge wilsonge - change - 15 Jun 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-06-15 22:48:55
Closed_By wilsonge
avatar wilsonge
wilsonge - comment - 15 Jun 2019

Merging on good test + code review. thanks!

avatar PhilETaylor
PhilETaylor - comment - 19 May 2021

everyone missed the typo of the method name here ;-)

geAttribute :-)

Add a Comment

Login with GitHub to post a comment