Defect in commit_updater permission checks when using ignore_auth_case

[ilewismsl]

On our system we have

ignore_auth_case = true

In general, this works without issue, but recently many users have seen the ticket.commit_updater fail, while it used to work for them. But, for me it has been working and I have been unable to see the problem under my user until today.

I have just isolated the difference between the times that worked for me and the failures I had today. My user on the system is ian. And, in the test commit messages that worked that is how the change set sees my users name. On the other hand, change sets by the users who have seen the commit_updater fail, show their user names in all upper case.

Today, for reasons I do not yet understand, and probably having something to do with Subversion, the changesets see me as IAN. And, for those change sets, and only those change sets, the commit_updater fails, just as it does for the other users who reported the failure to me.

Extract of relevant lines from the log where my commits failed to update a ticket (I do not have logs for other failures, but I would bet they would look the same):

  2014-02-14 20:47:22,622 Trac[commit_updater] DEBUG: Updating ticket #1113

  2014-02-14 20:47:22,622 Trac[perm] DEBUG: No policy allowed IAN performing TICKET_APPEND on <Resource u'ticket:1113'>
  2014-02-14 20:47:22,622 Trac[commit_updater] INFO: IAN doesn't have TICKET_APPEND permission for #1113

To me, this sounds like a defect in the authorization on change sets from the repository when ignore_auth_case = true. The system should be authorizing ignoring case, but it clearly is not doing that. Should I log a defect ticket on Trac?

Ian Lewis

www.mstarlabs.com

Package	Version
Trac	1.0.1
Docutils	0.11
Genshi	0.6 (without speedups)
mod_python	3.3.1
Pygments	1.4
pysqlite	2.4.1
Python	2.6.6 (r266:84297, Aug 24 2010, 18:46:32) [MSC v.1500 32 bit (Intel)]
setuptools	1.4.1
SQLite	3.5.9
Subversion	1.8.5 (r1542147)
jQuery	1.7.2

[Olemis Lang]

I'll take a look to try to identify what might be wrong with it . in #11440 some test cases have been proposed for commit_updater component . Those should be included in forthcoming Trac=1.0.2 , so I guess a few more testing this scenario might be worth the effort as well .

Thanks for reporting the issue ...

--
You received this message because you are subscribed to the Google Groups "Trac Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to trac-dev+u...@googlegroups.com.
To post to this group, send email to trac...@googlegroups.com.
Visit this group at http://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/groups/opt_out.

--
Regards,

Olemis - @olemislc

Apache™ Bloodhound contributor
http://issues.apache.org/bloodhound
http://blood-hound.net

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

[ilewismsl]

Thank you, Olemis. Let me know if there is anything we should try testing here. - Ian

[RjOllos]

On Friday, February 14, 2014 9:29:26 PM UTC-8, ilewismsl wrote:

On our system we have

ignore_auth_case = true

In general, this works without issue, but recently many users have seen the ticket.commit_updater fail, while it used to work for them. But, for me it has been working and I have been unable to see the problem under my user until today.

I have just isolated the difference between the times that worked for me and the failures I had today. My user on the system is ian. And, in the test commit messages that worked that is how the change set sees my users name. On the other hand, change sets by the users who have seen the commit_updater fail, show their user names in all upper case.

Today, for reasons I do not yet understand, and probably having something to do with Subversion, the changesets see me as IAN. And, for those change sets, and only those change sets, the commit_updater fails, just as it does for the other users who reported the failure to me.

Extract of relevant lines from the log where my commits failed to update a ticket (I do not have logs for other failures, but I would bet they would look the same):

  2014-02-14 20:47:22,622 Trac[commit_updater] DEBUG: Updating ticket #1113

  2014-02-14 20:47:22,622 Trac[perm] DEBUG: No policy allowed IAN performing TICKET_APPEND on <Resource u'ticket:1113'>
  2014-02-14 20:47:22,622 Trac[commit_updater] INFO: IAN doesn't have TICKET_APPEND permission for #1113

To me, this sounds like a defect in the authorization on change sets from the repository when ignore_auth_case = true. The system should be authorizing ignoring case, but it clearly is not doing that. Should I log a defect ticket on Trac?

Which protocol do you use to interact with Subversion?: HTTP/S, SVN over SSH, ...

How is the username displayed for the rev associated with the log message you show, when inspected through the SVN command line client? Is the username in uppercase or lowercase?

[RjOllos]

My previous comment was based on my understanding that you don't know why the Subversion username is appearing in all uppercase characters since you have authenticated using a username with lowercase characters. Presumably that is a Subversion issue, as you've suggested. It would be nice to know what the command line client displays for the changeset metadata though.

You are also wondering why the [trac] ignore_auth_case parameter is not being respected in CommitTicketUpdater. 

The effect of the ignore_auth_case parameter is to convert a username to lowercase when a Request object is constructed. Usually `PermissionCache` is constructed using `req.authname` for the `username` parameter: `PermissionCache(self.env, req.authname)`, and that PermissionCache is used to grant/deny authorization. In `CommitTicketUpdater` we aren't working with a `Request` object, but to get the same effect we can convert the author parameter to lowercase here:

http://trac.edgewall.org/browser/tags/trac-1.0.1/tracopt/ticket/commit_updater.py?marks=211#L209

However, that would leave the author of the ticket comment in its case-sensitive form.

