Issue Details (XML | Word | Printable)

Key: SERVLET_SPEC-14
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Shing Wai Chan
Reporter: markt_asf
Votes: 1
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
servlet-spec

Require FORM auth to issue 303 redirects

Created: 04/Oct/11 04:26 PM   Updated: 01/Mar/13 12:37 AM   Resolved: 01/Mar/13 12:37 AM
Component/s: None
Affects Version/s: None
Fix Version/s: None

Time Tracking:
Not Specified

Tags:
Participants: markt_asf, monzillo and Shing Wai Chan


 Description  « Hide

See https://issues.apache.org/bugzilla/show_bug.cgi?id=49779 for an example of why this would be useful.



Rajiv Mordani made changes - 06/Dec/11 11:04 PM
Field Original Value New Value
Assignee Shing Wai Chan [ swchan2 ]
monzillo added a comment - 10/Jan/13 11:11 PM

I downloaded the app, ported it, and ran it on Glassfish, and did not see the reported problem.
That said, constraint processing and FBL are areas where glassfish and tomcat differ in their implementations.

In any event, I took a closer look at FBL, and I can see how there is a problem (with FBL) which likely is independent of implementation.

As I understand FBL it works generally as follows (leaving out some failure cases like failure of password validation).

1. a request is made to a resource requiring authentication.

2. the container calls the authentication system which saves the request, and the login page is returned to the user.

3. the user fills in the login page and submits its contents in a POST request to j_security_check.

4. the container routes j_security_check to the authentication system which validates the username and password obtained from the post request, and then restores the uri of the saved request and then redirects the post request to the restored uri.

5. the browser repeats the restored request to the restored resource url (the http verb used by the browser depends on the verb used in step 4)

6. if the container determines that the repeated request requires authetication, it calls the authentication system; in which case, the authentication system notices that it has already authenticated this request, so it binds the principal to the request, and restores the details from the original saved request (including the http verb)

7. In either case, the container determines if the called principal (which may be null if the authentication system was skipped in step 6) is authorized to access the resource using verb of the request (which depends on both the way the redirect was done in step and whether the verb used by the browser in step 6, required authentication).

if the redirect of step 4 is done with a 302, then the restored request should be repeated (by the browser with POST, since it is a redirect in response to the POST of the login form. If the original request was done with a verb other than POST, then there is always the possibility that that verb required authentication, and POST does not; in which case the authenticator would not be called in step 6, the verb would not be restored, the caller principal would not be set, and the access check would presumably pass, although the wrong method of the resource would likely be called.

Conversely if the redirect of step 4 is done with a 303, then the restored request should be repeated with a GET; which can result in similar problems if the original request was done with a verb other than GET, and that verb requires authentication while GET does not.

I am not sure how to completely resolve this, but I think the following are true. a 303 redirect should be used when the original request came in with GET. This will ensure that the original request and the repeat following redirect are identical in terms or checked url and http verb.
a 302 redirect should be used when the original request cam in with post. If the request came in on any verb other than GET or POST, then the authenticator should fail if neither GET or POST have been configured to require authentication (at the request pattern). Otherwise 302 or 330 should be used depending on whether POST or GET require authentication.

If the dialog is short-circuited, for example by starting with a GET of the login page, or of a GET or POST to j_security_check, then I believe the convention is to redirect the request to the context root of the app, in which case, either redirection code can likely be accomodated.


markt_asf added a comment - 12/Jan/13 08:11 PM

I agree with all of the analysis above with a few minor tweaks:

I'd used 307 rather than 302 for HTTP/1.1 clients to be unambiguous.

I wonder if the following option is viable:

  • Always use 303 redirects
  • Always pass GET requests through the FORM authenticator
  • If the resource is protected for GET - proceed as currently
  • If the resource is not protected for GET, check if there is a saved request to be restored
  • If there is, restore it along with the original verb
  • If not, carry on

monzillo added a comment - 14/Jan/13 08:31 PM

Yes, I think so...but as i'm sure you know, the container runtime, which in our cases likely includes some variant of AuthenticatorBase, mostly just decides when to call the configured authenticator (based on evaluating constraints), while leaving it up to the authenticator to know how to deal with details/problems like the one we are discussing.

In this case, we are basically saying that (at least) one specific authenticator can't function properly unless it is called on every request (for which an caller identity has not yet been established), and that (when the runtime calls the authenticator on each request) the runtime will either need to pass the authenticator more info (describing the nature of constraints that apply to the call), or the authenticator will need to perform the constraint determination that would otherwise have been done by the runtime.

Since the runtime currently handles all authenticators in (basically) the same way, such a change might need to effect the implementation of all the existing authenticators, or at least we would need to establish a way for the runtime to be able to ask the authenticator if it must be called on every request. Since all the authenticators extend BasicAuthenticator, we could probably do this by adding a method to BasicAuthenticator, that could be overriden by only those authenticators that need to be called independent of constraint.

Aslo, and fwiw, the Servlet Profile of jsr 196 requires; that the configured auth context be called on every request, and the context of the request informs the auth module, as to whether or not the current request is subject to an auth contraint.


Shing Wai Chan added a comment - 01/Mar/13 12:37 AM

The spec has been updated to use 303 appropriately for HTTP/1.1.
See section 13.6.3 for Servlet 3.1 PFD.


Shing Wai Chan made changes - 01/Mar/13 12:37 AM
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]