Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1985 (closed defect: fixed)

Opened 10 years ago

Last modified 9 years ago

Identity failure status getting overriden by CP3

Reported by: kskuhlman Owned by: kskuhlman
Priority: high Milestone: 1.5
Component: TurboGears Version: 1.5 HEAD
Severity: blocker Keywords:
Cc:

Description

In CherryPy 3, CP exceptions set the response status code. This is a problem with identity.exceptions.IdentityFailure, which is trying to do a HTTPRedirect while setting the status to 401.

The current behavior is that CP is resetting the status code & returning a 200. Since CP 3 only allows 3XX statuses to be set for redirects, the solution is not as simple as raising HTTPRedirect (which allows a status parm) instead of InternalRedirect (which doesn't).

There are 10 tests in identity.tests.test_identity that have FIXMEs attached to them for this issue.

Change History

comment:1 Changed 10 years ago by kskuhlman

  • Status changed from new to assigned
  • Owner changed from anonymous to kskuhlman

Looks like we can store the status and then retrieve it back in a hook.

Going ahead and assigning ticket to myself now that I have an idea of how to solve it.

comment:2 Changed 10 years ago by kskuhlman

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed in r5539

comment:3 Changed 9 years ago by kskuhlman

  • Status changed from closed to reopened
  • Resolution fixed deleted

I'm having problems merging r6665, r6671, r6672, and r6717 because of identity status code issues. I think this ticket/commit is the source of the problem.

In hindsite, hijacking the wsgi environment just because it persists a redirect was not the best idea.

A possible option would be to monkey-patch InternalRedirect so that it allows arbitrary status codes.

Re-opening.

comment:4 Changed 9 years ago by kskuhlman

The other possibility here is to use cherrypy.tools.basic_auth instead of our own implementation. This would require deeper refactoring of the patches mentioned above, but would be cleaner.

comment:5 Changed 9 years ago by kskuhlman

Ported r6665 in r6920, using same wsgi environ trick as before.

comment:6 Changed 9 years ago by kskuhlman

Merged r6671 in r6921. NOTE: This rev only includes tests, and one of the json identity ones is currently failing.

comment:7 Changed 9 years ago by kskuhlman

  • Status changed from reopened to closed
  • Resolution set to fixed

Using the wsgi environ for identity redirects turned out to not be as messy as I'd feared. r6920 was pretty trivial.

The failing json identity test was fixed by chrisz in r6939 & was due to an unrelated change in CP.

Note: See TracTickets for help on using tickets.