On Wed, 12 Mar 2008, Kamesh Jayachandran wrote:
...
> >This sounds fairly reasonable to me. One big question I have is how
> >usernames
> >are then recorded in the history (e.g. uppercase, lowercase, or as entered
> >by the user?).
>
> Usernames are recorded as provided by the user. I did not want to change
> the request_rec->user.
Yeah, I'm a bit leary of that too. It would be nice to have some record of
what username the authentication occurred against, though. And I can't think
of anywhere better than the history to record that (since it provides an
accurrate audit trail). I guess I still favor changing it; it'd be nice to
hear from Justin or Sander or someone else more familiar with the ramifications
of doing something like that.
...
> >Ditto for the name of the directive, and even more so for the help (which
> >should mention that the case is applied to authz).
> >
> >
>
> Changed the directive to 'AuthzForceUsernameCase'.
+1
> >> (convert_to_uppercase_string,
> >> convert_to_lowercase_string,
> >> get_username_to_authorize): New functions.
> >>
> >
> >Do we really need both of those conversion functions? Seems like one
> >should be sufficient.
>
> Yes fixed that too.
I have some simplifications for that (attached, untested).
...
> >This patch doesn't match our coding conventions, but seems to comply with
> >the code in mod_authz_svn...
>
> Will fix mod_authz_svn itself to match our coding guidelines in
> subsequent commit.
Okay.
> >Yeah, these are really redundant; only the apr_toupper()/apr_tolower()
> >calls
> >differ. We definitely want a single function here.
> >
> >
>
> Fixed.
> >> if (conf->usernamecase) {
> >>+ username_to_authorize = apr_pstrdup(r->pool, r->user);
> >>+ if (strcasecmp(conf->usernamecase, "upper") == 0)
> >>+ convert_to_uppercase_string(username_to_authorize);
> >>+ else
> >>+ convert_to_lowercase_string(username_to_authorize);
> >>
> >
> >Ya know, I'd probably just inline the code here.
>
> Fixed.
...
> >If we don't change r->user, I think the history will take the username as
> >specified by the end-user. This means that the history will be potentially
> >subject to several capitalizations of the same name.
>
> Yes, you are correct. Can each module change the request_rec->user like
> this?
>
> Committed with above fixes in r29875
- text/plain attachment: patch
- application/pgp-signature attachment: stored
Received on 2008-03-12 19:08:56 CET