From 9fb63da3c7a2898b0ace22107c7268bb984e69bb Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Thu, 24 Apr 2014 09:56:42 -0700
Subject: [PATCH] Handle unexpected keys for pending OAuth credentials.

Duplicate keys aren't expected, but in case something weird happens,
just override the previous information associated with that key. We
simply insert nothing for non-string keys (e.g. an OAuth flow with no
`state` parameter, which should never happen normally).
---
 packages/oauth/oauth_tests.js                 | 19 +++++++++++++++
 packages/oauth/pending_credentials.js         | 12 ++++++++--
 .../oauth1/oauth1_pending_request_tokens.js   | 15 +++++++++---
 packages/oauth1/oauth1_tests.js               | 24 +++++++++++++++++++
 4 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/packages/oauth/oauth_tests.js b/packages/oauth/oauth_tests.js
index d560ae10d3..0984ab57ad 100644
--- a/packages/oauth/oauth_tests.js
+++ b/packages/oauth/oauth_tests.js
@@ -28,3 +28,22 @@ Tinytest.add("oauth - pendingCredential handles Meteor.Errors", function (test)
   test.equal(result.stack, testError.stack);
   test.isUndefined(result.meteorError);
 });
+
+Tinytest.add("oauth - null, undefined key for pendingCredential", function (test) {
+  var cred = Random.id();
+  test.throws(function () {
+    OAuth._storePendingCredential(null, cred);
+  });
+  test.throws(function () {
+    OAuth._storePendingCredential(undefined, cred);
+  });
+});
+
+Tinytest.add("oauth - pendingCredential handles duplicate key", function (test) {
+  var key = Random.id();
+  var cred = Random.id();
+  OAuth._storePendingCredential(key, cred);
+  var newCred = Random.id();
+  OAuth._storePendingCredential(key, newCred);
+  test.equal(OAuth._retrievePendingCredential(key), newCred);
+});
diff --git a/packages/oauth/pending_credentials.js b/packages/oauth/pending_credentials.js
index e841b06124..d5197a8d53 100644
--- a/packages/oauth/pending_credentials.js
+++ b/packages/oauth/pending_credentials.js
@@ -31,19 +31,27 @@ var _cleanStaleResults = function() {
 var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000);
 
 
-// Stores the key and credential in the _pendingCredentials collection
+// Stores the key and credential in the _pendingCredentials collection.
+// Will throw an exception if `key` is not a string.
 //
 // @param key {string}
 // @param credential {string}   The credential to store
 //
 OAuth._storePendingCredential = function (key, credential) {
+  check(key, String);
+
   if (credential instanceof Error) {
     credential = storableError(credential);
   } else {
     credential = OAuth.sealSecret(credential);
   }
 
-  OAuth._pendingCredentials.insert({
+  // We do an upsert here instead of an insert in case the user happens
+  // to somehow send the same `state` parameter twice during an OAuth
+  // login; we don't want a duplicate key error.
+  OAuth._pendingCredentials.upsert({
+    key: key
+  }, {
     key: key,
     credential: credential,
     createdAt: new Date()
diff --git a/packages/oauth1/oauth1_pending_request_tokens.js b/packages/oauth1/oauth1_pending_request_tokens.js
index 76e8d88401..ff025a577f 100644
--- a/packages/oauth1/oauth1_pending_request_tokens.js
+++ b/packages/oauth1/oauth1_pending_request_tokens.js
@@ -15,7 +15,8 @@
 // For this reason, the _pendingRequestTokens are stored in the database
 // so they can be shared across Meteor App servers.
 //
-
+// XXX This code is fairly similar to oauth/pending_credentials.js --
+// maybe we can combine them somehow.
 
 // Collection containing pending request tokens
 // Has key, requestToken, requestTokenSecret, and createdAt fields.
@@ -39,14 +40,22 @@ var _cleanStaleResults = function() {
 var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000);
 
 
-// Stores the key and request token in the _pendingRequestTokens collection
+// Stores the key and request token in the _pendingRequestTokens collection.
+// Will throw an exception if `key` is not a string.
 //
 // @param key {string}
 // @param requestToken {string}
 // @param requestTokenSecret {string}
 //
 OAuth._storeRequestToken = function (key, requestToken, requestTokenSecret) {
-  OAuth._pendingRequestTokens.insert({
+  check(key, String);
+
+  // We do an upsert here instead of an insert in case the user happens
+  // to somehow send the same `state` parameter twice during an OAuth
+  // login; we don't want a duplicate key error.
+  OAuth._pendingRequestTokens.upsert({
+    key: key
+  }, {
     key: key,
     requestToken: OAuth.sealSecret(requestToken),
     requestTokenSecret: OAuth.sealSecret(requestTokenSecret),
diff --git a/packages/oauth1/oauth1_tests.js b/packages/oauth1/oauth1_tests.js
index f85ead956f..3820a337e8 100644
--- a/packages/oauth1/oauth1_tests.js
+++ b/packages/oauth1/oauth1_tests.js
@@ -84,3 +84,27 @@ Tinytest.add("oauth1 - pendingCredential is stored and can be retrieved (with oa
     OAuthEncryption.loadKey(null);
   }
 });
+
+Tinytest.add("oauth1 - duplicate key for request token", function (test) {
+  var key = Random.id();
+  var token = Random.id();
+  var secret = Random.id();
+  OAuth._storeRequestToken(key, token, secret);
+  var newToken = Random.id();
+  var newSecret = Random.id();
+  OAuth._storeRequestToken(key, newToken, newSecret);
+  var result = OAuth._retrieveRequestToken(key);
+  test.equal(result.requestToken, newToken);
+  test.equal(result.requestTokenSecret, newSecret);
+});
+
+Tinytest.add("oauth1 - null, undefined key for request token", function (test) {
+  var token = Random.id();
+  var secret = Random.id();
+  test.throws(function () {
+    OAuth._storeRequestToken(null, token, secret);
+  });
+  test.throws(function () {
+    OAuth._storeRequestToken(undefined, token, secret);
+  });
+});