To work around that, we could replace the author of the Changeset object with its lower case form, but it should also be realized that the case-sensitivity of the username will be reflected anywhere the Subversion changeset username is displayed, for example on the Repository Browser page:

Side note: This relates back to my question about what you see in the SVN changeset messages. Do you see the username in uppercase characters when viewing the changeset messages through your SVN client, and when viewing those revisions in the repository browser?

If we wish to optionally treat all usernames from the SVN repository as lowercase, that issue would need to be dealt with in the Subversion FS wrapper:

http://trac.edgewall.org/browser/tags/trac-1.0.1/tracopt/versioncontrol/svn/svn_fs.py?marks=929#L918 

We could have an option to coerce all author names coming from Subversion changesets to lower case (effectively treating those usernames the same as ignore_auth_case). I think that would probably give us consistent behavior throughout Trac, but I'd like to get some more feedback from the Trac devs on that proposed change. 

An alternative is that you could just make your Subversion authorization mechanism to be case-sensitive, but I guess we need to get to the bottom of why that username is unexpectedly appearing in all uppercase.

[ilewismsl]

I had not looked into the issue of why SVN sees user IDs as upper case very much. I suspect that at least some of the story is that some people type in their user name in all upper case. However, that is not the whole story. I never enter my user name in all upper case. I only use 'ian'. A brief look around indicated that this was going to be hard for me to find. My user ID seems mostly to now be lower case again, unlike when I first reported this issue, though a number of other users largely have upper case user IDs showing in Subversion. However, I just looked at a small repository we have and several recent check ins show me as IAN. I have not logged into the SVN client on my machine for a long time and my cached credentials have not changes from ian@<domain>.

The reason I ignored this issue is that our Windows authorization is case insensitive. So, it does not matter a lot. And I suspect that the ultimate culprit on the user names changing is something in Windows. over which I have no control even if I could isolate it. That is a lot of why we have ignore_auth_case set. We have no good way to make users log in with case sensitive user name. Windows will not enforce it, and worse yet, sometimes Windows decides it would be a good idea to change the case provided (I do not know when it does this, but I am quite certain that it does). So, even if I could work out what is going on exactly, I would have a hard time getting users to log in consistently whether I like it or not and I doubt very much I could do anything about Windows. I could coerce the user names to lower case somewhere in Apache, but I looked at that when I first encountered the issue and I do not understand rewrites well enough to trust myself to mess with user IDs reliably.

I did just try looking at the revision logs on our small repository using svn log and what Trac shows in its timeline matches the user name case that svn log shows. So, Trac is getting correct user names as seen by Subversion.

To me, it seems that whether you log into Trac through your Subversion user name, you connect directly, or log in some other way, the authorization should follow the same rules, and preferably go through the same code to reach that goal. Of course, as you say, this is a Trac dev decision.

We already live with all user names showing up as all lower case in Trac. And, personally, I prefer the consistent user name case given that we have no easy option to get case sensitive user names. It would cause us no trouble if the same occurred under Subversion logs since our Subversion authorization is also case insensitive. But, I can easily imagine that someone else could be very unhappy with a timeline report that did not match what svn log reports. But, it also seems likely to me that such a person would never set  ignore_auth_case true.

I would personally prefer that the Subversion logs and Repository browser show the actual user name as seen in Subverison and that that the Trac authorizations use all lower case in all places. That lets me see what Subversion sees and it also means that Trac has consistent user names. Trac does not look at ignore_auth_case when deciding who made a change, at least as far as I can tell, so it sees a change by IAN as different from a change by ian. If all log ons always used ignore_auth_case rules, then that could not happen.

[Ethan Jucovy]

On Tue, Mar 18, 2014 at 8:12 PM, RjOllos <rjo...@gmail.com> wrote:

If we wish to optionally treat all usernames from the SVN repository as lowercase, that issue would need to be dealt with in the Subversion FS wrapper:

http://trac.edgewall.org/browser/tags/trac-1.0.1/tracopt/versioncontrol/svn/svn_fs.py?marks=929#L918 

We could have an option to coerce all author names coming from Subversion changesets to lower case (effectively treating those usernames the same as ignore_auth_case). I think that would probably give us consistent behavior throughout Trac, but I'd like to get some more feedback from the Trac devs on that proposed change.

As an alternative to a new configuration option, I think this could also be addressed by the "unified" changeset author-mapping patch I've proposed in http://trac.edgewall.org/ticket/10640#comment:7 

If the specific subversion username(s) that need lowercasing are known, the patch could be used as-is to translate those names ("IAN = ian").  Or the usernames coming from Subversion could all be lowercased by plugging in a custom IChangesetUserMapper component for the installation.

Sorry for the shameless plug :-)

-Ethan

[ilewismsl]

That looks like a quite useful feature, and it would let me work around the problem we have. But, I still think all Trac user authorization should honor the setting of ignore_auth_case wherever the user name comes from. - Ian

[RjOllos]

On Friday, March 21, 2014 5:02:00 AM UTC-7, Ethan Jucovy wrote:

A patch for the authorization issue has been proposed:

http://trac.edgewall.org/ticket/11559

In the future I'll take a look at #10640, towards addressing the usernames that are displayed from the repository.

[RjOllos]

The issue is fixed on the 1.0-stable branch now. 

[ilewismsl]

Thank you. I look forward to the 1.02 release so I can put it into service.