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