Adding a new authenticator which can be used to validate the remote user

remoting/protocol/validating_authenticator.h

Index: remoting/protocol/validating_authenticator.h
diff --git a/remoting/protocol/validating_authenticator.h b/remoting/protocol/validating_authenticator.h
new file mode 100644
index 0000000000000000000000000000000000000000..ad5bada5a8ffe0612eba0e882d1b186af617bdae
--- /dev/null
+++ b/remoting/protocol/validating_authenticator.h
@@ -0,0 +1,92 @@
+// Copyright 2016 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_VALIDATING_AUTHENTICATOR_H_
+#define REMOTING_PROTOCOL_VALIDATING_AUTHENTICATOR_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "remoting/protocol/authenticator.h"
+
+namespace remoting {
+namespace protocol {
+
+// This authenticator class provides a way to check the validity of a connection
+// as it is being established through an asynchronous callback.  The validation
+// logic supplied by the caller is run when the first message is received from
+// the client.  If the connection details are valid (i.e. conform to the current

[Jamie, 2016/08/26 23:12:08]

Since the validation is arbitrary, I think you mean e.g. here, not i.e. (the
caller could do something other than check policies, presumably).

[joedow, 2016/08/29 23:43:22]

True, I'll turn i.e. into e.g. and then I think this the comment is accurate and
generic (albeit with an example).

+// policies), then the initial message, and all subsequent messages, are passed
+// to the underlying authenticator instance for processing.
+class ValidatingAuthenticator : public Authenticator {
+ public:
+  enum class Result {
+    SUCCESS,
+    ERROR_INVALID_CREDENTIALS,
+    ERROR_INVALID_ACCOUNT,
+    ERROR_REJECTED_BY_USER
+  };
+
+  typedef base::Callback<void(Result validation_result)> ResultCallback;
+
+  typedef base::Callback<void(const std::string& remote_jid,
+                              const ResultCallback& callback)>
+      ValidationCallback;
+
+  ValidatingAuthenticator(const std::string& remote_jid,

[Jamie, 2016/08/26 23:12:08]

This class doesn't need to know the JID--it just passes it to the callback.
Since the callback might not need to know the JID, it seems like it would be
more flexible for the caller to bind() the JID into the callback if necessary.

[joedow, 2016/08/29 23:43:22]

The component providing the ValidationCallback does not know the JID at the time
the method is bound and the ProcessMessage interface method does not take that
param (i'd hate to increase the number of params just for this scenario).

At this point the Authenticator would be used for It2Me and Me2Me and both
require the remote JID to validate the incoming connection.

[Jamie, 2016/08/31 00:28:34]

You could rebind the callback with the JID at the point of construction of this
class. I think that's a little cleaner than adding an otherwise unnecessary
dependency on the remote JID, but maybe I've just been writing too much JS
lately. I'll leave it to your discretion.

[joedow, 2016/08/31 17:42:29]

Discussed offline.  I could do this but it wouldn't be very clean.

+                          const ValidationCallback& validation_callback,
+                          std::unique_ptr<Authenticator> current_authenticator);
+  ~ValidatingAuthenticator() override;
+
+  // Authenticator interface.
+  State state() const override;
+  bool started() const override;
+  RejectionReason rejection_reason() const override;
+  const std::string& GetAuthKey() const override;
+  std::unique_ptr<ChannelAuthenticator> CreateChannelAuthenticator()
+      const override;
+  void ProcessMessage(const buzz::XmlElement* message,
+                      const base::Closure& resume_callback) override;
+  std::unique_ptr<buzz::XmlElement> GetNextMessage() override;
+
+ private:
+  // Checks |validation_result|.  On success, |message| and |resume_callback|
+  // are passed on to |current_authenticator_|.  If the connection was rejected,
+  // |state_| and |rejection_reason_| are updated and |resume_callback| is run.
+  void OnValidateComplete(const buzz::XmlElement* message,
+                          const base::Closure& resume_callback,
+                          Result validation_result);
+
+  // 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);
+
+  bool first_message_received_ = false;
+
+  // The JID of the remote user.
+  std::string remote_jid_;
+
+  ValidationCallback validation_callback_;
+
+  // Returns the current state of the authenticator.
+  State state_ = Authenticator::WAITING_MESSAGE;
+
+  // Returns the rejection reason. Can be called only when in REJECTED state.
+  RejectionReason rejection_reason_ = Authenticator::INVALID_CREDENTIALS;
+
+  std::unique_ptr<Authenticator> current_authenticator_;
+
+  base::WeakPtrFactory<ValidatingAuthenticator> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ValidatingAuthenticator);
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_VALIDATING_AUTHENTICATOR_H_