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
"
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