Support Kerberos on Android

net/android/http_auth_negotiate_android.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/android/http_auth_negotiate_android.h"
+
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
+#include "base/callback_internal.h"
+#include "base/location.h"
+#include "base/thread_task_runner_handle.h"
+#include "jni/HttpNegotiateAuthenticator_jni.h"
+#include "net/base/auth.h"
+#include "net/base/net_errors.h"
+#include "net/http/http_auth_challenge_tokenizer.h"
+#include "net/http/http_auth_handler_negotiate_parse.h"
+
+using base::android::AttachCurrentThread;
+using base::android::ConvertUTF8ToJavaString;
+using base::android::ConvertJavaStringToUTF8;
+using base::android::ScopedJavaLocalRef;
+
+namespace net {
+namespace android {
+
+JavaNegotiateResultWrapper::JavaNegotiateResultWrapper(
+    const scoped_refptr<base::TaskRunner>& callback_task_runner,
+    const base::Callback<void(int, const std::string&)>& thread_safe_callback)
+    : callback_task_runner_(callback_task_runner),
+      thread_safe_callback_(thread_safe_callback) {
+}
+
+JavaNegotiateResultWrapper::~JavaNegotiateResultWrapper() {
+}
+
+void JavaNegotiateResultWrapper::SetResult(JNIEnv* env,
+                                           jobject obj,
+                                           int result,
+                                           jstring token) {
+  // This will be called on the UI thread, so we have to post a task back to the
+  // correct thread to actually save the result
+  std::string raw_token = ConvertJavaStringToUTF8(env, token);
+  // Always post, even if we are on the same thread. This guarantees that the
+  // result will be delayed until after the request has completed, which
+  // simplifies the logic. In practice the result will only ever come back on
+  // the original thread in an obscure error case.
+  callback_task_runner_->PostTask(
+      FROM_HERE, base::Bind(thread_safe_callback_, result, raw_token));
+  delete this;
+}
+
+HttpAuthNegotiateAndroid::HttpAuthNegotiateAndroid(
+    const std::string& account_type)
+    : account_type_(account_type),
+      can_delegate_(false),
+      first_challenge_(true),
+      auth_token_(nullptr),
+      weak_factory_(this) {
+  DCHECK(!account_type.empty());
+  auto env = AttachCurrentThread();
+  java_authenticator_.Reset(Java_HttpNegotiateAuthenticator_create(
+      env, ConvertUTF8ToJavaString(env, account_type).obj()));
+}
+
+HttpAuthNegotiateAndroid::~HttpAuthNegotiateAndroid() {
+}
+
+bool HttpAuthNegotiateAndroid::Init() {
+  return true;
+}
+
+bool HttpAuthNegotiateAndroid::NeedsIdentity() const {
+  return false;
+}
+
+bool HttpAuthNegotiateAndroid::AllowsExplicitCredentials() const {
+  return false;
+}
+
+HttpAuth::AuthorizationResult HttpAuthNegotiateAndroid::ParseChallenge(
+    net::HttpAuthChallengeTokenizer* tok) {
+  if (first_challenge_) {
+    first_challenge_ = false;
+    return net::ParseFirstNegotiateChallenge("negotiate", tok);
+  } else {

[Ryan Sleevi, 2015/06/29 13:56:44]

Don't do else after a return

if (first_challenge_) {
  first_challenge_ = false;
  return net::ParseFirst...
}

std::string decoded_auth_token;
return net::ParseAnother...

[aberent, 2015/07/02 21:13:35]

Done.

+    std::string decoded_auth_token;
+    return net::ParseAnotherNegotiateChallenge(
+        "negotiate", tok, &server_auth_token_, &decoded_auth_token);
+  }
+}
+
+int HttpAuthNegotiateAndroid::GenerateAuthToken(
+    const AuthCredentials* credentials,
+    const std::string& spn,
+    std::string* auth_token,
+    const net::CompletionCallback& callback) {
+  DCHECK(auth_token);
+  DCHECK(completion_callback_.is_null());
+  DCHECK(!callback.is_null());
+
+  auth_token_ = auth_token;
+  completion_callback_ = callback;
+  auto callback_task_runner = base::ThreadTaskRunnerHandle::Get();

[Ryan Sleevi, 2015/06/29 13:56:44]

STYLE: Not a valid use of auto

[aberent, 2015/07/02 21:13:34]

Please explain further. The Google style guide simply says:

"auto is permitted, for local variables only. Do not use auto for file-scope or
namespace-scope variables, or for class members. Never initialize an auto-typed
variable with a braced initializer list."

and the Chromium C++11 page simply references the Google style guide for auto.

These are all local variables, and the code seems considerably tidier using auto
here rather than explicit types.

[Ryan Sleevi, 2015/07/02 23:51:47]

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#auto

"Use auto to avoid type names that are just clutter. ***Continue to use manifest
type declarations when it helps readability,*** and never use auto for anything
but local variables."

From https://chromium-cpp.appspot.com/ on auto, you will see two discussion
threads linked. You can see examples such as
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/OQyYSfH9m2M/3qxtL...
or
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/5-Bt3BJzAo0/lpau2...

In every single use of "auto" you've used here, it is wholly non-obvious what
the return type is, which is why it isn't a valid use. It occludes ownership
semantics (such as this line, which has a scoped_refptr), it occludes the
expected callback signature type (such as 105), resulting in possibly incorrect
conversions. 107 - 111 all hide ownership semantics, and line 112 explicitly
(and dangerously) so.

The goal of auto - and the style guide in general - is to develop consistency
that primarily optimizes for the reader.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Background lays
out these principles.

I realize it makes it convenient for typing, but if as a reviewer - or a future
maintainer - I have to look up each of these function calls to see what they
return to determine whether or not this code is correct, then that's a net-loss
for productivity, even if there's a short-term gain for writability.

Sorry that I wasn't clearer in the original message, by pointing to the times on
chromium-dev where this has come up in the past (and has been enshrined in our
policy on auto)

[aberent, 2015/07/03 10:43:49]

Done.

+  auto thread_safe_callback = base::Bind(

[Ryan Sleevi, 2015/06/29 13:56:44]

STYLE: Not a valid use of auto

[aberent, 2015/07/03 10:43:49]

Done.

+      &HttpAuthNegotiateAndroid::SetResultInternal, weak_factory_.GetWeakPtr());
+  auto env = AttachCurrentThread();

[Ryan Sleevi, 2015/06/29 13:56:44]

STYLE: Not a valid use of auto

[aberent, 2015/07/03 10:43:49]

Done.

+  auto java_server_auth_token =
+      ConvertUTF8ToJavaString(env, server_auth_token_);

[Ryan Sleevi, 2015/06/29 13:56:45]

STYLE: Not a valid use of auto

[aberent, 2015/07/03 10:43:49]

Done.

+  auto java_spn = ConvertUTF8ToJavaString(env, spn);

[Ryan Sleevi, 2015/06/29 13:56:45]

STYLE: Not a valid use of auto

[aberent, 2015/07/03 10:43:49]

Done.

+  auto java_account_type = ConvertUTF8ToJavaString(env, account_type_);

[Ryan Sleevi, 2015/06/29 13:56:44]

STYLE: Not a valid use of auto

[aberent, 2015/07/03 10:43:49]

Done.

+  auto callback_wrapper = new JavaNegotiateResultWrapper(callback_task_runner,

[Ryan Sleevi, 2015/06/29 13:56:45]

STYLE: Not a valid use of auto

[aberent, 2015/07/03 10:43:49]

Done.

+                                                         thread_safe_callback);
+  Java_HttpNegotiateAuthenticator_getNextAuthToken(
+      env, java_authenticator_.obj(),
+      reinterpret_cast<intptr_t>(callback_wrapper), java_spn.obj(),
+      java_server_auth_token.obj(), can_delegate_);
+  return ERR_IO_PENDING;
+}
+
+void HttpAuthNegotiateAndroid::Delegate() {
+  can_delegate_ = true;
+}
+
+void HttpAuthNegotiateAndroid::SetResultInternal(int result,
+                                                 const std::string& raw_token) {
+  DCHECK(auth_token_);
+  DCHECK(!completion_callback_.is_null());
+  if (result == OK)
+    *auth_token_ = "Negotiate " + raw_token;
+  completion_callback_.Run(result);
+  completion_callback_.Reset();

[Ryan Sleevi, 2015/06/29 13:56:45]

use base::ResetAndReturn() here

[aberent, 2015/07/02 21:13:34]

Done.

+}
+
+bool HttpAuthNegotiateAndroid::Register(JNIEnv* env) {

[Ryan Sleevi, 2015/06/29 13:56:45]

The order of methods in the .cc should match the order of methods in the .h

[aberent, 2015/07/02 21:13:35]

Done.

+  return RegisterNativesImpl(env);
+}
+
+}  // namespace android
+}  // namespace net