Hide Forgot
Under certain circumstances (still have not figured out exactly what they are) /login can cause an infinite redirect loop. It happens when the user is authenticated but is trying to access something for which they do not have permissions (for example, hitting /labcontrollers/ when not in the 'admin' group). rmancy can reproduce this reliably on his browser but I cannot. I need to investigate further.
The problem here stems from the turbogears.identity.was_login_attempted() function, which does not always give a sensible result. In particular, when Apache is doing basic auth on /login (as in our production deployment if the user does not have a krb ticket) the presence of the Authorization header results in set_login_attempted(True) on every request -- even if the user did not actually type anything into a dialog box (turbogears/identity/visitor.py:124). The /login controller method checks three things when deciding whether to redirect (bkr/server/controllers.py:2028): if not identity.current.anonymous \ and identity.was_login_attempted() \ and not identity.get_identity_errors(): redirect(forward_url, redirect_params=kwargs) The assumption here is that if the user has just typed in their credentials and been successfully authenticated, then they get sent back to the URL they were requesting. But when using basic auth, identity.was_login_attempted() is always true and hence the infinite redirect loop. (The difference between rmancy's browser and mine is that I had a krb ticket whereas rmancy had filled in the basic auth prompt.)
The reason for the complicated conditional in /login is that the user may hit it under two different circumstances: either they are not logged in (i.e. identity.current.anonymous == True), or they are logged in but they lack permissions to access something (such as an admin-only controller). I think the best solution would be to use a different controller method in the latter case (logged in but insufficient permissions), something like /forbidden. It could show an explanatory message, without providing a login form. We could add some logic in our identity_failure_url function (bkr/server/controllers.py:81) to redirect to either /login or /forbidden as appropriate. Then the /login controller method only needs to handle one case: the user is not logged in, but they want to be. The conditional then becomes simply: if the user is logged in correctly, then redirect them away. This should eliminate the infinite redirect loop, and also maintain the existing behaviour of /login.
On second thought, I really hate it when a webapp redirects me to some other URL only to give an error. If the user has no permissions to see something, we should just return a 403 response code and "Forbidden" body for that request, instead of redirecting them away to another URL which just says "Forbidden". It's almost as bad as those annoying web sites which redirect you to /notfound instead of just giving a 404. However, I'm not sure how easy it would be to make TurboGears do the "right" (in the eyes of HTTP) thing in this case...