Add implementation for current IT2Me auth.

Add implementation for current IT2Me auth.

The new SimpleClientAuthenticator and SimpleHostAuthenticator implement
the current IT2Me authentication mechanism.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=111321

[Wez, 2011-11-22 22:58:05]

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_authen...
File remoting/protocol/simple_authenticator_unittest.cc (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_authen...
remoting/protocol/simple_authenticator_unittest.cc:5: // Unittests in this file
test both SimpleClientAuthenticator and
nit: Unit tests in this file...

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_authen...
remoting/protocol/simple_authenticator_unittest.cc:6: //
SimpleHostAuthenticator.
Do you really need this comment?

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
File remoting/protocol/simple_client_authenticator.cc (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.cc:51: remote_cert_ = "";
nit: Is there no .clear() or similar in std::string?

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
File remoting/protocol/simple_client_authenticator.h (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.h:15: class
SimpleClientAuthenticator : public Authenticator {
More descriptive name?

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.h:15: class
SimpleClientAuthenticator : public Authenticator {
Simple is not a very descriptive name. :(

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.h:21: // Authenticator
implementation.
nit: implementation -> interface?

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
File remoting/protocol/simple_host_authenticator.cc (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
remoting/protocol/simple_host_authenticator.cc:64:
QName(kChromotingXmlNamespace, kAuthenticationTag));
nit: Create |message| lower down, where it's actually used?

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
remoting/protocol/simple_host_authenticator.cc:94: // TODO(hclam): Need a better
way to clone a key.
nit: We should just create a bug for that, reference it here, and fix it!

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
File remoting/protocol/simple_host_authenticator.h (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
remoting/protocol/simple_host_authenticator.h:20: class SimpleHostAuthenticator
: public Authenticator {
nit: It may be worth folding the client and host authenticator implementations
into a single code file, to avoid duplicating some fields, and having an extra
pair of files for what will become a legacy auth protocol?

[Sergey Ulanov, 2011-11-23 02:02:24]

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_authen...
File remoting/protocol/simple_authenticator_unittest.cc (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_authen...
remoting/protocol/simple_authenticator_unittest.cc:5: // Unittests in this file
test both SimpleClientAuthenticator and
On 2011/11/22 22:58:05, Wez wrote:
> nit: Unit tests in this file...

Done.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_authen...
remoting/protocol/simple_authenticator_unittest.cc:6: //
SimpleHostAuthenticator.
On 2011/11/22 22:58:05, Wez wrote:
> Do you really need this comment?

Done.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
File remoting/protocol/simple_client_authenticator.cc (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.cc:51: remote_cert_ = "";
On 2011/11/22 22:58:05, Wez wrote:
> nit: Is there no .clear() or similar in std::string?

Done.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
File remoting/protocol/simple_client_authenticator.h (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.h:15: class
SimpleClientAuthenticator : public Authenticator {
On 2011/11/22 22:58:05, Wez wrote:
> More descriptive name?

Done.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.h:15: class
SimpleClientAuthenticator : public Authenticator {
On 2011/11/22 22:58:05, Wez wrote:
> Simple is not a very descriptive name. :(

Done.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_client...
remoting/protocol/simple_client_authenticator.h:21: // Authenticator
implementation.
On 2011/11/22 22:58:05, Wez wrote:
> nit: implementation -> interface?

Done.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
File remoting/protocol/simple_host_authenticator.cc (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
remoting/protocol/simple_host_authenticator.cc:64:
QName(kChromotingXmlNamespace, kAuthenticationTag));
On 2011/11/22 22:58:05, Wez wrote:
> nit: Create |message| lower down, where it's actually used?

We create XML tree here, and I think it's easier to read when it is created
starting from the root.

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
remoting/protocol/simple_host_authenticator.cc:94: // TODO(hclam): Need a better
way to clone a key.
On 2011/11/22 22:58:05, Wez wrote:
> nit: We should just create a bug for that, reference it here, and fix it!
Opened crbug.com/105220

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
File remoting/protocol/simple_host_authenticator.h (right):

http://codereview.chromium.org/8647001/diff/1/remoting/protocol/simple_host_a...
remoting/protocol/simple_host_authenticator.h:20: class SimpleHostAuthenticator
: public Authenticator {
On 2011/11/22 22:58:05, Wez wrote:
> nit: It may be worth folding the client and host authenticator implementations
> into a single code file, to avoid duplicating some fields, and having an extra
> pair of files for what will become a legacy auth protocol?

Done

[Wez, 2011-11-23 02:17:06]

lgtm

[commit-bot: I haz the power, 2011-11-23 04:18:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/8647001/8001

[commit-bot: I haz the power, 2011-11-23 05:55:03]

Change committed as 111321