Refactor HttpNegotiate exposed status codes

Refactor HttpNegotiate exposed status codes

BUG=552414

[dgn, 2015-11-13 16:34:03]

dgn@chromium.org changed reviewers:
+ asanka@chromium.org

[dgn, 2015-11-13 16:34:03]

asanka@: PTAL. What do you think about that way of grouping the status codes?
I'm not sure about the complete effects of each.

Also, please note it's very WIP. I'm experimenting with @IntDef, and I have a
build issue.

[asanka, 2015-11-25 14:54:38]

asanka@chromium.org changed reviewers:
+ aberent@chromium.org

[asanka, 2015-11-25 14:54:39]

Thanks for taking this on. This is going in the correct direction, but needs to
be tied into //net behavior. Also are we already working with a third party for
developing an authenticator? If so it might be good to get their take on the API
as well.

[asanka, 2015-11-25 15:08:08]

How does an authenticator handle the case where it can't authenticate to a
specific target and this fact isn't known until it talks to an external service?
How does the browser tell the difference between the authenticator taking its
time to respond vs. there not being an authenticator that supports the token
type?

Also, can there be multiple authentication handshakes going on at the same time
for the same token type? This is important since Negotiate / NTLM are connection
based authentication schemes. Each connection needs to be authenticated and
there could be multiple connection attempts going on at the same time.

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
File net/android/java/src/org/chromium/net/HttpNegotiateConstants.java (right):

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
net/android/java/src/org/chromium/net/HttpNegotiateConstants.java:43: public
static final int STATUS_CODE_OK = 0;
For multi-round authentication schemes like Negotiate, we expect two kinds of
success codes:

  1. So far so good. Here's a token to send back to the server. I expect at
least one more round of token exchanges.

  2. Success. We are done. No more token exchanges are expected and this
authentication session should not be used any more.

From the perspective of an HTTP client, this isn't super important, because in
both cases the client has to send back the token (if there is one) in a suitable
authorization header. But if we are going to establish authentication contexts,
then the difference means that we can extract metadata about the established
context after the final success code.

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
net/android/java/src/org/chromium/net/HttpNegotiateConstants.java:47: * Return
this to tell the browser to not retry until it is restarted.
Not sure what 'it' means here. Also the distinction we'd probably want to make
is:

 1. Permanent error. Don't call me again "for a while." This would be the case
where the authenticator doesn't have credentials.

 2. Target specific error. Don't call me again for this target "for a while."
This would be the case where the credentials can't be used to authenticate to
the specific target.

The state of the browser is irrelevant for authentication if the credentials are
supplied externally. Also the browser is unaware of the state of the
authenticator. Hence the browser can't decide whether or not to send a request
out based on whether the authenticator has restarted or not. There needs to be a
better signal.

[dgn, 2015-11-25 16:16:56]

On 2015/11/25 15:08:08, asanka wrote:
> How does an authenticator handle the case where it can't authenticate to a
> specific target and this fact isn't known until it talks to an external
service?
> How does the browser tell the difference between the authenticator taking its
> time to respond vs. there not being an authenticator that supports the token
> type?
>

So far, we have no timeout mechanism and just wait for the authenticator to
respond, expecting it to return an error if it doesn't support the token type.
If we get interrupted while waiting, we just fail (unexpected error)

> Also, can there be multiple authentication handshakes going on at the same
time
> for the same token type? This is important since Negotiate / NTLM are
connection
> based authentication schemes. Each connection needs to be authenticated and
> there could be multiple connection attempts going on at the same time.

Didn't look into that at all. I think a different HttpNegotiateAuthenticator is
created for each request, and concurrent authenticators should not share
anything besides the broadcast intents for credential confirmation on webview.

On 2015/11/25 14:54:39, asanka wrote:
> Thanks for taking this on. This is going in the correct direction, but needs
to
> be tied into //net behavior. Also are we already working with a third party
for
> developing an authenticator? If so it might be good to get their take on the
API
> as well.

We had requests for the feature but AFAIK not much communication after that. I
don't know much
about authenticators being developed. I also don't have that much bandwidth to
take on
discussion with partners and extend contexts on android. I'll see with some
enterprise people what
the timeline for this should be, and probably transfer this to someone else
depending on what needs
to be done. But I think it should independent from the basic Kerberos port to
webview.

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
File net/android/java/src/org/chromium/net/HttpNegotiateConstants.java (right):

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
net/android/java/src/org/chromium/net/HttpNegotiateConstants.java:43: public
static final int STATUS_CODE_OK = 0;
On 2015/11/25 15:08:08, asanka wrote:
> For multi-round authentication schemes like Negotiate, we expect two kinds of
> success codes:
> 
>   1. So far so good. Here's a token to send back to the server. I expect at
> least one more round of token exchanges.
> 
>   2. Success. We are done. No more token exchanges are expected and this
> authentication session should not be used any more.
> 
> From the perspective of an HTTP client, this isn't super important, because in
> both cases the client has to send back the token (if there is one) in a
suitable
> authorization header. But if we are going to establish authentication
contexts,
> then the difference means that we can extract metadata about the established
> context after the final success code.
> 

