-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mixed case username auth not working anymore #56
Comments
So as far as I understand the issue, |
As I said, I really don't know much about PHP programming, I'm only seeing this as a change to how it used to work. We actually have both 2.3.3 and 2.4 running side-by-side at the moment, and they behave differently. I'm not clear what you mean by "set an env variable with the same value as with AuthzForceUsernameCase". /J |
If set in httpd.conf, there won't be a need for a config option in config.php. Alternatively, you could modify |
I have not evaluated changing httpd.conf, basically because I think this is application specific, not apache specific. I've thought about changing this in the setUsername() function, but would like it to be configurable in the config.php to avoid future work when new WebSVN updates are installed. For now I will tell my users to login with lowercase names as that is the most safe route at the moment. |
Maybe it's clear for others, it wasn't for me, so feel free to ignore the following or correct me if I'm wrong:
That setting was not used by WebSVN itself, correct? It was somewhere in the httpd configuration files itself, because like @michael-o linked already, it seems to be only used by
I have the feeling that lower-case was simply used at all/some(?) places because some of those needed to be lower anyway to work at all because of the mentioned limitations of the parser. WebSVN did lower-case only as well, while In my opinion, what is done now is the correct behaviour, case should be significant by default if SVN decides so, but it's incompatible with former releases as well. The problem I see now is that this isn't only incompatible for usernames, but repo names and paths as well. So, should both incompatibilities be addressed? Or only usernames and if so, how? In my opinion again we should only care about usernames if at all, because Let's compare how complex it is to fix this using httpd, like @michael-o mentioned. For URIs:
https://stackoverflow.com/a/8308541/2055163 Which doesn't work if browsers ask for usernames, in such a case it seems one would need to rewrite the environment variable directly?
https://stackoverflow.com/a/18476427/2055163 Doesn't look that wrong, but probably more difficult than to only change one setting in WebSVN. OTOH, the same approach could be used for usernames, paths, repo names etc. as well, everything that is now different than before and we might simply not want to change anymore. Thoughts? |
I would not touch repo and path names because they are case sensitive. URIs are always case-sensitive. This looks fine to me:
I still think that this is an implemenation detail performed by the subversion module and it should be done by the httpd.conf as well. We could allow a |
I agree that repo names and path names should not be touched, that is up to the server to handle. We authenticate our users against an OpenLDAP server, and the users can login with User, uSer, user, USER ... which means that authz must cover all these combinations. Unless we use "AuthzForceUsernameCase Lower" to only have to write each user once. In my view WebSVN is kind of a frontend to subversion and should preferably work the same way. /J |
Interesting point, but can be handled easily: It didn't work that way before and therefore won't be introduced newly. ;-) If at all, this issue is about re-introducing reasonable backwards compatibility for usernames. Your argument seems to be a good point for "forcing" admins to deal with those specifics on the level of the webserver anyway, because they have full control for all requests there.
Not that I'm voting against this, but we should keep in mind that there's more than Apache httpd out there. Everyone would need to reimplement at least the username thing, which doesn't seem that uncommon according to my quick research. |
Ideally I'd like to see an an additional option in the config.php parts, e.g. or something similar. That way each repo can be controlled separately. |
@jmwf This could work. Though still ugly, because I think that username normalization is a auth provider issue. I see the directory in the module as a compromise too. @ams-tschoening WDYT? Is that a way we can go? |
One problem I think is that WebSVN can use URI in the form "file://" to access subversion repositories for performance, and in this case subversion doesn't handle authz at all (which I think is the reason for the $config->useAccessFile in the first place). The authz file is only relevant if served through svnserve, apache etc. /J |
@jmwf I don't get your last point, WebSVN internally uses @michael-o I think adding it there makes sense, but we need to be careful about the syntax, because an additional optional argument to provide a repo name is already supported:
I'm using that myself as well. So we would at least need something like:
Or introduce some hash-based settings? Not sure if it works this way in PHP:
All functions processing these calls could easily forward "lowercase" to authz.php and that could recognize it. |
It probably misunderstood how WebSVN accesses the repositories :-( /J |
First off, thanks you for updating this great tool!
We have been using WebSVN for a number of years and stayed happily with 2.3.3 until last week.
The new version (2.4) works great in supporting downloads and comparison in a PHP 7+ environment, but one thing that has changed is the way that WebSVN connects to subversion.
In our apache/subversion setup we use
AuthzForceUsernameCase Lower
The old WebSVN (2.3.3) worked very well with this setting, but WebSVN 2.4 require users to login with their username in lowercase to match the configuration used in the subversion authz-file. While it is not really a big deal, it still probably affects a lot of installations.
I don't know php-programming, but comparing the old auth.php and the new authz.php indicates that the old 2.3.3 version used strtolower() on the user name, but the new version does not?
My suggestion would be to add a new configuration parameter to let the user decide if WebSVN should force lowercase/uppercase or not.
/J
The text was updated successfully, but these errors were encountered: