White-listing Chrome Remote Desktop to use the identity API in Public Session

chrome/browser/extensions/api/identity/identity_api.cc

Index: chrome/browser/extensions/api/identity/identity_api.cc
diff --git a/chrome/browser/extensions/api/identity/identity_api.cc b/chrome/browser/extensions/api/identity/identity_api.cc
index 9aa4d7f65d9e8d2a400f1074ce54f7197921774c..820a592fa6a4c8ab0d7a178888fc9d41fdcd1a11 100644
--- a/chrome/browser/extensions/api/identity/identity_api.cc
+++ b/chrome/browser/extensions/api/identity/identity_api.cc
@@ -72,6 +72,14 @@ namespace {
 static const char kChromiumDomainRedirectUrlPattern[] =
     "https://%s.chromiumapp.org/";
 
+// The list of apps that are allowed to use the Identity API to retrieve the
+// token from the device robot account in a public session.
+const char* const kPublicSessionAllowedOrigins[] = {
+    // Chrome Remote Desktop - Chromium branding.
+    "chrome-extension://ljacajndfccfgnfohlgkdphmbnpkjflk/",
+    // Chrome Remote Desktop - Official branding.
+    "chrome-extension://gbchcmhmhahfdphkhkmpfmihenigjmpp/"};
+
 std::string GetPrimaryAccountId(content::BrowserContext* context) {
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetForProfile(Profile::FromBrowserContext(context));
@@ -371,7 +379,8 @@ bool IdentityGetAuthTokenFunction::RunAsync() {
 #if defined(OS_CHROMEOS)
   policy::BrowserPolicyConnectorChromeOS* connector =
       g_browser_process->platform_part()->browser_policy_connector_chromeos();
-  if (user_manager::UserManager::Get()->IsLoggedInAsKioskApp() &&
+  if ((user_manager::UserManager::Get()->IsLoggedInAsKioskApp() ||
+       IsOriginWhitelistedInPublicSession()) &&

[bartfab (slow), 2015/09/18 12:24:57]

You should first check whether this is a public session. Otherwise, you are
short-cutting the flow in lines 390 onward for the whitelisted apps when run in
a regular session.

[kelvinp, 2015/09/22 00:50:37]

Good point.  IsOriginWhitelistedInPublicSession() returns false when we are not
in a public session.  See line 768.
I did a quick check and the API does return the user's identity in a non-public
session so I think we are covered in this scenario.

[Michael Courage, 2015/09/22 03:09:02]

The Kiosk path is already kind of convoluted. It would be nice if it the code
could be more obviously correct. It's hard to follow because of the large
conditional here, almost but not quite replicated in StartMintToken, plus the
fact that IsOriginWhitelistedInPublicSession checks two things (1. is it a
public session, and 2. is this app allowed to operate in a public session).

A possible improvement:
Are non-whitelisted apps running in a public session ever supposed to get a
useful result out of getAuthToken? If not, I think it would be good to error out
right up top as we do for incognito on 315. With the whitelist taken care of up
front, you don't have to worry about it through the rest of the flow.

[bartfab (slow), 2015/09/23 12:37:42]

The order needs to be the opposite: Check whether we are in a public session
first and only if so, consult the whitelist. Otherwise, we would start refusing
non-whitelisted apps in non-public-sessions as well.

I guess we can collapse the kiosk and public session cases somewhat if we want
to:

if (enterprise_managed && (is_kiosk || is_public_session)) {
  if (is_public_session && !is_origin_whitelisted())
    return false;
  StartMintTokenFlow();
  return true;
}
     

[kelvinp, 2015/09/23 21:08:54]

I don't think there is a concern.  For regular sessions (non-public session),
IsOrginWhiteListedInPublicSession will return false. And it will go through the
normal flow in line 390.

       connector->IsEnterpriseManaged()) {
     StartMintTokenFlow(IdentityMintRequestQueue::MINT_TYPE_NONINTERACTIVE);
     return true;
@@ -495,7 +504,8 @@ void IdentityGetAuthTokenFunction::StartMintToken(
       case IdentityTokenCacheValue::CACHE_STATUS_NOTFOUND:
 #if defined(OS_CHROMEOS)
         // Always force minting token for ChromeOS kiosk app.
-        if (user_manager::UserManager::Get()->IsLoggedInAsKioskApp()) {
+        if (user_manager::UserManager::Get()->IsLoggedInAsKioskApp() ||
+            IsOriginWhitelistedInPublicSession()) {
           gaia_mint_token_mode_ = OAuth2MintTokenFlow::MODE_MINT_TOKEN_FORCE;
           policy::BrowserPolicyConnectorChromeOS* connector =
               g_browser_process->platform_part()
@@ -753,6 +763,22 @@ void IdentityGetAuthTokenFunction::StartDeviceLoginAccessTokenRequest() {
                             scopes,
                             this);
 }
+
+bool IdentityGetAuthTokenFunction::IsOriginWhitelistedInPublicSession() {
+  if (!user_manager::UserManager::Get()->IsLoggedInAsPublicAccount()) {
+    return false;
+  }
+
+  for (unsigned int i = 0; i < arraysize(kPublicSessionAllowedOrigins); i++) {
+    URLPattern allowed_origin(URLPattern::SCHEME_ALL,
+                              kPublicSessionAllowedOrigins[i]);
+    DCHECK(extension());
+    if (allowed_origin.MatchesSecurityOrigin(extension()->url())) {
+      return true;
+    }
+  }
+  return false;
+}
 #endif
 
 void IdentityGetAuthTokenFunction::StartLoginAccessTokenRequest() {