Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
197313004:
Support policy registration using a preobtained access token. (Closed)
|
Created:
6 years, 9 months ago by dconnelly Modified:
6 years, 7 months ago Reviewers:
Joao da Silva CC:
chromium-reviews Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionSupport policy registration using a preobtained access token.
This allows platforms that obtain OAuth access tokens from external APIs
to use CloudPolicyClientRegistrationHelper to register policy clients.
BUG=318803
R=joaodasilva@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=267130
Patch Set 1 #
Total comments: 4
Patch Set 2 : cleanups #Patch Set 3 : rebase for real #Patch Set 4 : update UPSS on iOS #Patch Set 5 : allow registering policy clieint while signed in on iOS #Patch Set 6 : rebase #Patch Set 7 : rebase #
Messages
Total messages: 42 (0 generated)
lgtm https://codereview.chromium.org/197313004/diff/1/components/policy/core/commo... File components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc (right): https://codereview.chromium.org/197313004/diff/1/components/policy/core/commo... components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc:7: #include <vector> Already in .h https://codereview.chromium.org/197313004/diff/1/components/policy/core/commo... components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc:226: scopes.push_back(kServiceScopeGetUserInfo); While you're here: replace this constant with GaiaConstants::kOAuthWrapBridgeUserInfoScope
https://codereview.chromium.org/197313004/diff/1/components/policy/core/commo... File components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc (right): https://codereview.chromium.org/197313004/diff/1/components/policy/core/commo... components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc:7: #include <vector> On 2014/03/13 13:07:41, Joao da Silva wrote: > Already in .h Done. https://codereview.chromium.org/197313004/diff/1/components/policy/core/commo... components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc:226: scopes.push_back(kServiceScopeGetUserInfo); On 2014/03/13 13:07:41, Joao da Silva wrote: > While you're here: replace this constant with > GaiaConstants::kOAuthWrapBridgeUserInfoScope Done.
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/60001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
Hunk #4 FAILED at 129.
Hunk #5 succeeded at 204 (offset -1 lines).
1 out of 5 hunks FAILED -- saving rejects to file
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc.rej
Patch:
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
Index:
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
diff --git
a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
index
065a814ca674dd283da2d752ae1fbd0a81bbcb0a..2a0655854b526af69e750066894e6ab4b2a923a8
100644
---
a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
+++
b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
@@ -4,8 +4,6 @@
#include
"components/policy/core/common/cloud/cloud_policy_client_registration_helper.h"
-#include <vector>
-
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/logging.h"
@@ -24,10 +22,6 @@
namespace policy {
-// OAuth2 scope for the userinfo service.
-const char kServiceScopeGetUserInfo[] =
- "https://www.googleapis.com/auth/userinfo.email";
-
// The key under which the hosted-domain value is stored in the UserInfo
// response.
const char kGetHostedDomainKey[] = "hd";
@@ -77,7 +71,7 @@ void
CloudPolicyClientRegistrationHelper::TokenServiceHelper::FetchAccessToken(
OAuth2TokenService::ScopeSet scopes;
scopes.insert(GaiaConstants::kDeviceManagementServiceOAuth);
- scopes.insert(kServiceScopeGetUserInfo);
+ scopes.insert(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
token_request_ = token_service->StartRequest(account_id, scopes, this);
}
@@ -135,15 +129,12 @@ void
CloudPolicyClientRegistrationHelper::LoginTokenHelper::FetchAccessToken(
// userinfo services.
oauth2_access_token_fetcher_.reset(
new OAuth2AccessTokenFetcherImpl(this, context));
- std::vector<std::string> scopes;
- scopes.push_back(GaiaConstants::kDeviceManagementServiceOAuth);
- scopes.push_back(kServiceScopeGetUserInfo);
GaiaUrls* gaia_urls = GaiaUrls::GetInstance();
oauth2_access_token_fetcher_->Start(
gaia_urls->oauth2_chrome_client_id(),
gaia_urls->oauth2_chrome_client_secret(),
login_refresh_token,
- scopes);
+ GetScopes());
}
void CloudPolicyClientRegistrationHelper::LoginTokenHelper::OnGetTokenSuccess(
@@ -211,6 +202,24 @@ void
CloudPolicyClientRegistrationHelper::StartRegistrationWithLoginToken(
base::Bind(&CloudPolicyClientRegistrationHelper::OnTokenFetched,
base::Unretained(this)));
}
+
+void CloudPolicyClientRegistrationHelper::StartRegistrationWithAccessToken(
+ const std::string& access_token,
+ const base::Closure& callback) {
+ DCHECK(!client_->is_registered());
+ callback_ = callback;
+ client_->AddObserver(this);
+ OnTokenFetched(access_token);
+}
+
+// static
+std::vector<std::string>
+CloudPolicyClientRegistrationHelper::GetScopes() {
+ std::vector<std::string> scopes;
+ scopes.push_back(GaiaConstants::kDeviceManagementServiceOAuth);
+ scopes.push_back(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
+ return scopes;
+}
#endif
void CloudPolicyClientRegistrationHelper::OnTokenFetched(
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/70001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
Hunk #4 FAILED at 129.
Hunk #5 succeeded at 204 (offset -1 lines).
1 out of 5 hunks FAILED -- saving rejects to file
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc.rej
Patch:
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
Index:
components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
diff --git
a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
index
065a814ca674dd283da2d752ae1fbd0a81bbcb0a..2a0655854b526af69e750066894e6ab4b2a923a8
100644
---
a/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
+++
b/components/policy/core/common/cloud/cloud_policy_client_registration_helper.cc
@@ -4,8 +4,6 @@
#include
"components/policy/core/common/cloud/cloud_policy_client_registration_helper.h"
-#include <vector>
-
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/logging.h"
@@ -24,10 +22,6 @@
namespace policy {
-// OAuth2 scope for the userinfo service.
-const char kServiceScopeGetUserInfo[] =
- "https://www.googleapis.com/auth/userinfo.email";
-
// The key under which the hosted-domain value is stored in the UserInfo
// response.
const char kGetHostedDomainKey[] = "hd";
@@ -77,7 +71,7 @@ void
CloudPolicyClientRegistrationHelper::TokenServiceHelper::FetchAccessToken(
OAuth2TokenService::ScopeSet scopes;
scopes.insert(GaiaConstants::kDeviceManagementServiceOAuth);
- scopes.insert(kServiceScopeGetUserInfo);
+ scopes.insert(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
token_request_ = token_service->StartRequest(account_id, scopes, this);
}
@@ -135,15 +129,12 @@ void
CloudPolicyClientRegistrationHelper::LoginTokenHelper::FetchAccessToken(
// userinfo services.
oauth2_access_token_fetcher_.reset(
new OAuth2AccessTokenFetcherImpl(this, context));
- std::vector<std::string> scopes;
- scopes.push_back(GaiaConstants::kDeviceManagementServiceOAuth);
- scopes.push_back(kServiceScopeGetUserInfo);
GaiaUrls* gaia_urls = GaiaUrls::GetInstance();
oauth2_access_token_fetcher_->Start(
gaia_urls->oauth2_chrome_client_id(),
gaia_urls->oauth2_chrome_client_secret(),
login_refresh_token,
- scopes);
+ GetScopes());
}
void CloudPolicyClientRegistrationHelper::LoginTokenHelper::OnGetTokenSuccess(
@@ -211,6 +202,24 @@ void
CloudPolicyClientRegistrationHelper::StartRegistrationWithLoginToken(
base::Bind(&CloudPolicyClientRegistrationHelper::OnTokenFetched,
base::Unretained(this)));
}
+
+void CloudPolicyClientRegistrationHelper::StartRegistrationWithAccessToken(
+ const std::string& access_token,
+ const base::Closure& callback) {
+ DCHECK(!client_->is_registered());
+ callback_ = callback;
+ client_->AddObserver(this);
+ OnTokenFetched(access_token);
+}
+
+// static
+std::vector<std::string>
+CloudPolicyClientRegistrationHelper::GetScopes() {
+ std::vector<std::string> scopes;
+ scopes.push_back(GaiaConstants::kDeviceManagementServiceOAuth);
+ scopes.push_back(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
+ return scopes;
+}
#endif
void CloudPolicyClientRegistrationHelper::OnTokenFetched(
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/110001
The CQ bit was unchecked by dconnelly@chromium.org
On 2014/03/13 14:19:56, dconnelly wrote: > The CQ bit was unchecked by mailto:dconnelly@chromium.org joaodasilva: PTAL. I've added changes to UPSS.
lgtm
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/130001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on linux_chromium_chromeos_rel
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/130001
The CQ bit was unchecked by dconnelly@chromium.org
Daniel, what's the status of this CL?
On 2014/04/28 11:29:53, Joao da Silva wrote: > Daniel, what's the status of this CL? The downstream stuff went in on Friday, so I'll land this today.
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/190001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on chromium_presubmit
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/190001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/210001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium linux_chromium_chromeos_rel on tryserver.chromium
The CQ bit was checked by dconnelly@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/197313004/210001
Message was sent while issue was closed.
Committed patchset #7 manually as r267130 (presubmit successful).
Message was sent while issue was closed.
Failed to apply patch for
chrome/browser/policy/cloud/user_policy_signin_service_base.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file chrome/browser/policy/cloud/user_policy_signin_service_base.cc
Hunk #1 FAILED at 151.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/policy/cloud/user_policy_signin_service_base.cc.rej
Patch: chrome/browser/policy/cloud/user_policy_signin_service_base.cc
Index: chrome/browser/policy/cloud/user_policy_signin_service_base.cc
diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_base.cc
b/chrome/browser/policy/cloud/user_policy_signin_service_base.cc
index
2125ca4f4d43c9c1bc325e1621421508c69b06ac..6cfb58182a81b01b6f19e5205484e015daf907b8
100644
--- a/chrome/browser/policy/cloud/user_policy_signin_service_base.cc
+++ b/chrome/browser/policy/cloud/user_policy_signin_service_base.cc
@@ -151,7 +151,11 @@
UserPolicySigninServiceBase::CreateClientForRegistrationOnly(
const std::string& username) {
DCHECK(!username.empty());
// We should not be called with a client already initialized.
+#if !defined(OS_IOS)
+ // On iOS we check if an account has policy while the profile is signed in
+ // to another account.
DCHECK(!policy_manager() || !policy_manager()->core()->client());
+#endif
// If the user should not get policy, just bail out.
if (!policy_manager() || !ShouldLoadPolicyForUser(username)) {
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/257373002/ by dconnelly@chromium.org. The reason for reverting is: accident. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
