unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
@ 2011-03-04 20:47 Leonidas Tsampros
  2011-03-05  3:07 ` Ted Zlatanov
  0 siblings, 1 reply; 8+ messages in thread
From: Leonidas Tsampros @ 2011-03-04 20:47 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 489 bytes --]

Hi,

Attached you can find a small patch to make ldap.el use auth-source if
passwd is not defined in the ldap-host-parameters-list.

I find it useful not to carry passwords in my configurations files, so I
was motivated enough to do that for ldap.el since I use it through
eudc-ldap.

Thanks to nnimap.el from which I copied this piece of code.

I do not think that I break any existing configuration with this
patch but I'm not entirely sure about that.

Best Regards,
Leonidas Tsampros


[-- Attachment #2: ldap patch auth-source --]
[-- Type: text/plain, Size: 2859 bytes --]

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: ltsampros@upnet.gr-20110302174918-h06k6ohsksc8ymju
# target_branch: file:///home/ltsampros/repos2/emacs/trunk/
# testament_sha1: 2d5eb512df6c9c23c2c736a832130bd0e48e6a4c
# timestamp: 2011-03-02 19:54:44 +0200
# base_revision_id: schwab@linux-m68k.org-20110302104247-\
#   z4p2vqooiv34g8bd
# 
# Begin patch
=== modified file 'lisp/net/ldap.el'
--- lisp/net/ldap.el	2011-01-25 04:08:28 +0000
+++ lisp/net/ldap.el	2011-03-02 17:49:18 +0000
@@ -36,6 +36,8 @@
 (require 'custom)
 (eval-when-compile (require 'cl))
 
+(autoload 'auth-source-search "auth-source")
+
 (defgroup ldap nil
   "Lightweight Directory Access Protocol."
   :version "21.1"
@@ -512,7 +514,15 @@
 	(scope (plist-get search-plist 'scope))
 	(binddn (plist-get search-plist 'binddn))
         (auth (plist-get search-plist 'auth))
-	(passwd (plist-get search-plist 'passwd))
+	(passwd (or (plist-get search-plist 'passwd)
+		    (let ((found (nth 0 (auth-source-search :host (if (boundp 'host) host)
+							    :create t))))
+		      (if found
+			  (let ((secret (plist-get found :secret)))
+			    (if (functionp secret)
+				(funcall secret)
+			      secret))))
+		    ))
 	(deref (plist-get search-plist 'deref))
 	(timelimit (plist-get search-plist 'timelimit))
 	(sizelimit (plist-get search-plist 'sizelimit))

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWU8YDq8AAcXfgFgwUOP/93Ik
RAC////wUAQed7Ne2q9a2HQeg0iZI/VHqZpD1GgNDTRp6jQAACNIpiZkaI9Gp6gAAAAAAyRqZqPU
mibU9Gk9Rpo0NGgBoyAGmiKBpoBo00aBoAAAACSQECYIZNExTTDQk9pRpo9CeUpISEyUkMyYBnh0
aJm5uBF/3z/I2eWRTNqED83b6CD6LhBcsGXrh+PyuHUdXTIf77qetLDMf79ycKu1YQibUEFRn4FP
YYfH7lxDCYmMezK++njXcgfPKpQ3p+ZsE/ZHNDBuRpVQEJjOrQoOlVKZCHSCIEyRNYcqUCFVibA9
w3CnnukPE7g/n8JvJPcZEubw3rfVEJpXfRZO1VVwS6N4N5iTjzQMg8WT0yvFLC9XvU5KDcGoqFUI
Hkl2IQJFUGgcva4IsMytC5KYa6V92NmSRXdVDHPrWJj2vFy2q01QPTZE20riwjFX7FAqcKSz/h8g
axx0oaEvagijCZ5nCmcEUUZg1nR9CpLBRPmNRUOZ7QkJ0NiibkYsFKocVhm0QWGUM5zfxmJUjjxu
bLVHUT0HWVfK9lrfG9+/lwKb3vwPlZU4QvYy1lDpgc4N4GlSxAyrmdzHAZz02mlkBulz4xJhqg7V
2vsMDYMrBdmrCFKdasBRKrDdQ8hbOqFpAdk91UidlnnB6EecMKEgIQKNWBKMhUA1SeIlsvqTivQc
27XpuO0lzYgkDyn3o9QqiakA3wBecQALdKV0a3dC205DivgNfTpitsgBCsnrrqAQiKHARR+bf63p
s6HvGp1U6IHaEDcvVfVGMtB3eeGYtIHTRY+zgi3Gaxdca0SINzYnB+RoybSGMilYUrtJhZXcLc5f
SHbVYddd8ujGV78mhZxO3GETzqoDEsYBNHKBhoSQbYMpF2VE+RFU1wOqLMzGiH3cRwNEy/erS/Yx
x2v48DgVcBg1gQcilOkFyxW47DQ071M/Xd5FrHe3RnOLw6LwR9iWMf3yS0yYGRMTtAfYU/aXkKav
CZpYYT+E50cVXzNygPHeIVznyWeEcQqq+oTDLWuc4QFs9E3IW3aExb6M0q+UQhWBUMLdyNOAVFfG
QSh2llUyyWRrfbELQqBiDxv6kJYIRgJDzuYo2XyYJIaJdilC2ExuqyI5OTmWDhfTnSsruwVafHjQ
GrpY/8KotoC7xBrlhBSA6h43yXr2hwj0belx7Ft04uZa/J8i2UTklraiQFa6KizkLemeD8aQcD+x
yGRV3XF0ghV4vB2j70DyCGq4u3wSnu+HJeYq1OxasmIfpsWi19oNxFWuEgrwU70wLLRQVgGBLy8h
iSISojKoj28BzKsDnzhM1VB9FOyszjKsKAlSk5avkO1Pdg3atQeiVTgsuFQql1TResA/pPyVJyTR
tWKcsV1F10V1pL/xdyRThQkE8YDq8A==

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-04 20:47 [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list Leonidas Tsampros
@ 2011-03-05  3:07 ` Ted Zlatanov
  2011-03-05  9:16   ` Leonidas Tsampros
  0 siblings, 1 reply; 8+ messages in thread
From: Ted Zlatanov @ 2011-03-05  3:07 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Fri, 04 Mar 2011 22:47:20 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote: 

LT> Attached you can find a small patch to make ldap.el use auth-source if
LT> passwd is not defined in the ldap-host-parameters-list.

Cool!  I made some changes to it, please let me know what you think:

- must pass `auth-source' set to t in the SEARCH-PLIST to activate this
  functionality (I don't know if you agree)

- search for 'port "ldap"' entries in the netrc file (should we default
  to no port?)

- take the password and use it

- take "binddn" or "user" tokens for `binddn'

- take "base" tokens for `base'

so the user can have a netrc line like this

machine X user Y password Z base B port ldap

I tried to document it to explain all this.  If I'm going in the wrong
direction, tell me.  I don't know ldap.el so well.

Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: auth-source.ldap.patch --]
[-- Type: text/x-diff, Size: 3439 bytes --]

