freman / caddy-reauth

Auth your Caddyserver requests against another server
MIT License
27 stars 15 forks source link

Error specifying LDAP timeout #16

Closed jbq closed 6 years ago

jbq commented 6 years ago

Hi, I try to setup reauth with LDAP backend following the example at: https://github.com/freman/caddy-reauth#ldap

I can't figure out how to specify the LDAP timeout, the example mentions "timeout":"5s" but this value is not recognized.

Here is my Caddyfile:

0.0.0.0

browse

log stdout

errors stdout

reauth {
    path /
    ldap {"host":"ldap.jumpcloud.com","port":636,"ldaps":true,"tls":false,"timeout":"5s","bindUsername":"xxx","bindPassword":"xxx","base":"ou=Users,o=xxx,dc=jumpcloud,dc=com","filter":"(&(objectClass=inetOrgPerson)(uid=%s))"}
}

Here is the caddy error log:

2018/07/04 12:24:23 json: cannot unmarshal string into Go struct field LDAP.timeout of type time.Duration for ldap (/etc/Caddyfile:11)
freman commented 6 years ago

Ok, that's a relatively simple fix, time.Duration isn't directly unmarshalable.

I'm tempted to refactor that and do away with the json so it's consistent with the others. It also needs better tests.

How urgent do you need a fix? I can knock out a quick patch now, for you, or I can refactor it in the morning to be more consistent with the other modules?

jbq commented 6 years ago

Not urgent as I finally understood I was behind a f**ù%%£££ corporate firewall :-)

jbq commented 6 years ago

Indeed getting rid of JSON as configuration format would be very valuable

freman commented 6 years ago

Hey, wanna test now? The push I just pushed up should solve this one and https://github.com/freman/caddy-reauth/issues/18

It's no longer json and the arguments have changed, and there's connection pooling.

It's probably worth me pointing out that base and filter really do require "s if you have commas in the string

edit: also, to use ldaps make your url ldaps://ldap.example.com:636 - port is actually optional 😀

jbq commented 6 years ago

Wow you did an impressive job rewriting the LDAP backend. But I'm sorry I cannot afford to test the new implementation anymore, I switched to nginx+ldap in-between, it's been working right out of the box :-|

freman commented 6 years ago

Sa'll good, I should have been a bit more careful accepting that original merge request, was just excited someone did it for me, I honestly think no-one's actually used it after all this time.