Cleanup AuthenticatorMethod usage.

remoting/protocol/negotiating_authenticator_unittest.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/macros.h"
 #include "net/base/net_errors.h"
 #include "remoting/base/rsa_key_pair.h"
+#include "remoting/protocol/auth_util.h"
 #include "remoting/protocol/authenticator_test_base.h"
 #include "remoting/protocol/channel_authenticator.h"
 #include "remoting/protocol/connection_tester.h"
 #include "remoting/protocol/negotiating_authenticator_base.h"
 #include "remoting/protocol/negotiating_client_authenticator.h"
 #include "remoting/protocol/negotiating_host_authenticator.h"
 #include "remoting/protocol/pairing_registry.h"
 #include "remoting/protocol/protocol_mock_objects.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 28 matching lines @@
  public:
   NegotiatingAuthenticatorTest() {}
   ~NegotiatingAuthenticatorTest() override {}
 
  protected:
   void InitAuthenticators(const std::string& client_id,
                           const std::string& client_paired_secret,
                           const std::string& client_interactive_pin,
                           const std::string& host_secret,
                           bool it2me,
-                          bool client_hmac_only) {
+                          bool client_no_it2me) {
     if (it2me) {
       host_ = NegotiatingHostAuthenticator::CreateForIt2Me(
           host_cert_, key_pair_, host_secret);
     } else {
-      std::string host_secret_hash = ApplySharedSecretHashFunction(
-          HashFunction::HMAC_SHA256, kTestHostId, host_secret);
+      std::string host_secret_hash =
+          GetSharedSecretHash(kTestHostId, host_secret);
       host_ = NegotiatingHostAuthenticator::CreateWithPin(
           host_cert_, key_pair_, host_secret_hash, pairing_registry_);
     }
 
-    std::vector<AuthenticationMethod> methods;
-    methods.push_back(AuthenticationMethod::SPAKE2_PAIR);
-    methods.push_back(AuthenticationMethod::SPAKE2_SHARED_SECRET_HMAC);
-    if (!client_hmac_only) {
-      methods.push_back(AuthenticationMethod::SPAKE2_SHARED_SECRET_PLAIN);
-    }
     bool pairing_expected = pairing_registry_.get() != nullptr;
     FetchSecretCallback fetch_secret_callback =
         base::Bind(&NegotiatingAuthenticatorTest::FetchSecret,
                    client_interactive_pin,
                    pairing_expected);
     client_as_negotiating_authenticator_ = new NegotiatingClientAuthenticator(
-        client_id, client_paired_secret,
-        kTestHostId, fetch_secret_callback,
-        nullptr, methods);
+        client_id, client_paired_secret, kTestHostId, fetch_secret_callback,
+        nullptr);
+    client_as_negotiating_authenticator_->methods_.clear();
+    client_as_negotiating_authenticator_->methods_.push_back(
+        NegotiatingAuthenticatorBase::Method::SPAKE2_PAIR);
+    client_as_negotiating_authenticator_->methods_.push_back(
+        NegotiatingAuthenticatorBase::Method::SPAKE2_SHARED_SECRET_HMAC);
+    if (!client_no_it2me) {
+      client_as_negotiating_authenticator_->methods_.push_back(
+          NegotiatingAuthenticatorBase::Method::SPAKE2_SHARED_SECRET_PLAIN);
+    }

[Jamie, 2016/03/08 02:17:38]

What's the purpose of this test? It seems like you're forcing the authenticator
to operate with a set of Methods that can't be used in production code (because
there's no public method to set them).

[Sergey Ulanov, 2016/03/08 18:32:40]

Yes. We need to test the authenticator in the case when the host and client are
not compatible  (see NegotiatingAuthenticatorTest.IncompatibleMethods) to make
sure correct error is returned. It's not possible to do that using public
interface.

[Jamie, 2016/03/08 22:42:50]

Would it make more sense to override them only for the IncompatibleMethods test,
with a comment explaining why it's needed in that case? I'd prefer that the
majority of tests use the code the way it's used in production to minimize
duplication and code-rot.

[Sergey Ulanov, 2016/03/09 03:45:00]

