Hi Chris/Valerie Can you take a review on a related bug. I found it when I wrote the test for the previous one.
6829283: HTTP/Negotiate: Authenticator triggered again when user cancels the first one http://cr.openjdk.java.net/~weijun/6829283/webrev.00/ Basically, it's because for HTTP/Negotiate, it's ... -> Callback -> Authenticator We have 2 callbacks (user and pass) in JAAS, but there's only 1 Authenticator (doing user and pass at the same time). If user cancels the first call, we shouldn't bother her again. Thanks Max Max Wang (Weijun) wrote: > Hi Chris > > A new webrev is created at > http://cr.openjdk.java.net/~weijun/6578647/webrev.01 > > Now all HttpCallerInfo creations are inline, so the diff is much > clearer. There's one place I didn't call toLowerCase(), the call is > moved into NegotiatorImpl right before the service principal name is > created. > > I also add a test, putting two Kerberos KDC, one HTTP server, one proxy > server in a single regression test is fun! > > Thanks > Mx > > On Apr 14, 2009, at 8:55 PM, Max (Weijun) Wang wrote: > >> >> On Apr 14, 2009, at 5:59 PM, Christopher Hegarty - Sun Microsystems >> Ireland wrote: >> >>> Hi Max, >>> >>> I only looked at the networking part of the changes. They look fine, >>> I just have a few questions/comments: >>> >>> 1) sun.net.www.protocol.http.HttpURLConnection >>> Can you use the same HttpCallerInfo instance for proxy authentication >>> at line 1108? This instance has been created using the single arg >>> constructor therefore it is has authType = RequestorType.SERVER, >>> right? >> >> Yes, you're right. Will update tomorrow. >> >>> >>> 2) sun.net.www.protocol.http.HttpCallerInfo >>> It is just my preference, but I would prefer to see all the fields of >>> HttpCallerInfo private and have simple accessors: >>> private final String host; >>> ...... >>> >>> public String host() { >>> return host; >>> } >>> ...... >> >> Your suggestion is more formal. But I think making all fields final is >> also sufficient to make it immutable. >> >>> >>> 3) Are the changes to use HttpCallerInfo in AuthenticationHeader, >>> HttpURLConnection, NegotiateAuthentication and NegotiatorImpl >>> strictly necessary? They seem to be changed just for consistency of >>> using the new class. I only see that NegotiateCallbackHandler is >>> required to use this new class on the networking side. >> >> There needs a way to transfer these info into the JGSS underneath (so >> that NegotiateCallbackHandler has a chance to know them), and the only >> bridge is inside NegotiatorImpl. I don't know if there's a better way >> to do this. The HttpClient class seems having similar info but >> sometimes it's null and I don't know why. Sorry if I reinvent a >> wheel-cart to carry these info. >> >> Thanks >> Max >> >>> >>> This is not a problem just a question to see if I understand >>> correctly the changes. >>> >>> -Chris. >>> >>> >>> >>> >>> >>> On 04/13/09 03:27, Weijun Wang wrote: >>>> Hi Valerie and Networking guys >>>> Please take a review at this bug fix: >>>> http://cr.openjdk.java.net/~weijun/6578647/webrev.00/ >>>> The bug is >>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6578647 >>>> The bug report says that no URL-related info is available in >>>> Authenticator when using HTTP/Negotiate. The reason is that in the long >>>> stack of >>>> HTTP/Negotiate -> JGSS -> JAAS -> Krb5LoginModule >>>> -> Callback -> Authenticator >>>> The URL info is lost. In order to support special actions for >>>> HTTP/Negotiate calls in JGSS (say, using Authenticator instead of >>>> text-based callback, honor the OK-AS-DELEGATE flag...), we already used >>>> an integer field (caller) to tell the codes deep below who initiates >>>> the >>>> JGSS calls. It seems an integer is not enough to carry too much >>>> information. (oh, I love the C void*) >>>> The fix is simple: change the caller from integer to a Java class: >>>> GSSCaller, which includes as much as info it likes. For HTTP/Negotiate, >>>> a child class HttpCaller, encapsulates all info an Authenticator needs. >>>> The fix includes three parts: >>>> 1. Three new classes: >>>> sun.sec.jgss.GSSCaller: >>>> the new caller >>>> sun.sec.jgss.HttpCaller: >>>> a child of GSSCaller, knows everything about HTTP >>>> sun.net.www.protocol.http.HttpCallerInfo: >>>> the info GSSCaller knows, this class is created on the >>>> network side so that no sun.security.jgss.* codes are >>>> dragged into the bootstrap building process. >>>> 2. On the network side: >>>> Refactoring HTTP codes in sun.net.www.protocol.http.* to fill info >>>> into the HttpCallerInfo class. >>>> 3. On the JGSS side: >>>> Multiple changes in sun.security.jgss.* classes. *All* the >>>> code changes are simply s/int/GSSCaller/g changes. >>>> I also moved the pre-defined callers from GSSUtil to >>>> GSSCaller. >>>> Thanks >>>> Max >> >
