Remove AccessVerifier interface.

Remove AccessVerifier interface.

AccessVerifier will be replaced with the new 
protocol::Authenticator interface. This also changes behavior 
of Session interface when session is rejected due to bad 
auth: now the session is first accepted by ChromotingHost 
but then goes to FAILED state with the new 
AUTHENTICATION_FAILED error. Temporarily auth token is 
verified in JingleSession::AcceptConnection(). Later 
Authenticator implementation will be used instead.

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

[Wez, 2011-11-22 23:44:12]

How does the change to going through the accept state to failed affect
compatibility between versions?

[Wez, 2011-11-23 01:23:04]

I'm not sure how this interacts with your other recent CLs, specifically in the
placement of VerifySupportAuthToken().  Also not sure whether we really need
AccessVerifier any more?

http://codereview.chromium.org/8662001/diff/1/remoting/host/access_verifier.h
File remoting/host/access_verifier.h (right):

http://codereview.chromium.org/8662001/diff/1/remoting/host/access_verifier.h...
remoting/host/access_verifier.h:16: // in the Me2Me scenario,
SupportAccessVerifier is used for Me2Mom.
Presumably SupportAccessVerifier is no longer required?

http://codereview.chromium.org/8662001/diff/1/remoting/host/access_verifier.h...
remoting/host/access_verifier.h:17: class AccessVerifier {
Why do we still need this interface?  Can't we apply this access control in
Authenticator implementations?

http://codereview.chromium.org/8662001/diff/1/remoting/host/access_verifier.h...
remoting/host/access_verifier.h:22: virtual bool VerifyPermissions(const
std::string& client_jid) = 0;
Do we want the |client_jid|, or the user's GAIA Id?
If this interface now controls access purely based on JID (or GAIA Id?) then
perhaps it needs a new name?

http://codereview.chromium.org/8662001/diff/1/remoting/host/client_session.h
File remoting/host/client_session.h (right):

http://codereview.chromium.org/8662001/diff/1/remoting/host/client_session.h#...
remoting/host/client_session.h:37: // Called after authentication has failed.
Must not tear doen this
typo: doen

[Sergey Ulanov, 2011-11-23 02:26:08]

On 2011/11/22 23:44:12, Wez wrote:
> How does the change to going through the accept state to failed affect
> compatibility between versions?

This change doesn't affect the protocol. I changed interaction between layers,
but not the protocol. We still reject session without accepting it when
session-initiate has invalid auth token.

[Sergey Ulanov, 2011-11-23 21:18:46]

Removed AccessVerifier.

http://codereview.chromium.org/8662001/diff/1/remoting/host/access_verifier.h
File remoting/host/access_verifier.h (right):

http://codereview.chromium.org/8662001/diff/1/remoting/host/access_verifier.h...
remoting/host/access_verifier.h:17: class AccessVerifier {
On 2011/11/23 01:23:04, Wez wrote:
> Why do we still need this interface?  Can't we apply this access control in
> Authenticator implementations?
We may still need something like it later, e.g. for directory-side access
lists,but we can add it back if necessary. Removed for now.

http://codereview.chromium.org/8662001/diff/1/remoting/host/client_session.h
File remoting/host/client_session.h (right):

http://codereview.chromium.org/8662001/diff/1/remoting/host/client_session.h#...
remoting/host/client_session.h:37: // Called after authentication has failed.
Must not tear doen this
On 2011/11/23 01:23:04, Wez wrote:
> typo: doen

Done.

[Sergey Ulanov, 2011-11-29 00:41:28]

ping

[Wez, 2011-11-29 05:58:47]

lgtm

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_session.h
File remoting/host/client_session.h (right):

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:38: // object. Separate OnSessionClosed() is made
after this handler
nit: Separate OnSessionClosed() is made ... -> OnSessionClosed is notified ...

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:42: // Called if connection has failed or after
the client closed it.
nit: if -> after?

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:45: // Called for to notify of each message's
sequence number. The
nit: for to notify of each message's ... -> to notify on receipt of a message

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:46: // callback cannot tear down this object.
nit: cannot -> must not

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.cc
File remoting/host/host_secret.cc (right):

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.c...
remoting/host/host_secret.cc:20: const char kHostSecretAlphabet[] =
"0123456789";
Now that the alphabet for codes is 0-9, we could just call
CryptoRandomInt(10**5) and stringify that.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.c...
remoting/host/host_secret.cc:32: std::vector<char> result;
Can't we just create the string and generate random data into it, rather than
going via a vector?

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.h
File remoting/host/host_secret.h (right):

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.h...
remoting/host/host_secret.h:9: // Generates random host secret.q
Is there supposed to be a 'q' on the end of this line?

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.h...
remoting/host/host_secret.h:10: std::string GenerateRandomHostSecret();
Call this GenerateSupportHostSecret()?  Random seems implicit in it being
"generated", and we need to make it clear that this secret is nothing to do with
the host private key, nor Me2Me PIN.

[Sergey Ulanov, 2011-11-29 18:52:00]

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_session.h
File remoting/host/client_session.h (right):

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:38: // object. Separate OnSessionClosed() is made
after this handler
On 2011/11/29 05:58:47, Wez wrote:
> nit: Separate OnSessionClosed() is made ... -> OnSessionClosed is notified ...

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:42: // Called if connection has failed or after
the client closed it.
On 2011/11/29 05:58:47, Wez wrote:
> nit: if -> after?

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:45: // Called for to notify of each message's
sequence number. The
On 2011/11/29 05:58:47, Wez wrote:
> nit: for to notify of each message's ... -> to notify on receipt of a message

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/client_sessio...
remoting/host/client_session.h:46: // callback cannot tear down this object.
On 2011/11/29 05:58:47, Wez wrote:
> nit: cannot -> must not

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.cc
File remoting/host/host_secret.cc (right):

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.c...
remoting/host/host_secret.cc:20: const char kHostSecretAlphabet[] =
"0123456789";
On 2011/11/29 05:58:47, Wez wrote:
> Now that the alphabet for codes is 0-9, we could just call
> CryptoRandomInt(10**5) and stringify that.

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.c...
remoting/host/host_secret.cc:32: std::vector<char> result;
On 2011/11/29 05:58:47, Wez wrote:
> Can't we just create the string and generate random data into it, rather than
> going via a vector?

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.h
File remoting/host/host_secret.h (right):

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.h...
remoting/host/host_secret.h:9: // Generates random host secret.q
On 2011/11/29 05:58:47, Wez wrote:
> Is there supposed to be a 'q' on the end of this line?

Done.

http://codereview.chromium.org/8662001/diff/12001/remoting/host/host_secret.h...
remoting/host/host_secret.h:10: std::string GenerateRandomHostSecret();
On 2011/11/29 05:58:47, Wez wrote:
> Call this GenerateSupportHostSecret()?  Random seems implicit in it being
> "generated", and we need to make it clear that this secret is nothing to do
with
> the host private key, nor Me2Me PIN.

Done.

[commit-bot: I haz the power, 2011-11-29 18:52:34]

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

[commit-bot: I haz the power, 2011-11-29 20:20:01]

Change committed as 112009