=== modified file 'lisp/net/ldap.el'
--- lisp/net/ldap.el	2011-01-25 04:08:28 +0000
+++ lisp/net/ldap.el	2011-03-05 03:05:46 +0000
@@ -36,6 +36,8 @@
 (require 'custom)
 (eval-when-compile (require 'cl))
 
+(autoload 'auth-source-search "auth-source")
+
 (defgroup ldap nil
   "Lightweight Directory Access Protocol."
   :version "21.1"
@@ -480,6 +482,21 @@
   "Perform a search on a LDAP server.
 SEARCH-PLIST is a property list describing the search request.
 Valid keys in that list are:
+
+  `auth-source', if non-nil, will use `auth-source-search' and
+will grab the :host, :secret, :base, and (:user or :binddn)
+tokens into the `host', `passwd', `base', and `binddn' parameters
+respectively if they are not provided in SEARCH-PLIST.  So for
+instance *each* of these netrc lines has the same effect if you
+ask for the host \"ldapserver:2400\":
+
+  machine ldapserver:2400 login myDN secret myPassword base myBase port ldap
+  machine ldapserver:2400 binddn myDN secret myPassword port ldap
+  login myDN secret myPassword base myBase port ldap
+
+but if you have more than one in your netrc file, only the first
+matching one will be used.
+
   `host' is a string naming one or more (blank-separated) LDAP servers to
 to try to connect to.  Each host name may optionally be of the form HOST:PORT.
   `filter' is a filter string for the search as described in RFC 1558.
@@ -500,19 +517,35 @@
 its distinguished name DN.
 The function returns a list of matching entries.  Each entry is itself
 an alist of attribute/value pairs."
-  (let ((buf (get-buffer-create " *ldap-search*"))
+  (let* ((buf (get-buffer-create " *ldap-search*"))
 	(bufval (get-buffer-create " *ldap-value*"))
 	(host (or (plist-get search-plist 'host)
 		  ldap-default-host))
+         ;; find entries with port "ldap" that match the requested host if any
+         (asfound (when (plist-get search-plist 'auth-source)
+                    (nth 0 (auth-source-search :host (or host t)
+                                               :port "ldap"
+                                               :create t))))
+         ;; if no host was requested, get it from the auth-source entry
+         (host (or host (plist-get asfound :host)))
+         ;; get the password from the auth-source
+         (passwd (or (plist-get search-plist 'passwd)
+                     (plist-get asfound :secret)))
+         ;; convert the password from a function call if needed
+         (passwd (if (functionp passwd) (funcall passwd) passwd))
+         ;; get the binddn from the search-list or from the
+         ;; auth-source user or binddn tokens
+         (binddn (or (plist-get search-plist 'binddn)
+                     (plist-get asfound :user)
+                     (plist-get asfound :binddn)))
+         (base (or (plist-get search-plist 'base)
+                   (plist-get asfound :base)
+                   ldap-default-base))
 	(filter (plist-get search-plist 'filter))
 	(attributes (plist-get search-plist 'attributes))
 	(attrsonly (plist-get search-plist 'attrsonly))
-	(base (or (plist-get search-plist 'base)
-		  ldap-default-base))
 	(scope (plist-get search-plist 'scope))
-	(binddn (plist-get search-plist 'binddn))
         (auth (plist-get search-plist 'auth))
-	(passwd (plist-get search-plist 'passwd))
 	(deref (plist-get search-plist 'deref))
 	(timelimit (plist-get search-plist 'timelimit))
 	(sizelimit (plist-get search-plist 'sizelimit))


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-05  3:07 ` Ted Zlatanov
@ 2011-03-05  9:16   ` Leonidas Tsampros
  2011-03-07 10:55     ` Ted Zlatanov
  0 siblings, 1 reply; 8+ messages in thread
From: Leonidas Tsampros @ 2011-03-05  9:16 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Fri, 04 Mar 2011 22:47:20 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote:
>
> LT> Attached you can find a small patch to make ldap.el use auth-source if
> LT> passwd is not defined in the ldap-host-parameters-list.
>
> Cool!  I made some changes to it, please let me know what you think:
>
> - must pass `auth-source' set to t in the SEARCH-PLIST to activate this
>   functionality (I don't know if you agree)

Agreed. It's nice not to catch users by surprise.

> - search for 'port "ldap"' entries in the netrc file (should we default
>   to no port?)

This is the only part I disagree a bit. For example, my imap host is the
same with my ldap host, the credentials are the same for both services,
and both imap/ldap use the default ports. (This is a classic case of an
Exchange server I think)

So I was thinking that it might be good idea to use the same authinfo line
for both of them, IN CASE, there is not something more specific in the
authinfo file.

However, I'm not strong about this, and I really didn't mind having a
second authinfo line at all.

> - take the password and use it
>
> - take "binddn" or "user" tokens for `binddn'
>
> - take "base" tokens for `base'
>
> so the user can have a netrc line like this
>
> machine X user Y password Z base B port ldap
>

All of the above are self explanatory, and really thanks for doing it.

> I tried to document it to explain all this.  If I'm going in the wrong
> direction, tell me.  I don't know ldap.el so well.
>
> Ted
>

I tested your patch and I think that it works really great!
Many thanks!

(Really there is no wrong direction in this. We just don't want to carry
passwords in our configuration files. :)




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-05  9:16   ` Leonidas Tsampros
@ 2011-03-07 10:55     ` Ted Zlatanov
  2011-03-10 21:05       ` Leonidas Tsampros
  0 siblings, 1 reply; 8+ messages in thread
From: Ted Zlatanov @ 2011-03-07 10:55 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

On Sat, 05 Mar 2011 11:16:39 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote: 

LT> Ted Zlatanov <tzz@lifelogs.com> writes:
>> On Fri, 04 Mar 2011 22:47:20 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote:
>> 
LT> Attached you can find a small patch to make ldap.el use auth-source if
LT> passwd is not defined in the ldap-host-parameters-list.
>> 
>> Cool!  I made some changes to it, please let me know what you think:
>> 
>> - must pass `auth-source' set to t in the SEARCH-PLIST to activate this
>> functionality (I don't know if you agree)

LT> Agreed. It's nice not to catch users by surprise.

>> - search for 'port "ldap"' entries in the netrc file (should we default
>> to no port?)

LT> This is the only part I disagree a bit. For example, my imap host is the
LT> same with my ldap host, the credentials are the same for both services,
LT> and both imap/ldap use the default ports. (This is a classic case of an
LT> Exchange server I think)

This makes sense, especially since ldap.el hosts can be in the format
"server:port".  I removed the "port ldap" requirement.

>> - take the password and use it
>> 
>> - take "binddn" or "user" tokens for `binddn'
>> 
>> - take "base" tokens for `base'

LT> (Really there is no wrong direction in this. We just don't want to carry
LT> passwords in our configuration files. :)

Agreed, and that's why I've been working on auth-source.el.

Try the attached patch.  If it works for you I'll push it into the
trunk.  Let me know if I need to update any other documentation besides
the docstring.  It looks like only Chong Yidong has made significant
changes to ldap.el since 2005 so I'm not CC-ing the original author.

Thanks
Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: auth-source.ldap.patch --]
[-- Type: text/x-diff, Size: 3405 bytes --]

=== modified file 'lisp/net/ldap.el'
--- lisp/net/ldap.el	2011-01-25 04:08:28 +0000
+++ lisp/net/ldap.el	2011-03-07 10:48:11 +0000
@@ -36,6 +36,8 @@
 (require 'custom)
 (eval-when-compile (require 'cl))
 
+(autoload 'auth-source-search "auth-source")
+
 (defgroup ldap nil
   "Lightweight Directory Access Protocol."
   :version "21.1"
@@ -480,6 +482,22 @@
   "Perform a search on a LDAP server.
 SEARCH-PLIST is a property list describing the search request.
 Valid keys in that list are:
+
+  `auth-source', if non-nil, will use `auth-source-search' and
+will grab the :host, :secret, :base, and (:user or :binddn)
+tokens into the `host', `passwd', `base', and `binddn' parameters
+respectively if they are not provided in SEARCH-PLIST.  So for
+instance *each* of these netrc lines has the same effect if you
+ask for the host \"ldapserver:2400\":
+
+  machine ldapserver:2400 login myDN secret myPassword base myBase
+  machine ldapserver:2400 binddn myDN secret myPassword port ldap
+  login myDN secret myPassword base myBase
+
+but if you have more than one in your netrc file, only the first
+matching one will be used.  Note the \"port ldap\" part is NOT
+required.
+
   `host' is a string naming one or more (blank-separated) LDAP servers to
 to try to connect to.  Each host name may optionally be of the form HOST:PORT.
   `filter' is a filter string for the search as described in RFC 1558.
@@ -500,19 +518,34 @@
 its distinguished name DN.
 The function returns a list of matching entries.  Each entry is itself
 an alist of attribute/value pairs."
-  (let ((buf (get-buffer-create " *ldap-search*"))
+  (let* ((buf (get-buffer-create " *ldap-search*"))
 	(bufval (get-buffer-create " *ldap-value*"))
 	(host (or (plist-get search-plist 'host)
 		  ldap-default-host))
+         ;; find entries with port "ldap" that match the requested host if any
+         (asfound (when (plist-get search-plist 'auth-source)
+                    (nth 0 (auth-source-search :host (or host t)
+                                               :create t))))
+         ;; if no host was requested, get it from the auth-source entry
+         (host (or host (plist-get asfound :host)))
+         ;; get the password from the auth-source
+         (passwd (or (plist-get search-plist 'passwd)
+                     (plist-get asfound :secret)))
+         ;; convert the password from a function call if needed
+         (passwd (if (functionp passwd) (funcall passwd) passwd))
+         ;; get the binddn from the search-list or from the
+         ;; auth-source user or binddn tokens
+         (binddn (or (plist-get search-plist 'binddn)
+                     (plist-get asfound :user)
+                     (plist-get asfound :binddn)))
+         (base (or (plist-get search-plist 'base)
+                   (plist-get asfound :base)
+                   ldap-default-base))
 	(filter (plist-get search-plist 'filter))
 	(attributes (plist-get search-plist 'attributes))
 	(attrsonly (plist-get search-plist 'attrsonly))
-	(base (or (plist-get search-plist 'base)
-		  ldap-default-base))
 	(scope (plist-get search-plist 'scope))
-	(binddn (plist-get search-plist 'binddn))
         (auth (plist-get search-plist 'auth))
-	(passwd (plist-get search-plist 'passwd))
 	(deref (plist-get search-plist 'deref))
 	(timelimit (plist-get search-plist 'timelimit))
 	(sizelimit (plist-get search-plist 'sizelimit))


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-07 10:55     ` Ted Zlatanov
@ 2011-03-10 21:05       ` Leonidas Tsampros
  2011-03-10 22:05         ` Ted Zlatanov
  0 siblings, 1 reply; 8+ messages in thread
From: Leonidas Tsampros @ 2011-03-10 21:05 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:
> On Sat, 05 Mar 2011 11:16:39 +0200 Leonidas Tsampros
> <ltsampros@upnet.gr> wrote:
>
> LT> Ted Zlatanov <tzz@lifelogs.com> writes:
>>> On Fri, 04 Mar 2011 22:47:20 +0200 Leonidas Tsampros
>>> <ltsampros@upnet.gr> wrote:
>>>
> LT> Attached you can find a small patch to make ldap.el use auth-source if
> LT> passwd is not defined in the ldap-host-parameters-list.
>>>
>>> Cool!  I made some changes to it, please let me know what you think:
>>>
>>> - must pass `auth-source' set to t in the SEARCH-PLIST to activate this
>>> functionality (I don't know if you agree)
>
> LT> Agreed. It's nice not to catch users by surprise.
>
>>> - search for 'port "ldap"' entries in the netrc file (should we default
>>> to no port?)
>
> LT> This is the only part I disagree a bit. For example, my imap host is the
> LT> same with my ldap host, the credentials are the same for both services,
> LT> and both imap/ldap use the default ports. (This is a classic case of an
> LT> Exchange server I think)
>
> This makes sense, especially since ldap.el hosts can be in the format
> "server:port".  I removed the "port ldap" requirement.
>
>>> - take the password and use it
>>>
>>> - take "binddn" or "user" tokens for `binddn'
>>>
>>> - take "base" tokens for `base'
>
> LT> (Really there is no wrong direction in this. We just don't want to carry
> LT> passwords in our configuration files. :)
>
> Agreed, and that's why I've been working on auth-source.el.
>
> Try the attached patch.  If it works for you I'll push it into the
> trunk.  Let me know if I need to update any other documentation besides
> the docstring.  It looks like only Chong Yidong has made significant
> changes to ldap.el since 2005 so I'm not CC-ing the original author.
>
> Thanks
> Ted
>

Hi Ted,

sorry for the late reply but here are my findings after applying your
patch:

1) authinfo machine ip-address login username password secret-key imap

and (setq ldap-host-parameters-alist
      (quote (("ip-address" base "dc=domain,dc=com"
               binddn "domain\\username" "auth-source "yes"))))

Logging in to both the LDAP and the IMAP server works as expected (on
the assumption that they are the same hosts, and this is my personal
setup, so I'm pretty happy.

2) authinfo machine ip-address login username password secret-key imap
   authinfo machine ip-address binddn domain\username password secret-key port ldap

and (setq ldap-host-parameters-alist
      (quote (("ip-address" base "dc=domain,dc=com"
               auth-source "yes"))))

The above is the more general case of having a separate LDAP
server. This case doesn't work, since auth-source-search returns the
first entry and so (plist-get asfound :binddin) returns nothing.

In order to work around this case, I thought of the following:

3) authinfo machine ip-address login username password secret-key imap
   authinfo machine ip-address:389 binddn domain\username password secret-key

and (setq ldap-host-parameters-alist
      (quote (("ip-address:389" base "dc=domain,dc=com"
               auth-source "yes"))))

which should work as expected but I think it breaks eudc-ldap. The
module seems trying to match only the 'ip-address' part of the
ldap-host-parameters-alist entry and as such a check to see if a base dn
is defined fails. However I want to double check this again.

Anyway, since we have the auth-source switch to enable this explicitly,
I think you can commit this pretty safely and then figure out how to
move with case 2) and 3), as I don't think that it breaks any
functionality.

I will try to give it a shot too, in case I come up with a plan.

Thanks for the patch and the effort, and please correct me if I have
misunderstood the user scenarios that we would like to support wrt to
ldap configuration.

Best Regards,
Leonidas Tsampros



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-10 21:05       ` Leonidas Tsampros
@ 2011-03-10 22:05         ` Ted Zlatanov
  2011-03-11 13:50           ` Leonidas Tsampros
  0 siblings, 1 reply; 8+ messages in thread
From: Ted Zlatanov @ 2011-03-10 22:05 UTC (permalink / raw)
  To: emacs-devel

On Thu, 10 Mar 2011 23:05:25 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote: 

LT> 1) authinfo machine ip-address login username password secret-key imap

LT> and (setq ldap-host-parameters-alist
LT>       (quote (("ip-address" base "dc=domain,dc=com"
LT>                binddn "domain\\username" "auth-source "yes"))))

LT> Logging in to both the LDAP and the IMAP server works as expected (on
LT> the assumption that they are the same hosts, and this is my personal
LT> setup, so I'm pretty happy.

Good.

LT> 2) authinfo machine ip-address login username password secret-key imap
LT>    authinfo machine ip-address binddn domain\username password secret-key port ldap

LT> and (setq ldap-host-parameters-alist
LT>       (quote (("ip-address" base "dc=domain,dc=com"
LT>                auth-source "yes"))))

LT> The above is the more general case of having a separate LDAP
LT> server. This case doesn't work, since auth-source-search returns the
LT> first entry and so (plist-get asfound :binddin) returns nothing.

Yeah, we don't require the "ldap" port...  I think it's OK, the user can
reorder them since IMAP does require the port.

LT> In order to work around this case, I thought of the following:

LT> 3) authinfo machine ip-address login username password secret-key imap
LT>    authinfo machine ip-address:389 binddn domain\username password secret-key

LT> and (setq ldap-host-parameters-alist
LT>       (quote (("ip-address:389" base "dc=domain,dc=com"
LT>                auth-source "yes"))))

LT> which should work as expected but I think it breaks eudc-ldap. The
LT> module seems trying to match only the 'ip-address' part of the
LT> ldap-host-parameters-alist entry and as such a check to see if a base dn
LT> is defined fails. However I want to double check this again.

I think this has to work so users can specify other LDAP ports.  So we
need to fix eudc-ldap then.  Can you look at what's needed?  I don't use
that package so I'm hardly the one to fix this issue.

Let me know if you agree with the above and I'll push the patch tonight.

Thanks
Ted




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-10 22:05         ` Ted Zlatanov
@ 2011-03-11 13:50           ` Leonidas Tsampros
  2011-03-13 22:06             ` Ted Zlatanov
  0 siblings, 1 reply; 8+ messages in thread
From: Leonidas Tsampros @ 2011-03-11 13:50 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:
> On Thu, 10 Mar 2011 23:05:25 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote: 
>
> LT> 1) authinfo machine ip-address login username password secret-key imap
>
> LT> and (setq ldap-host-parameters-alist
> LT>       (quote (("ip-address" base "dc=domain,dc=com"
> LT>                binddn "domain\\username" "auth-source "yes"))))
>
> LT> Logging in to both the LDAP and the IMAP server works as expected (on
> LT> the assumption that they are the same hosts, and this is my personal
> LT> setup, so I'm pretty happy.
>
> Good.
>
> LT> 2) authinfo machine ip-address login username password secret-key imap
> LT>    authinfo machine ip-address binddn domain\username password secret-key port ldap
>
> LT> and (setq ldap-host-parameters-alist
> LT>       (quote (("ip-address" base "dc=domain,dc=com"
> LT>                auth-source "yes"))))
>
> LT> The above is the more general case of having a separate LDAP
> LT> server. This case doesn't work, since auth-source-search returns the
> LT> first entry and so (plist-get asfound :binddin) returns nothing.
>
> Yeah, we don't require the "ldap" port...  I think it's OK, the user can
> reorder them since IMAP does require the port.
>
> LT> In order to work around this case, I thought of the following:
>
> LT> 3) authinfo machine ip-address login username password secret-key imap
> LT>    authinfo machine ip-address:389 binddn domain\username password secret-key
>
> LT> and (setq ldap-host-parameters-alist
> LT>       (quote (("ip-address:389" base "dc=domain,dc=com"
> LT>                auth-source "yes"))))
>
> LT> which should work as expected but I think it breaks eudc-ldap. The
> LT> module seems trying to match only the 'ip-address' part of the
> LT> ldap-host-parameters-alist entry and as such a check to see if a base dn
> LT> is defined fails. However I want to double check this again.
>
> I think this has to work so users can specify other LDAP ports.  So we
> need to fix eudc-ldap then.  Can you look at what's needed?  I don't use
> that package so I'm hardly the one to fix this issue.
>
> Let me know if you agree with the above and I'll push the patch tonight.
>
> Thanks
> Ted

Of course I agree since the specific testcases work fine.

FWIW the problem I noticed in eudc-ldap seem to be something different
and most probably is irrelevant to the ldap.el change. I'll probably
take a look at it again within the weekend.

Thanks,
Leonidas Tsampros



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list
  2011-03-11 13:50           ` Leonidas Tsampros
@ 2011-03-13 22:06             ` Ted Zlatanov
  0 siblings, 0 replies; 8+ messages in thread
From: Ted Zlatanov @ 2011-03-13 22:06 UTC (permalink / raw)
  To: emacs-devel

On Fri, 11 Mar 2011 15:50:49 +0200 Leonidas Tsampros <ltsampros@upnet.gr> wrote: 

LT> Of course I agree since the specific testcases work fine.

I have now pushed my change; it's optional and off by default as we
agreed.  Please suggest changes to the eudc.texi manual or elsewhere in
the Emacs manuals since I don't use EUDC to explain this change to
users.

We can turn it on by default if users request that.

Ted




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-03-13 22:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-04 20:47 [PATCH] ldap.el: use auth-source if passwd is not defined in ldap-host-parameters-list Leonidas Tsampros
2011-03-05  3:07 ` Ted Zlatanov
2011-03-05  9:16   ` Leonidas Tsampros
2011-03-07 10:55     ` Ted Zlatanov
2011-03-10 21:05       ` Leonidas Tsampros
2011-03-10 22:05         ` Ted Zlatanov
2011-03-11 13:50           ` Leonidas Tsampros
2011-03-13 22:06             ` Ted Zlatanov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).