add samaccountname to session #254

butonic · 2018-06-29T14:33:47Z

use samaccountname for wnd

codecov-io · 2018-06-29T15:42:13Z

Codecov Report

Merging #254 into master will decrease coverage by <.01%.
The diff coverage is 43.47%.

@@             Coverage Diff              @@
##             master     #254      +/-   ##
============================================
- Coverage     35.39%   35.39%   -0.01%     
- Complexity     1361     1368       +7     
============================================
  Files            32       32              
  Lines          3936     3953      +17     
============================================
+ Hits           1393     1399       +6     
- Misses         2543     2554      +11

Impacted Files	Coverage Δ	Complexity Δ
lib/Configuration.php	`94.85% <ø> (ø)`	`93 <0> (ø)`	⬇️
lib/Connection.php	`71.86% <ø> (ø)`	`113 <0> (ø)`	⬇️
lib/User_LDAP.php	`46.53% <0%> (-2.43%)`	`39 <2> (+2)`
lib/User_Proxy.php	`0% <0%> (ø)`	`43 <2> (+2)`	⬆️
lib/User/Manager.php	`56.25% <66.66%> (+0.46%)`	`67 <0> (+1)`	⬆️
lib/User/UserEntry.php	`97.7% <88.88%> (-0.71%)`	`56 <6> (+2)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d7252f...bb4c72c. Read the comment docs.

cdamken · 2018-06-29T16:25:00Z

Could be here possible to add a configuration option in the config.php like:

"wnd.login.attribute"  => "cn",

In case that the server in an LDAP and does not have sAMAccouontName available?

PVince81 · 2018-07-30T10:23:14Z

lib/User_LDAP.php

+		// hack to store a samaccountname in the session
+		$samaccountname = $userEntry->getSAMAccountName();
+		if ($samaccountname !== null) {
+			\OC::$server->getSession()->set('samaccountname', $samaccountname);


I'd like to call this "alternateLogin" or something more generic to make it reusable

@DeepDiver1975 are you ok with this approach ?
also see https://github.com/owncloud/windows_network_drive/pull/175 and owncloud/core#31951 for usage

if we agree I'll adjust the PRs to rename the attributes

had a chat with @DeepDiver1975 and he's ok with this

PVince81 · 2018-07-31T11:42:44Z

Could be here possible to add a configuration option in the config.php like:

in that case I'd rather have a more generic setting called something like ldap.altloginname_attribute => "cn" which would then tell the LDAP app to take the altloginname value (which will be used by WND) from another attribute

PVince81 · 2018-08-09T12:48:25Z

make LDAP attribute configurable that will be used for "altloginname"

tomneedham · 2018-08-15T11:23:50Z

lib/Connection.php

@@ -94,6 +99,9 @@ public function __construct(ILDAPWrapper $ldap, Configuration $configuration, $c
 		$this->hasPagedResultSupport =
 			(int)$this->configuration->ldapPagingSize !== 0
 			|| $this->getLDAP()->hasPagedResultSupport();
+
+		// TODO: should this be in Configuration / Wizard ?
+		$this->altLoginAttribute = $this->configuration->getCoreConfig()->getSystemValue('user_ldap.altloginattribute', 'samaccountname');


We need to document this - somewhere. It will get lost / forgotten.

butonic · 2018-08-16T09:28:50Z

In the context of ldap this is not an alternate login name. It is the username. The username is what is used to mount the wnd. In general we need to map several attributes to an owncloud account:

uid: should be a uuid attribute in ldap, AD uses objectguid, openldap uses entryuuid. This is used to uniquely identify a user in subsequent sync runs. The user will be looked up by this id and the other attributes will be updated.
displayname: typically displayname in ldap
email: typically mail
additional search attributes that are synced and allow finding the user in the sharing dialog.

Currently ownCloud cannot explicitly store the username that might have been configured as the login attribute, eg cn or samaccountname. Basically it gets lost after login.

To clarify: there is no alternate login. It literally IS the username.

I'll clean this up later. Currently looking into the core part.

PVince81 · 2018-08-20T12:14:48Z

then we could rename "altLoginName" in the core PRs to something like "extusername" or "externalusername" ? (meaning the user name from the external system, as it's not stored currently)

PVince81 · 2018-09-04T20:19:18Z

lib/User_Proxy.php

+		IProvidesQuotaBackend,
+		IProvidesExtendedSearchBackend,
+		IProvidesEMailBackend,
+		IProvidesUserNameBackend


since this only exists in 10.0.10 through owncloud/core#32579, we need to set min-version to 10.0.10

@butonic please adjust

PVince81 · 2018-09-25T10:22:35Z

@butonic can you finish this or hand over ?

butonic · 2018-10-10T08:55:38Z

I renamed alt login name to username because that is what it is. ldapUserName is currently redundant with the ldapExpertUsernameAttr but the latter determines the uid that will be used in owncloud. we want to move to uuid there anyway. but that is a different issue. For now, I want to name the new option correctly.

It can currently only be set via cli and defaults to samaccountname. in core it is only used for wnd mounts, so no harm done.

PVince81 · 2018-10-16T13:38:30Z

some test needs adjusting for the new attribute:

' contains "| ldapAgentPassword             |              |".

/var/www/owncloud/apps/user_ldap/tests/unit/CommandTest.php:67

add samaccountname to session Renamed "samaccountname" session var to "altloginname" Configurable altloginname attribute allow username sync instead of storing in session IProvidesUserNameBackend requires 10.0.10 fix codestyle fix rebase fix copy paste use Username instead of alt loginname, use ldap Configuration, add test fix CommandTest

PVince81

👍

PVince81 reviewed Jul 30, 2018

View reviewed changes

PVince81 self-assigned this Jul 30, 2018

PVince81 added the 2 - Developing label Jul 30, 2018

PVince81 added this to the development milestone Jul 30, 2018

PVince81 force-pushed the bugfix/samaccountname-as-wnd-username branch from 7cf710d to 74cce70 Compare July 31, 2018 11:52

tomneedham reviewed Aug 15, 2018

View reviewed changes

PVince81 mentioned this pull request Aug 24, 2018

LDAP 0.12.0 release #252

Closed

21 tasks

butonic mentioned this pull request Sep 3, 2018

user_ldap 0.11.1 release #282

Closed

21 tasks

PVince81 reviewed Sep 4, 2018

View reviewed changes

butonic force-pushed the bugfix/samaccountname-as-wnd-username branch from 8626a0a to 8694cd4 Compare October 9, 2018 13:13

butonic self-assigned this Oct 10, 2018

butonic force-pushed the bugfix/samaccountname-as-wnd-username branch from 161a5d9 to f158e01 Compare October 10, 2018 08:51

butonic force-pushed the bugfix/samaccountname-as-wnd-username branch 3 times, most recently from 41ff3df to dc19db2 Compare October 18, 2018 08:02

butonic force-pushed the bugfix/samaccountname-as-wnd-username branch from dc19db2 to bb4c72c Compare October 19, 2018 12:27

PVince81 approved these changes Oct 20, 2018

View reviewed changes

PVince81 merged commit 1dd308f into master Oct 20, 2018

PVince81 deleted the bugfix/samaccountname-as-wnd-username branch October 20, 2018 10:07

PVince81 removed this from the development milestone Oct 20, 2018

PVince81 added this to the QA milestone Oct 20, 2018

davitol mentioned this pull request Oct 25, 2018

Tracking results for LDAP 0.12.0 #296

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add samaccountname to session #254

add samaccountname to session #254

butonic commented Jun 29, 2018

codecov-io commented Jun 29, 2018 •

edited

cdamken commented Jun 29, 2018

PVince81 Jul 30, 2018

PVince81 Jul 30, 2018

PVince81 commented Jul 31, 2018

PVince81 commented Aug 9, 2018 •

edited

tomneedham Aug 15, 2018

butonic commented Aug 16, 2018

PVince81 commented Aug 20, 2018

PVince81 Sep 4, 2018

PVince81 Sep 12, 2018

PVince81 commented Sep 25, 2018

butonic commented Oct 10, 2018

PVince81 commented Oct 16, 2018

PVince81 left a comment

add samaccountname to session #254

add samaccountname to session #254

Conversation

butonic commented Jun 29, 2018

codecov-io commented Jun 29, 2018 • edited

Codecov Report

cdamken commented Jun 29, 2018

PVince81 Jul 30, 2018

Choose a reason for hiding this comment

PVince81 Jul 30, 2018

Choose a reason for hiding this comment

PVince81 commented Jul 31, 2018

PVince81 commented Aug 9, 2018 • edited

tomneedham Aug 15, 2018

Choose a reason for hiding this comment

butonic commented Aug 16, 2018

PVince81 commented Aug 20, 2018

PVince81 Sep 4, 2018

Choose a reason for hiding this comment

PVince81 Sep 12, 2018

Choose a reason for hiding this comment

PVince81 commented Sep 25, 2018

butonic commented Oct 10, 2018

PVince81 commented Oct 16, 2018

PVince81 left a comment

Choose a reason for hiding this comment

codecov-io commented Jun 29, 2018 •

edited

PVince81 commented Aug 9, 2018 •

edited