Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(62)

Issue 165293004: Refactor TokenValidatorImpl into a base class + implementation. (Closed)

Created:
6 years, 10 months ago by rmsousa
Modified:
6 years, 10 months ago
CC:
chromoting-reviews_chromium.org
Visibility:
Public.

Description

Refactor TokenValidatorImpl into a base class + implementation. Most of the common logic and response handling code are moved into a base class, and the implementation contains just the logic to prepare the validation request (which is where the actual keypair-based authentication happens). This makes it easier to implement different, non-keypair-based host authentication mechanisms. BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253378

Patch Set 1 #

Total comments: 2

Patch Set 2 : Refactor TokenValidator out of ThirdPartyHostAuthenticator #

Patch Set 3 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+213 lines, -401 lines) Patch
M remoting/host/remoting_me2me_host.cc View 1 2 2 chunks +3 lines, -2 lines 0 comments Download
A remoting/host/token_validator_base.h View 1 1 chunk +90 lines, -0 lines 0 comments Download
A + remoting/host/token_validator_base.cc View 9 chunks +18 lines, -132 lines 0 comments Download
M remoting/host/token_validator_factory_impl.h View 1 2 chunks +6 lines, -20 lines 0 comments Download
M remoting/host/token_validator_factory_impl.cc View 1 7 chunks +12 lines, -194 lines 0 comments Download
M remoting/host/token_validator_factory_impl_unittest.cc View 1 1 chunk +1 line, -2 lines 0 comments Download
M remoting/protocol/me2me_host_authenticator_factory.h View 1 3 chunks +3 lines, -4 lines 0 comments Download
M remoting/protocol/me2me_host_authenticator_factory.cc View 1 2 chunks +2 lines, -1 line 0 comments Download
M remoting/protocol/negotiating_host_authenticator.h View 1 2 chunks +2 lines, -2 lines 0 comments Download
M remoting/protocol/negotiating_host_authenticator.cc View 1 2 chunks +2 lines, -1 line 0 comments Download
M remoting/protocol/third_party_authenticator_unittest.cc View 1 3 chunks +3 lines, -4 lines 0 comments Download
M remoting/protocol/third_party_host_authenticator.h View 1 3 chunks +2 lines, -39 lines 0 comments Download
M remoting/protocol/third_party_host_authenticator.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
A remoting/protocol/token_validator.h View 1 1 chunk +65 lines, -0 lines 0 comments Download
M remoting/remoting_host.gypi View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M remoting/remoting_srcs.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
rmsousa
6 years, 10 months ago (2014-02-14 03:03:47 UTC) #1
Sergey Ulanov
LGTM with one optional suggestion https://codereview.chromium.org/165293004/diff/1/remoting/host/token_validator_base.h File remoting/host/token_validator_base.h (right): https://codereview.chromium.org/165293004/diff/1/remoting/host/token_validator_base.h#newcode38 remoting/host/token_validator_base.h:38: public protocol::ThirdPartyHostAuthenticator::TokenValidator { Maybe ...
6 years, 10 months ago (2014-02-18 07:10:04 UTC) #2
rmsousa
6 years, 10 months ago (2014-02-19 02:29:12 UTC) #3
rmsousa
https://codereview.chromium.org/165293004/diff/1/remoting/host/token_validator_base.h File remoting/host/token_validator_base.h (right): https://codereview.chromium.org/165293004/diff/1/remoting/host/token_validator_base.h#newcode38 remoting/host/token_validator_base.h:38: public protocol::ThirdPartyHostAuthenticator::TokenValidator { On 2014/02/18 07:10:04, Sergey Ulanov wrote: ...
6 years, 10 months ago (2014-02-19 02:29:42 UTC) #4
rmsousa
The CQ bit was checked by rmsousa@chromium.org
6 years, 10 months ago (2014-02-20 22:55:16 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rmsousa@chromium.org/165293004/270001
6 years, 10 months ago (2014-02-20 22:56:03 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-20 22:56:15 UTC) #7
commit-bot: I haz the power
Failed to apply patch for remoting/remoting.gyp: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
6 years, 10 months ago (2014-02-20 22:56:16 UTC) #8
rmsousa
The CQ bit was checked by rmsousa@chromium.org
6 years, 10 months ago (2014-02-25 01:51:26 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rmsousa@chromium.org/165293004/440001
6 years, 10 months ago (2014-02-25 02:04:52 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rmsousa@chromium.org/165293004/440001
6 years, 10 months ago (2014-02-25 07:37:05 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-25 08:18:09 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel
6 years, 10 months ago (2014-02-25 08:18:10 UTC) #13
rmsousa
The CQ bit was checked by rmsousa@chromium.org
6 years, 10 months ago (2014-02-26 02:13:48 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rmsousa@chromium.org/165293004/440001
6 years, 10 months ago (2014-02-26 04:40:30 UTC) #15
Paweł Hajdan Jr.
The CQ bit was unchecked by phajdan.jr@chromium.org
6 years, 10 months ago (2014-02-26 04:40:30 UTC) #16
Paweł Hajdan Jr.
The CQ bit was checked by phajdan.jr@chromium.org
6 years, 10 months ago (2014-02-26 05:29:00 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rmsousa@chromium.org/165293004/440001
6 years, 10 months ago (2014-02-26 05:38:14 UTC) #18
commit-bot: I haz the power
6 years, 10 months ago (2014-02-26 06:30:32 UTC) #19
Message was sent while issue was closed.
Change committed as 253378

Powered by Google App Engine
This is Rietveld 408576698