Vlado,
Excellent, I will get to this as soon as possible.
Thanks again,
Trevor
On Wed, 2011-09-07 at 17:49 +0200, Vladimir Motoska wrote:
Hi Trevor,
Here is the modified version of the patch.
The things I done:
- finished script authenticator ( needs testing )
- adding a sample script would be good
- added possibility of changing passwords via web UI
- added option external-sync for pump external users
- rewrote a bit the main authenticator class
Hope it will work ;)
B.R.
Vlado
On (02/09/11 18:07), Trevor McKay wrote:
> Vlado,
>
> Okay, thanks.
>
> 1) I didn't realize about the ",". The patch I have has the splitter
as
> a space, " ". I was thinking that not using space as a splitter would
> allow us to relax the form from "ldap=blah" so there could be whitespace
> around the equals. But, space, semicolon, or anything else that is
> legal and unambiguous is fine.
>
> 2) Moving the auth parameter to common is fine, too. In fact, anything
> set in [common] can also be set individually in the other sections,
> since they inherit from [common]. I often place general config stuff in
> the common section when I think it might be used by more than 1 app,
> even if the current usage happens to be just for cumin-data, for
> example.
>
> 4) Someone here at Red Hat actually dug up a reference in the 2.4
> reference manual that says "global ldap" followed by "import
ldap" is
> actually illegal Python, but not enforced! Who knew? :) We can make
> sure there is a try block around the import.
>
> "
> Actually this is illegal according to
>
http://docs.python.org/release/2.4/ref/global.html
> "
>
> Talk to you soon. Holiday here in the US on Monday, but I'll be back
> Tuesday.
>
> Best,
>
> Trevor
>
> On Fri, 2011-09-02 at 22:04 +0200, Vladimir Motoska wrote:
> > Hi Trevor,
> >
> > I just checked out and patched the cumin from svn and I have a couple of
> > questions.
> >
> > 1) The splitter character mentioned in "3". This is a very bad idea
> > because ldap urls contain "," so you get a "Unsupported
authenticator
> > type" error. Either we leave the splitter as semicolon or we can do
> > something like
> > --
> > auth: ldap,script,ldap,internal
> > auth_1: ldap://server:port/dc=some,dc=base ... .
> > auth_2: /bin/auth_script %s:%p
> > auth_3: ldap://server2:port/dc=other,dc=base ....
> > --
> >
> > 2) I moved the auth parameter from the web config section to common so
> > when invoking cumin-admin we have all the instances of app.authenticators.
> > This is required to have the import from external source functionality.
> > The other approach would be to have a separate sync tool but I think
> > this is too much ;)
> >
> > 3) The password change isn't so hard for plain ldap or script. It would
> > be harder for ldap that proxies auth to kerberos.
> >
> > 4) The import statement inside the class was intended to catch the
> > exception if the ldap module ins not present. If we use a global import
> > the app will crash ( but I think this is not an issue with regular
> > packaging and dependencies )
> >
> > If you can give me feedback about 2 first points so I can adapt the
> > changes and send the final version.
> >
> > Thanks
> >
> > Vlado
> >
> >
> > On (01/09/11 13:32), Trevor McKay wrote:
> > > Vladimir,
> > >
> > > Okay, thanks. I'm looking forward to it! This patch will be going
> > > into the cumin "trunk" on fedorahosted very soon.
> > >
> > > In the meantime, here is a modified patch (generated with svn diff)
> > > against revision 4794 from the svn repository.
> > >
> > > I changed a few things:
> > >
> > > 1) The biggest issue for an existing install base is a reschema of the
> > > database. We like to try to avoid that if possible. In this case,
> > > there is an alternative to adding the "external" column to the
User
> > > table. Since a zero-length password field in the database is impossible
> > > in previous versions of cumin (even "" for a password is crypted
and
> > > produces a non-zero-length field), we can use len(user.password) == 0 as
> > > the sentinel value for an external user. This avoids any reschema but
> > > is functionally equivalent, and it remove changes to cumin.xml and
> > > admin.py.
> > >
> > > 2) Also for backward compatibility of existing customers and
> > > documentation, I reverted the "add-user" command and added an
explicit
> > > "external-user" command to cumin-admin. This keeps
"add-user" from
> > > changing, and also lets me gate this entire feature by simply including
> > > or excluding the "external-user" command in a release, even if
the
> > > supporting code is present -- it will never be executed since all users
> > > will be internal.
> > >
> > > 3) Changed the split character on authmech to "," to be
consistent with
> > > other lists cumin parses.
> > >
> > > 4) Fixed up cumin-admin "list-users" to flag external users in
the
> > > output.
> > >
> > > 5) Fixed up cumin-admin import-users to no longer complain about
> > > zero-lengh password fields, so that export/import works for external
> > > users.
> > >
> > > 6) Fixed formatting of authenticator.py to match the rest of cumin.
> > > Somehow tab indenting snuck in there :)
> > >
> > > 7) Added failure message on Change_Password dialog for an external user.
> > > This could be a little better, it would be nice if it failed immediately
> > > when the link is activated instead of on submit but that's harder.
And,
> > > I suppose we may want to allow Change_Password of external mechanisms
> > > anyway at some point, through LDAP or a script.
> > >
> > > 8) Minor, changed import statements in authenticator.py. The use of
> > > global and import from within a class isn't something cumin does
other
> > > places and doesn't to my knowledge offer a real advantage, so I
changed
> > > it for consistency.
> > >
> > > Thanks again, comments welcome.
> > >
> > > Best regards,
> > >
> > > Trevor
> > >
> > > On Wed, 2011-08-31 at 17:16 +0200, Vladimir Motoska wrote:
> > > > Hi Trevor,
> > > >
> > > > sorry I generated the patch from a wrong environment. I`m on the
road
> > > > now, I will send you the complete patch including those 2 methods
for
> > > > sync on friday when I will be back in the office.
> > > >
> > > > B.R.
> > > >
> > > > Vlado
> > > > --
> > > > Sent from my Android phone with K-9 Mail. Please excuse my brevity.
> > > >
> > > > Trevor McKay <tmckay(a)redhat.com> wrote:
> > > > Hi Vladimir,
> > > >
> > > > I've started looking at the LDAP patch. Thanks again.
> > > >
> > > > In cumin-admin, the ldap-status and ldap-sync commands
appear to be
> > > > stubs. Do you have something to go there? What do you
intend?
> > > >
> > > > Thanks,
> > > >
> > > > Trevor
> > > >
> > > > On Tue, 2011-08-30 at 13:31 +0200, Vladimir Motoska wrote:
> > > > > Hi all,
> > > > >
> > > > > some time ago we briefly talked to Matthew Farrellee
about LDAP authentication.
> > > > >
> > > > > We created a simple patch to Cumin 0.1.4794 which
enables LDAP authentication.
> > > > > The configuration is easy just add "auth" line
to config file. Auth parameter
> > > > > has the following format
> > > > > [backend]=[url]
> > > > >
> > > > > i.e.:
> > > > >
> > > >
auth:ldap=ldap://ldap.domain.local/ou=People,dc=domain,dc=local??sub?(&(|(accessTo=cluster.domain.local)(accessTo=domain.local)(trustmodel=fullaccess))(accountActive=TRUE)(uid=%%s))
> > > > >
> > > > > To enable a user to login with ldap credentials you need
to create the user in
> > > > > cumin db by entering
> > > > > #cumin-admin add-user [database] USER [PASSWORD]
> > > > > database can be internal or external ( internal is the
default auth mechanism )
> > > > >
> > > > > For the future it would be maybe better to enable a http
proxy authentication
> > > > > like apache... This would enable easy integration with
SSO infrastructure or
> > > > > various backends. However we would appreciate your
feedback in this matter
> > > > > since you have more clear vision of further Cumin
development.
> > > > >
> > > > > Best Regards
> > > > >
> > > >
______________________________________________________________
> > > >
> > > > > cumin-users mailing list
> > > > > cumin-users(a)lists.fedorahosted.org
> > > > >
https://fedorahosted.org/mailman/listinfo/cumin-users
> > > >
> > > >
> > > >
> > > >
______________________________________________________________
> > > >
> > > > cumin-users mailing list
> > > > cumin-users(a)lists.fedorahosted.org
> > > >
https://fedorahosted.org/mailman/listinfo/cumin-users
> > > > _______________________________________________
> > > > cumin-users mailing list
> > > > cumin-users(a)lists.fedorahosted.org
> > > >
https://fedorahosted.org/mailman/listinfo/cumin-users
> > >
> >
> > > Index: cumin/python/cumin/config.py
> > > ===================================================================
> > > --- cumin/python/cumin/config.py (revision 4794)
> > > +++ cumin/python/cumin/config.py (working copy)
> > > @@ -36,6 +36,9 @@
> > > param = ConfigParameter(web, "port", int)
> > > param.default = 45672
> > >
> > > + param = ConfigParameter(web, "auth", str)
> > > + param.default = 'internal'
> > > +
> > > param = ConfigParameter(web, "operator-email", str)
> > >
> > > param = ConfigParameter(web, "user", str)
> > > Index: cumin/python/cumin/main.py
> > > ===================================================================
> > > --- cumin/python/cumin/main.py (revision 4794)
> > > +++ cumin/python/cumin/main.py (working copy)
> > > @@ -21,6 +21,7 @@
> > > from user import *
> > > from util import *
> > > from widgets import *
> > > +from authenticator import *
> > >
> > > from wooly import Session
> > > from cumin.stat import PieChartPage
> > > @@ -31,10 +32,12 @@
> > >
> > > class Cumin(Application):
> > > def __init__(self, home, broker_uris, database_dsn,
> > > - host="localhost", port=45672,
persona="default"):
> > > + host="localhost", port=45672,
persona="default",
> > > + authmech=["internal"]):
> > > super(Cumin, self).__init__()
> > >
> > > self.home = home
> > > + self.authmech = authmech
> > >
> > > model_dir = os.path.join(self.home, "model")
> > >
> > > @@ -43,6 +46,7 @@
> > > self.database = CuminDatabase(self, database_dsn)
> > > self.server = CuminServer(self, host, port)
> > > self.admin = CuminAdmin(self)
> > > + self.authenticator = CuminAuthenticator(self)
> > >
> > > self.add_resource_dir(os.path.join(self.home,
"resources-wooly"))
> > > self.add_resource_dir(os.path.join(self.home,
"resources"))
> > > @@ -590,6 +594,7 @@
> > >
> > > def render_cell_content(self, session, record):
> > > created = self.field.get_content(session, record)
> > > + import time
> > > return fmt_duration(time.time() - secs(created))
> > >
> > > def render_text_align(self, session):
> > > Index: cumin/python/cumin/account/widgets.py
> > > ===================================================================
> > > --- cumin/python/cumin/account/widgets.py (revision 4794)
> > > +++ cumin/python/cumin/account/widgets.py (working copy)
> > > @@ -61,7 +61,6 @@
> > > def render_content(self, sessino):
> > > return "Change password"
> > >
> > > -
> > > class LoginPage(HtmlPage):
> > > def __init__(self, app, name):
> > > super(LoginPage, self).__init__(app, name)
> > > @@ -127,15 +126,16 @@
> > > if not user:
> > > self.login_invalid.set(session, True)
> > > return
> > > -
> > > - crypted = user.password
> > > -
> > > - if crypted and crypt(password, crypted) == crypted:
> > > +# hack begin
> > > + authenticated =
self.app.authenticator.authenticate(name,password)
> > > +# crypted = user.password
> > > + if authenticated:
> > > +# if crypted and crypt(password, crypted) == crypted:
> > > # You're in!
> > >
> > > login = LoginSession(self.app, user)
> > >
session.client_session.attributes["login_session"] = login
> > > -
> > > + log.debug(session.client_session)
> > > url = self.page.origin.get(session)
> > >
> > > self.page.redirect.set(session, url)
> > > @@ -178,21 +178,23 @@
> > > self.add_field(self.new1)
> > >
> > > def validate(self, session):
> > > + user =
session.client_session.attributes["login_session"].user
> > > + # In case a different login session for this user has made
> > > + # changes, refresh the user object
> > > + user.load(session.cursor)
> > > + if len(user.password) == 0:
> > > + error = FormError("An external authentication method is
used for "\
> > > + "this user, password change
ignored")
> > > + self.errors.add(session, error)
> > > + return
> > > +
> > > super(ChangePasswordForm, self).validate(session)
> > >
> > > current = self.current.get(session)
> > > new0 = self.new0.get(session)
> > > new1 = self.new1.get(session)
> > >
> > > - user =
session.client_session.attributes["login_session"].user
> > > -
> > > - # In case a different login session for this user has made
> > > - # changes, refresh the user object
> > > -
> > > - user.load(session.cursor)
> > > -
> > > crypted = user.password
> > > -
> > > if crypt_password(current, crypted) != crypted:
> > > error = FormError("The password is incorrect")
> > > self.errors.add(session, error)
> > > Index: cumin/python/cumin/authenticator.py
> > > ===================================================================
> > > --- cumin/python/cumin/authenticator.py (revision 0)
> > > +++ cumin/python/cumin/authenticator.py (revision 0)
> > > @@ -0,0 +1,84 @@
> > > +from util import *
> > > +import ldap
> > > +import ldapurl
> > > +
> > > +log = logging.getLogger("cumin.authenticator")
> > > +
> > > +class CuminAuthenticator(object):
> > > +
> > > + def __init__(self,app):
> > > + self.authenticators=[]
> > > + self.app = app
> > > + log.info("Initializing %s", self)
> > > + for authmech in app.authmech:
> > > + if authmech.startswith("ldap="):
> > > + try:
> > > + log.info("Adding ldap authenticator")
> > > +
self.authenticators.append(CuminAuthenticatorLDAP(authmech))
> > > + except:
> > > + log.error("Cannot make instance of %s, missing
LDAP module.", authmech )
> > > + elif authmech.startswith("script="):
> > > + log.info("Adding script authenticator")
> > > +
self.authenticators.append(CuminAuthenticatorScript(authmech))
> > > + elif authmech.startswith("internal"):
> > > + log.info("Adding internal authenticator.")
> > > + else:
> > > + log.error("Unsupported authenticator type %s",
authmech)
> > > +
> > > + def authenticate(self,username,password):
> > > + log.debug("Calling authenticate")
> > > + cursor = self.app.database.get_read_cursor()
> > > + cls = self.app.model.com_redhat_cumin.User
> > > + user = cls.get_object(cursor, name=username)
> > > + external = len(user.password) == 0
> > > + log.debug("is external user %s" % external)
> > > + if not external:
> > > + return crypt(password, user.password) == user.password
> > > + else:
> > > + for authenticator in self.authenticators:
> > > + log.debug("Trying authenticator %s",
authenticator )
> > > + if authenticator.authenticate(username,password):
> > > + return True
> > > + return False
> > > +
> > > +class CuminAuthenticatorLDAP(object):
> > > +
> > > + def __init__(self,url):
> > > + log.info("Initializing %s", self)
> > > + myurl = url.split("=",1)[1]
> > > + log.debug("Parsing ldap url: %s", myurl)
> > > + try:
> > > + self.url = ldapurl.LDAPUrl(myurl)
> > > + except Exception,e:
> > > + log.error("Cannot parse ldap url %s", e)
> > > +
> > > + def authenticate(self,username,password):
> > > + log.debug("Authenticating against %s", self)
> > > + try:
> > > + conn = ldap.initialize(self.url.urlscheme + "://" +
self.url.hostport )
> > > + except Exception,e:
> > > + log.error("Cannot contact ldap server %s",e)
> > > + return False
> > > +
> > > + if self.url.who and self.url.cred:
> > > + try:
> > > + self.conn.simple_bind_s(self.url.who,self.url.cred)
> > > + except:
> > > + log.error("Cannot authenticate as service
%s",self.url.who)
> > > + return False
> > > + filter = self.url.filterstr % username
> > > + res = conn.search_s(self.url.dn,self.url.scope,filter)
> > > + for dn,ent in res:
> > > + try:
> > > + conn.simple_bind_s(dn,password)
> > > + return True
> > > + except:
> > > + log.info("Unable to authenticate %s " , dn
)
> > > + return False
> > > +
> > > +class CuminAuthenticatorScript(object):
> > > + def __init__(self,commandline):
> > > + self.commandline = commandline
> > > +
> > > + def authenticate(self,username,password):
> > > + log.debug("Executing %s
%s",(self.commandline,username))
> > > Index: cumin/bin/cumin-admin
> > > ===================================================================
> > > --- cumin/bin/cumin-admin (revision 4794)
> > > +++ cumin/bin/cumin-admin (working copy)
> > > @@ -94,6 +94,7 @@
> > > lines.append("User commands:")
> > > lines.append("")
> > > lines.append(" add-user USER [PASSWORD] Add USER")
> > > + lines.append(" external-user USER Add USER with
external authentication")
> > > lines.append(" remove-user USER Remove
USER")
> > > lines.append(" add-assignment USER ROLE Add USER to
ROLE")
> > > lines.append(" remove-assignment USER ROLE Remove USER from
ROLE")
> > > @@ -161,24 +162,43 @@
> > >
> > > def handle_list_users(app, cursor, opts, args):
> > >
> > > - print " ID Name Roles"
> > > - print "---- -------------------- --------------------"
> > > + print " ID Name Roles"
> > > + print "---- -------------------- --------------------"
> > >
> > > users, roles_by_user_id = get_users(app, cursor)
> > > -
> > > + externals = 0
> > > for user in users:
> > > try:
> > > roles = ", ".join(roles_by_user_id[user._id])
> > > except KeyError:
> > > roles = ""
> > > + if len(user.password) == 0:
> > > + ex = "*"
> > > + externals += 1
> > > + else:
> > > + ex = " "
> > > + print "%4i %s %-20s %-20s" % (user._id, ex, user.name,
roles)
> > >
> > > - print "%4i %-20s %-20s" % (user._id, user.name, roles)
> > > -
> > > count = len(users)
> > >
> > > print
> > > print "(%i user%s found)" % (count, ess(count))
> > > + if externals > 0:
> > > + print "(%i external user%s, indicated by *)" %
(externals, ess(externals))
> > >
> > > +def handle_external_user(app, cursor, opts, args):
> > > + try:
> > > + name = args[0]
> > > + except IndexError:
> > > + error("USER is required")
> > > + role = app.admin.get_role(cursor, "user")
> > > + try:
> > > + user = app.admin.add_user(cursor, name, "")
> > > + except IntegrityError:
> > > + error("A user called '%s' already exists" %
name)
> > > + app.admin.add_assignment(cursor, user, role)
> > > + print "External user '%s' is added" % name
> > > +
> > > def handle_add_user(app, cursor, opts, args):
> > > try:
> > > name = args[0]
> > > @@ -189,18 +209,14 @@
> > > password = args[1]
> > > except IndexError:
> > > password = prompt_password()
> > > -
> > > crypted = crypt_password(password)
> > >
> > > role = app.admin.get_role(cursor, "user")
> > > -
> > > try:
> > > user = app.admin.add_user(cursor, name, crypted)
> > > except IntegrityError:
> > > error("A user called '%s' already exists" %
name)
> > > -
> > > app.admin.add_assignment(cursor, user, role)
> > > -
> > > print "User '%s' is added" % name
> > >
> > > def handle_export_users(app, cursor, opts, args):
> > > @@ -240,15 +256,19 @@
> > > for info in user_data:
> > > line += 1
> > >
> > > + count = 0
> > > for field in info:
> > > # Don't suppose it's possible for data to be anything
other than
> > > # strings from csv, but this is where such type checking
goes
> > > if type(field) != str:
> > > error("Data error, line %u, fields must be
strings" % line)
> > > - if len(field) == 0:
> > > - error("Data error, line %u, importer "\
> > > - "does not allow empty fields" %
line)
> > >
> > > + # Empty password field is external auth indicator, allow it
> > > + if len(field) == 0 and count != 1:
> > > + error("Data error, line %u, field %u, importer
"\
> > > + "does not allow empty field" % (line,
field+1))
> > > + count += 1
> > > +
> > > if len(info) < 2:
> > > error("Data error, line %u, not enough fields. "\
> > > " User and password are required" % (line))
> > > @@ -263,7 +283,7 @@
> > > # before we get an integrity error
> > > warn("A user called '%s' already exists,
skipping" % info[0])
> > > continue
> > > -
> > > +
> > > try:
> > > user = app.admin.add_user(cursor, info[0], info[1])
> > > except IntegrityError:
> > > @@ -284,10 +304,12 @@
> > > error("USER is required")
> > >
> > > user = app.admin.get_user(cursor, name)
> > > - user.delete(cursor)
> > > + if user is None:
> > > + print "No such user '%s'" % name
> > > + else:
> > > + user.delete(cursor)
> > > + print "User '%s' is removed" % name
> > >
> > > - print "User '%s' is removed" % name
> > > -
> > > def handle_list_roles(app, cursor, opts, args):
> > > cls = app.model.com_redhat_cumin.Role
> > > roles = cls.get_selection(cursor)
> > > Index: cumin/bin/cumin-web
> > > ===================================================================
> > > --- cumin/bin/cumin-web (revision 4794)
> > > +++ cumin/bin/cumin-web (working copy)
> > > @@ -81,9 +81,11 @@
> > > values.log_max_archives)
> > >
> > > broker_uris = [x.strip() for x in
opts.brokers.split(",")]
> > > + authmech = [ x.strip() for x in
values.auth.split(",")]
> > >
> > > cumin = Cumin(config.get_home(), broker_uris, opts.database,
> > > - opts.host, opts.port, values.persona)
> > > + opts.host, opts.port, values.persona,
> > > + authmech)
> > >
> > > if type(values.sasl_mech_list) == str:
> > > cumin.sasl_mech_list = values.sasl_mech_list.upper()
> >
> > > _______________________________________________
> > > cumin-users mailing list
> > > cumin-users(a)lists.fedorahosted.org
> > >
https://fedorahosted.org/mailman/listinfo/cumin-users
> >
> >
>
>
> _______________________________________________
> cumin-users mailing list
> cumin-users(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/cumin-users
_______________________________________________
cumin-users mailing list
cumin-users(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/cumin-users