Is this distinction made on the desktop? AFAIK on Android we just have one
success code and it's up to the server to require another token.

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
net/android/java/src/org/chromium/net/HttpNegotiateConstants.java:47: * Return
this to tell the browser to not retry until it is restarted.
On 2015/11/25 15:08:08, asanka wrote:
> Not sure what 'it' means here. Also the distinction we'd probably want to make
> is:
> 
>  1. Permanent error. Don't call me again "for a while." This would be the case
> where the authenticator doesn't have credentials.
> 
>  2. Target specific error. Don't call me again for this target "for a while."
> This would be the case where the credentials can't be used to authenticate to
> the specific target.
> 
> The state of the browser is irrelevant for authentication if the credentials
are
> supplied externally. Also the browser is unaware of the state of the
> authenticator. Hence the browser can't decide whether or not to send a request
> out based on whether the authenticator has restarted or not. There needs to be
a
> better signal.

'it' was referring to the browser, I wrote that based on the comments on the net
constant comments. But yes, it's not accurate and going for an explicitly vague
description seems better, since authenticators should not care about the
browser's state

[asanka, 2015-11-25 20:10:08]

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
File net/android/java/src/org/chromium/net/HttpNegotiateConstants.java (right):

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
net/android/java/src/org/chromium/net/HttpNegotiateConstants.java:43: public
static final int STATUS_CODE_OK = 0;
On 2015/11/25 at 16:16:56, dgn wrote:
> On 2015/11/25 15:08:08, asanka wrote:
> > For multi-round authentication schemes like Negotiate, we expect two kinds
of
> > success codes:
> > 
> >   1. So far so good. Here's a token to send back to the server. I expect at
> > least one more round of token exchanges.
> > 
> >   2. Success. We are done. No more token exchanges are expected and this
> > authentication session should not be used any more.
> > 
> > From the perspective of an HTTP client, this isn't super important, because
in
> > both cases the client has to send back the token (if there is one) in a
suitable
> > authorization header. But if we are going to establish authentication
contexts,
> > then the difference means that we can extract metadata about the established
> > context after the final success code.
> > 
> 
> Is this distinction made on the desktop? AFAIK on Android we just have one
success code and it's up to the server to require another token.

That depends on what the strategy is for handling an authentication session. If
we don't need to do any cleanup upon completion of a handshake, then //net
doesn't need to know the difference. This is something that //net doesn't do a
good job of currently and needs to be fixed.

Looking at
https://www.chromium.org/developers/design-documents/http-authentication/writ...
, it seems that the way to tear down a session is for the authenticator to not
specify a KEY_SPNEGO_CONTEXT key in the bundle. So I think we are okay with this
aspect of the API. I.e. the server can already signal the end of an
authentication session.

https://codereview.chromium.org/1442053002/diff/1/net/android/java/src/org/ch...
net/android/java/src/org/chromium/net/HttpNegotiateConstants.java:47: * Return
this to tell the browser to not retry until it is restarted.
On 2015/11/25 at 16:16:56, dgn wrote:
> On 2015/11/25 15:08:08, asanka wrote:
> > Not sure what 'it' means here. Also the distinction we'd probably want to
make
> > is:
> > 
> >  1. Permanent error. Don't call me again "for a while." This would be the
case
> > where the authenticator doesn't have credentials.
> > 
> >  2. Target specific error. Don't call me again for this target "for a
while."
> > This would be the case where the credentials can't be used to authenticate
to
> > the specific target.
> > 
> > The state of the browser is irrelevant for authentication if the credentials
are
> > supplied externally. Also the browser is unaware of the state of the
> > authenticator. Hence the browser can't decide whether or not to send a
request
> > out based on whether the authenticator has restarted or not. There needs to
be a
> > better signal.
> 
> 'it' was referring to the browser, I wrote that based on the comments on the
net constant comments. But yes, it's not accurate and going for an explicitly
vague description seems better, since authenticators should not care about the
browser's state

Ok. But what does "Return this to tell the browser to not retry until it is
restarted" mean? I'm guessing that's a typo and that this error signals that the
browser should try to invoke the authenticator again if an authentication
challenge comes along for the same target?

[aberent, 2016-10-04 08:44:43]

Is this CL still live? If not then please close it.