Done.

     client_.reset(client_as_negotiating_authenticator_);
   }
 
   void CreatePairingRegistry(bool with_paired_client) {
     pairing_registry_ = new SynchronousPairingRegistry(
         make_scoped_ptr(new MockPairingRegistryDelegate()));
     if (with_paired_client) {
       PairingRegistry::Pairing pairing(
           base::Time(), kTestClientName, kTestClientId, kTestPairedSecret);
       pairing_registry_->AddPairing(pairing);
@@ skipping 13 matching lines @@
     ASSERT_TRUE(client_->state() == Authenticator::REJECTED ||
                 host_->state() == Authenticator::REJECTED);
     if (client_->state() == Authenticator::REJECTED) {
       ASSERT_EQ(client_->rejection_reason(), reason);
     }
     if (host_->state() == Authenticator::REJECTED) {
       ASSERT_EQ(host_->rejection_reason(), reason);
     }
   }
 
-  void VerifyAccepted(const AuthenticationMethod& expected_method) {
+  void VerifyAccepted(NegotiatingAuthenticatorBase::Method expected_method) {
     ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
 
     ASSERT_EQ(Authenticator::ACCEPTED, host_->state());
     ASSERT_EQ(Authenticator::ACCEPTED, client_->state());
 
     client_auth_ = client_->CreateChannelAuthenticator();
     host_auth_ = host_->CreateChannelAuthenticator();
     RunChannelAuth(false);
 
     EXPECT_TRUE(client_socket_.get() != nullptr);
     EXPECT_TRUE(host_socket_.get() != nullptr);
 
     StreamConnectionTester tester(host_socket_.get(), client_socket_.get(),
                                   kMessageSize, kMessages);
 
     tester.Start();
     message_loop_.Run();
     tester.CheckResults();
-    EXPECT_EQ(
-        expected_method,
-        client_as_negotiating_authenticator_->current_method_for_testing());
+    EXPECT_EQ(expected_method,
+              client_as_negotiating_authenticator_->current_method_);
   }
 
   // Use a bare pointer because the storage is managed by the base class.
   NegotiatingClientAuthenticator* client_as_negotiating_authenticator_;
 
  private:
   scoped_refptr<PairingRegistry> pairing_registry_;
 
   DISALLOW_COPY_AND_ASSIGN(NegotiatingAuthenticatorTest);
 };
 
 TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthMe2MePin) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(kNoClientId, kNoPairedSecret,
                                              kTestPin, kTestPin, false, false));
-  VerifyAccepted(AuthenticationMethod::SPAKE2_SHARED_SECRET_HMAC);
+  VerifyAccepted(
+      NegotiatingAuthenticatorBase::Method::SPAKE2_SHARED_SECRET_HMAC);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthIt2me) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(kNoClientId, kNoPairedSecret,
                                              kTestPin, kTestPin, true, false));
-  VerifyAccepted(AuthenticationMethod::SPAKE2_SHARED_SECRET_PLAIN);
+  VerifyAccepted(
+      NegotiatingAuthenticatorBase::Method::SPAKE2_SHARED_SECRET_PLAIN);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, InvalidMe2MePin) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kNoClientId, kNoPairedSecret, kTestPinBad, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
 
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
 }
 
@@ skipping 10 matching lines @@
       kNoClientId, kNoPairedSecret, kTestPin, kTestPinBad, true, true));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
 
   VerifyRejected(Authenticator::PROTOCOL_ERROR);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingNotSupported) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(kTestClientId, kTestPairedSecret,
                                              kTestPin, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-  VerifyAccepted(AuthenticationMethod::SPAKE2_SHARED_SECRET_HMAC);
+  VerifyAccepted(
+      NegotiatingAuthenticatorBase::Method::SPAKE2_SHARED_SECRET_HMAC);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingSupportedButNotPaired) {
   CreatePairingRegistry(false);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(kNoClientId, kNoPairedSecret,
                                              kTestPin, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-  VerifyAccepted(AuthenticationMethod::SPAKE2_PAIR);
+  VerifyAccepted(NegotiatingAuthenticatorBase::Method::SPAKE2_PAIR);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingRevokedPinOkay) {
   CreatePairingRegistry(false);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(kTestClientId, kTestPairedSecret,
                                              kTestPin, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-  VerifyAccepted(AuthenticationMethod::SPAKE2_PAIR);
+  VerifyAccepted(NegotiatingAuthenticatorBase::Method::SPAKE2_PAIR);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingRevokedPinBad) {
   CreatePairingRegistry(false);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecret, kTestPinBad, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingSucceeded) {
   CreatePairingRegistry(true);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecret, kTestPinBad, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-  VerifyAccepted(AuthenticationMethod::SPAKE2_PAIR);
+  VerifyAccepted(NegotiatingAuthenticatorBase::Method::SPAKE2_PAIR);
 }
 
 TEST_F(NegotiatingAuthenticatorTest,
        PairingSucceededInvalidSecretButPinOkay) {
   CreatePairingRegistry(true);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestClientId, kTestPairedSecretBad, kTestPin, kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-  VerifyAccepted(AuthenticationMethod::SPAKE2_PAIR);
+  VerifyAccepted(NegotiatingAuthenticatorBase::Method::SPAKE2_PAIR);
 }
 
 TEST_F(NegotiatingAuthenticatorTest, PairingFailedInvalidSecretAndPin) {
   CreatePairingRegistry(true);
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(kTestClientId,
                                              kTestPairedSecretBad, kTestPinBad,
                                              kTestPin, false, false));
   ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
   VerifyRejected(Authenticator::INVALID_CREDENTIALS);
 }
 
 }  // namespace protocol
 }  // namespace remoting