Cleanup AuthenticatorMethod usage.

remoting/protocol/negotiating_authenticator_base.h

 // 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.
 
 #ifndef REMOTING_PROTOCOL_NEGOTIATING_AUTHENTICATOR_BASE_H_
 #define REMOTING_PROTOCOL_NEGOTIATING_AUTHENTICATOR_BASE_H_
 
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "remoting/protocol/authentication_method.h"
 #include "remoting/protocol/authenticator.h"
 #include "third_party/webrtc/libjingle/xmllite/xmlelement.h"
 
 namespace remoting {
 namespace protocol {
 
 // This class provides the common base for a meta-authenticator that allows
 // clients and hosts that support multiple authentication methods to negotiate a
 // method to use.
 //
@@ skipping 28 matching lines @@
 //  * The client may optimistically pick a method on its first message (assuming
 //      it doesn't require user interaction to start). If the host doesn't
 //      support that method, it will just discard that message, and choose
 //      another method from the client's supported methods list.
 //  * The host never sends its own supported methods back to the client, so once
 //      the host picks a method from the client's list, it's final.
 //  * Any change in this class must maintain compatibility between any version
 //      mix of webapp, client plugin and host, for both Me2Me and IT2Me.
 class NegotiatingAuthenticatorBase : public Authenticator {
  public:
+  // Method represents an authentication algorithm.
+  enum class Method {
+    INVALID,
+    SPAKE2_SHARED_SECRET_PLAIN,

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

Unrelated to this CL, but when do we use PLAIN, and should we consider removing
support for it?

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

It's used for It2Me. It can potentially be switched to using hashed access code,
but there is little value to doing that - it doesn't make anything more secure.
Also we cannot just switch It2Me host to SPAKE2_SHARED_SECRET_HMAC because for
It2Me the webapp doesn't pass correct authentication_tag. That will need to be
fixed and rolled out before SPAKE2_SHARED_SECRET_PLAIN is removed from the host.

I think for the new version of SPAKE2 authenticator I'm going to implement only
the HMAC case.

+    SPAKE2_SHARED_SECRET_HMAC,
+    SPAKE2_PAIR,
+    THIRD_PARTY,
+  };
+
   ~NegotiatingAuthenticatorBase() override;
 
   // Authenticator interface.
   State state() const override;
   bool started() const override;
   RejectionReason rejection_reason() const override;
   const std::string& GetAuthKey() const override;
   scoped_ptr<ChannelAuthenticator> CreateChannelAuthenticator() const override;
 
   // Calls |current_authenticator_| to process |message|, passing the supplied
   // |resume_callback|.
   void ProcessMessageInternal(const buzz::XmlElement* message,
                               const base::Closure& resume_callback);
 
-  const AuthenticationMethod& current_method_for_testing() const {
-    return current_method_;
-  }
+ protected:
+  friend class NegotiatingAuthenticatorTest;

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

FRIEND_TEST_ALL_PREFIXES?

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

FRIEND_TEST_ALL_PREFIXES is used to friend specific tests.
NegotiatingAuthenticatorTest is a base class for bunch of tests. It doesn't get
prefixed when tests are disabled, so there is no reason to use
FRIEND_TEST_ALL_PREFIXES here.

 
- protected:
   static const buzz::StaticQName kMethodAttributeQName;
   static const buzz::StaticQName kSupportedMethodsAttributeQName;
   static const char kSupportedMethodsSeparator;
 
+  // Parses a string that defines an authentication method. Returns
+  // Method::INVALID if the string is invalid.
+  static Method ParseMethodString(const std::string& value);
+
+  // Returns string representation of |method|.
+  static std::string MethodToString(Method method);
+
   explicit NegotiatingAuthenticatorBase(Authenticator::State initial_state);
 
-  void AddMethod(AuthenticationMethod method);
+  void AddMethod(Method method);
 
   // Updates |state_| to reflect the current underlying authenticator state.
   // |resume_callback| is called after the state is updated.
   void UpdateState(const base::Closure& resume_callback);
 
   // Gets the next message from |current_authenticator_|, if any, and fills in
   // the 'method' tag with |current_method_|.
   virtual scoped_ptr<buzz::XmlElement> GetNextMessageInternal();
 
-  std::vector<AuthenticationMethod> methods_;
-  AuthenticationMethod current_method_ = AuthenticationMethod::INVALID;
+  std::vector<Method> methods_;
+  Method current_method_ = Method::INVALID;
   scoped_ptr<Authenticator> current_authenticator_;
   State state_;
   RejectionReason rejection_reason_ = INVALID_CREDENTIALS;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NegotiatingAuthenticatorBase);
 };
 
 }  // namespace protocol
 }  // namespace remoting
 
 #endif  // REMOTING_PROTOCOL_NEGOTIATING_AUTHENTICATOR_BASE_H_