Implementation of sensitive card information escrowing

chrome/browser/autofill/wallet/wallet_service_url.cc

Index: chrome/browser/autofill/wallet/wallet_service_url.cc
diff --git a/chrome/browser/autofill/wallet/wallet_service_url.cc b/chrome/browser/autofill/wallet/wallet_service_url.cc
index 811d9379f3c5257bfcd09a6a6e158af8a65ac56d..521dae1056567f5179a641d1e7f81174f93350f3 100644
--- a/chrome/browser/autofill/wallet/wallet_service_url.cc
+++ b/chrome/browser/autofill/wallet/wallet_service_url.cc
@@ -12,21 +12,33 @@
 
 namespace {
 
-const char kDefaultWalletServiceUrl[] = "https://wallet.google.com/online/v2/";
+const char kDefaultWalletServiceUrl[] = "https://wallet.google.com/";
 
 const char kDefaultWalletSecureServiceUrl[] =
-    "https://wallet.google.com/online-secure/temporarydata/cvv?s7e=cvv";
+    "https://wallet.google.com/";

[Ilya Sherman, 2013/01/09 23:30:48]

This constant is now identical to the one above it.  Why do you need both fo
them?

[ahutter, 2013/01/10 00:24:46]

In development they can be different.

[Ilya Sherman, 2013/01/10 00:35:40]

That sounds like reason to keep both the switches, but not both the constants.

 
 GURL GetBaseWalletUrl() {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  std::string base_wallet_service_url =
+  std::string base_wallet_service_host =

[Ilya Sherman, 2013/01/09 23:30:48]

nit: What is a "base host"?  Probably just name this variable
"wallet_service_hostname" if you don't like "url" in this variable name.

[ahutter, 2013/01/10 00:24:46]

Done.

       command_line.GetSwitchValueASCII(switches::kWalletServiceUrl);
-  return !base_wallet_service_url.empty() ? GURL(base_wallet_service_url) :
-                                            GURL(kDefaultWalletServiceUrl);
+  return !base_wallet_service_host.empty() ? GURL(base_wallet_service_host) :
+                                             GURL(kDefaultWalletServiceUrl);
+}
+
+GURL GetPrefixedWalletUrl() {

[Ilya Sherman, 2013/01/09 23:30:48]

Optional nit: Perhaps name this function "GetBaseWalletUrl", and the other
function "GetWalletHostUrl"?

[ahutter, 2013/01/10 00:24:46]

Done.

+  return GetBaseWalletUrl().Resolve("online/v2/");
 }
 
 GURL GetBaseAutocheckoutUrl() {
-  return GetBaseWalletUrl().Resolve("wallet/autocheckout/");
+  return GetPrefixedWalletUrl().Resolve("wallet/autocheckout/");
+}
+
+GURL GetBaseSecureUrl() {
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  std::string wallet_secure_url =
+      command_line.GetSwitchValueASCII(switches::kWalletSecureServiceUrl);
+  return !wallet_secure_url.empty() ? GURL(wallet_secure_url) :
+                                      GURL(kDefaultWalletSecureServiceUrl);
 }
 
 }  // anonymous namespace
@@ -38,8 +50,7 @@ namespace wallet {
 const char kApiKey[] = "abcdefg";
 
 GURL GetGetWalletItemsUrl() {
-  return GetBaseWalletUrl().Resolve(
-      "wallet/autocheckout/getWalletItemsJwtless");
+  return GetBaseAutocheckoutUrl().Resolve("getWalletItemsJwtless");

[Raman Kakilate, 2013/01/09 22:54:21]

Can you DCHECK if the resolved GURL is valid. Here and other places ?

[Ilya Sherman, 2013/01/09 23:30:48]

What would cause it to be invalid?

[Albert Bodenhamer, 2013/01/09 23:58:43]

You could get a bad URL if you passed in a bad command line switch.  That should
NOT cause a DCHECK.  DLOG would be fine, if you think it will be a problem.

[ahutter, 2013/01/10 00:24:46]

I don't think this is something we need to worry about too much.  If the command
line switch is bad all requests will 404 and be pretty obvious.

 }
 
 GURL GetGetFullWalletUrl() {
@@ -59,15 +70,15 @@ GURL GetSaveToWalletUrl() {
 }
 
 GURL GetPassiveAuthUrl() {
-  return GetBaseWalletUrl().Resolve("passiveauth");
+  return GetPrefixedWalletUrl().Resolve("passiveauth");
 }
 
-GURL GetSecureUrl() {
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  std::string wallet_secure_url =
-      command_line.GetSwitchValueASCII(switches::kWalletSecureServiceUrl);
-  return !wallet_secure_url.empty() ? GURL(wallet_secure_url) :
-                                      GURL(kDefaultWalletSecureServiceUrl);
+GURL GetEncryptionUrl() {
+  return GetBaseWalletUrl().Resolve("online-secure/temporarydata/cvv?s7e=cvv");

[Ilya Sherman, 2013/01/09 23:30:48]

I'm confused.  Should this not be GetBaseSecureUrl?

[ahutter, 2013/01/10 00:24:46]

OTP encryption actually does go through Sugar servers. It was my bad before for
thinking it didn't. I updated the comment in chrome_switches.cc to reflect this.

+}
+
+GURL GetEscrowUrl() {
+  return GetBaseSecureUrl().Resolve("dehEfe?s7e=cardNumber%3Bcvv");
 }
 
 }  // namespace wallet