Add |id_token| field to AccessTokenInfo and populate it from AccessTokenFetcher/PrimaryAccountAccessTokenFetcher |
||
Issue descriptionThis information is supplied by OAuth2TokenService in a success response to a request for an access token but is not passed along by AccessTokenFetcher. At least one consumer needs it (see blocked bug).
,
Sep 27
Thanks Colin. Just to clarify, by "adding overloads to the relevant FakeProfileOAuth2TokenService methods", do you mean those like the following one?
// Helper routines to issue tokens for pending requests.
void IssueAllTokensForAccount(const std::string& account_id,
const std::string& access_token,
const base::Time& expiration);
..which would get an overload like this:
// Helper routines to issue tokens for pending requests.
void IssueAllTokensForAccount(const std::string& account_id,
const std::string& access_token,
const base::Time& expiration,
const std::string& id_token);
And then I suppose I'd need to adapt some unittest as well, access_token_fetcher_unittest.cc maybe? Anything else?
,
Sep 27
That's what I was thinking, yup! Feel free to tackle it in two stages though: a CL adding the field and passing an empty string in the tests with a TODO to pass valid data through and check that it's preserved, and then a CL fixing that TODO via the additions of the overloads.
,
Sep 27
Thanks for clarifying Colin, much appreciated. I also think it makes sense to split into 2 CLs, here comes the first one (no tests): https://chromium-review.googlesource.com/c/chromium/src/+/1248625
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f0249ae91750468210e7047a54e6cb76ed6d737 commit 8f0249ae91750468210e7047a54e6cb76ed6d737 Author: Mario Sanchez Prada <mario@igalia.com> Date: Thu Sep 27 14:27:03 2018 Add |id_token| field to AccessTokenInfo This hasn't yet been migrated from OAuth2AccessTokenConsumer::TokenResponse and it will be useful at least to determine whether we are under advanced protection from AdvancedProtectionStatusManager once a token is fetched. Additional test coverage will be added in a follow-up patch. TBR=thestig@chromium.org Bug: 889764 Change-Id: Id9a8bcc7b2e3a5238b7a1c98f7787a8e905b6add Reviewed-on: https://chromium-review.googlesource.com/1248625 Commit-Queue: Mario Sanchez Prada <mario@igalia.com> Reviewed-by: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#594713} [modify] https://crrev.com/8f0249ae91750468210e7047a54e6cb76ed6d737/chrome/browser/printing/cloud_print/gcd_api_flow_unittest.cc [modify] https://crrev.com/8f0249ae91750468210e7047a54e6cb76ed6d737/services/identity/public/cpp/access_token_fetcher.cc [modify] https://crrev.com/8f0249ae91750468210e7047a54e6cb76ed6d737/services/identity/public/cpp/access_token_fetcher_unittest.cc [modify] https://crrev.com/8f0249ae91750468210e7047a54e6cb76ed6d737/services/identity/public/cpp/access_token_info.cc [modify] https://crrev.com/8f0249ae91750468210e7047a54e6cb76ed6d737/services/identity/public/cpp/access_token_info.h [modify] https://crrev.com/8f0249ae91750468210e7047a54e6cb76ed6d737/services/identity/public/cpp/primary_account_access_token_fetcher_unittest.cc
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/844ab293fe660ce60031970a4bd83eab8e6650f5 commit 844ab293fe660ce60031970a4bd83eab8e6650f5 Author: Mario Sanchez Prada <mario@igalia.com> Date: Thu Oct 04 13:46:32 2018 Test id_token with AccessTokenFetcher and PrimaryAccountAccessTokenFetcher Add new overloads to FakeProfileOAuth2TokenService and IdentityTestEnvironment methods so that we can specify the |id_token| that should be returned along with a token when fetched via AccessTokenFetcher, and use that to check that such information is indeed passed along from the relevant unit tests. Bug: 889764 Change-Id: I593dca07362ce7392d2cd2439369f1b440deb1b3 Reviewed-on: https://chromium-review.googlesource.com/c/1249086 Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Mario Sanchez Prada <mario@igalia.com> Cr-Commit-Position: refs/heads/master@{#596649} [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/components/signin/core/browser/fake_profile_oauth2_token_service.cc [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/components/signin/core/browser/fake_profile_oauth2_token_service.h [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/services/identity/public/cpp/DEPS [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/services/identity/public/cpp/access_token_fetcher_unittest.cc [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/services/identity/public/cpp/identity_test_environment.h [modify] https://crrev.com/844ab293fe660ce60031970a4bd83eab8e6650f5/services/identity/public/cpp/primary_account_access_token_fetcher_unittest.cc
,
Oct 4
Resolving now |
||
►
Sign in to add a comment |
||
Comment 1 by blundell@chromium.org
, Sep 27Owner: ma...@igalia.com