Do not allow credentials files to be symlinks.

Reviewed in https://codereview.appspot.com/6476062/.

oauth2client/file.py

@@ -29,6 +29,10 @@ from client import Storage as BaseStorage
 from client import Credentials
 
 
+class CredentialsFileSymbolicLinkError(Exception):
+  """Credentials files must not be symbolic links."""
+
+
 class Storage(BaseStorage):
   """Store and retrieve a single credential to and from a file."""
 
@@ -36,6 +40,11 @@ class Storage(BaseStorage):
     self._filename = filename
     self._lock = threading.Lock()
 
+  def _validate_file(self):
+    if os.path.islink(self._filename):
+      raise CredentialsFileSymbolicLinkError(
+          'File: %s is a symbolic link.' % self._filename)
+
   def acquire_lock(self):
     """Acquires any lock necessary to access this Storage.
 
@@ -55,8 +64,12 @@ class Storage(BaseStorage):
 
     Returns:
       oauth2client.client.Credentials
+
+    Raises:
+      CredentialsFileSymbolicLinkError if the file is a symbolic link.
     """
     credentials = None
+    self._validate_file()
     try:
       f = open(self._filename, 'rb')
       content = f.read()
@@ -90,9 +103,13 @@ class Storage(BaseStorage):
 
     Args:
       credentials: Credentials, the credentials to store.
+
+    Raises:
+      CredentialsFileSymbolicLinkError if the file is a symbolic link.
     """
 
     self._create_file_if_needed()
+    self._validate_file()
     f = open(self._filename, 'wb')
     f.write(credentials.to_json())
     f.close()

oauth2client/locked_file.py

@@ -28,11 +28,20 @@ from oauth2client import util
 logger = logging.getLogger(__name__)
 
 
+class CredentialsFileSymbolicLinkError(Exception):
+  """Credentials files must not be symbolic links."""
+
+
 class AlreadyLockedException(Exception):
   """Trying to lock a file that has already been locked by the LockedFile."""
   pass
 
 
+def validate_file(filename):
+  if os.path.islink(filename):
+    raise CredentialsFileSymbolicLinkError(
+        'File: %s is a symbolic link.' % filename)
+
 class _Opener(object):
   """Base class for different locking primitives."""
 
@@ -91,12 +100,14 @@ class _PosixOpener(_Opener):
     Raises:
       AlreadyLockedException: if the lock is already acquired.
       IOError: if the open fails.
+      CredentialsFileSymbolicLinkError if the file is a symbolic link.
     """
     if self._locked:
       raise AlreadyLockedException('File %s is already locked' %
                                    self._filename)
     self._locked = False
 
+    validate_file(self._filename)
     try:
       self._fh = open(self._filename, self._mode)
     except IOError, e:
@@ -159,12 +170,14 @@ try:
       Raises:
         AlreadyLockedException: if the lock is already acquired.
         IOError: if the open fails.
+        CredentialsFileSymbolicLinkError if the file is a symbolic link.
       """
       if self._locked:
         raise AlreadyLockedException('File %s is already locked' %
                                      self._filename)
       start_time = time.time()
 
+      validate_file(self._filename)
       try:
         self._fh = open(self._filename, self._mode)
       except IOError, e:
@@ -232,12 +245,14 @@ try:
       Raises:
         AlreadyLockedException: if the lock is already acquired.
         IOError: if the open fails.
+        CredentialsFileSymbolicLinkError if the file is a symbolic link.
       """
       if self._locked:
         raise AlreadyLockedException('File %s is already locked' %
                                      self._filename)
       start_time = time.time()
 
+      validate_file(self._filename)
       try:
         self._fh = open(self._filename, self._mode)
       except IOError, e:

oauth2client/multistore_file.py

@@ -76,7 +76,7 @@ def get_credential_storage(filename, client_id, user_agent, scope,
     An object derived from client.Storage for getting/setting the
     credential.
   """
-  filename = os.path.realpath(os.path.expanduser(filename))
+  filename = os.path.expanduser(filename)
   _multistores_lock.acquire()
   try:
     multistore = _multistores.setdefault(

tests/test_oauth2client_file.py

@@ -32,12 +32,13 @@ import tempfile
 import unittest
 
 from apiclient.http import HttpMockSequence
+from oauth2client import file
+from oauth2client import locked_file
 from oauth2client import multistore_file
 from oauth2client.anyjson import simplejson
 from oauth2client.client import AccessTokenCredentials
 from oauth2client.client import AssertionCredentials
 from oauth2client.client import OAuth2Credentials
-from oauth2client.file import Storage
 
 
 FILENAME = tempfile.mktemp('oauth2client_test.data')
@@ -58,10 +59,23 @@ class OAuth2ClientFileTests(unittest.TestCase):
       pass
 
   def test_non_existent_file_storage(self):
-    s = Storage(FILENAME)
+    s = file.Storage(FILENAME)
     credentials = s.get()
     self.assertEquals(None, credentials)
 
+  def test_no_sym_link_credentials(self):
+    if hasattr(os, 'symlink'):
+      SYMFILENAME = FILENAME + '.sym'
+      os.symlink(FILENAME, SYMFILENAME)
+      s = file.Storage(SYMFILENAME)
+      try:
+        s.get()
+        self.fail('Should have raised an exception.')
+      except file.CredentialsFileSymbolicLinkError:
+        pass
+      finally:
+        os.unlink(SYMFILENAME)
+
   def test_pickle_and_json_interop(self):
     # Write a file with a pickled OAuth2Credentials.
     access_token = 'foo'
@@ -83,13 +97,13 @@ class OAuth2ClientFileTests(unittest.TestCase):
 
     # Storage should be not be able to read that object, as the capability to
     # read and write credentials as pickled objects has been removed.
-    s = Storage(FILENAME)
+    s = file.Storage(FILENAME)
     read_credentials = s.get()
     self.assertEquals(None, read_credentials)
 
     # Now write it back out and confirm it has been rewritten as JSON
     s.put(credentials)
-    f = file(FILENAME)
+    f = open(FILENAME)
     data = simplejson.load(f)
     f.close()
 
@@ -111,7 +125,7 @@ class OAuth2ClientFileTests(unittest.TestCase):
         refresh_token, token_expiry, token_uri,
         user_agent)
 
-    s = Storage(FILENAME)
+    s = file.Storage(FILENAME)
     s.put(credentials)
     credentials = s.get()
     new_cred = copy.copy(credentials)
@@ -135,7 +149,7 @@ class OAuth2ClientFileTests(unittest.TestCase):
         refresh_token, token_expiry, token_uri,
         user_agent)
 
-    s = Storage(FILENAME)
+    s = file.Storage(FILENAME)
     s.put(credentials)
     credentials = s.get()
     self.assertNotEquals(None, credentials)
@@ -149,7 +163,7 @@ class OAuth2ClientFileTests(unittest.TestCase):
 
     credentials = AccessTokenCredentials(access_token, user_agent)
 
-    s = Storage(FILENAME)
+    s = file.Storage(FILENAME)
     credentials = s.put(credentials)
     credentials = s.get()
 
@@ -188,6 +202,22 @@ class OAuth2ClientFileTests(unittest.TestCase):
       self.assertTrue(store._multistore._read_only)
     os.chmod(FILENAME, 0600)
 
+  def test_multistore_no_symbolic_link_files(self):
+    if hasattr(os, 'symlink'):
+      SYMFILENAME = FILENAME + 'sym'
+      os.symlink(FILENAME, SYMFILENAME)
+      store = multistore_file.get_credential_storage(
+          SYMFILENAME,
+          'some_client_id',
+          'user-agent/1.0',
+          ['some-scope', 'some-other-scope'])
+      try:
+        store.get()
+        self.fail('Should have raised an exception.')
+      except locked_file.CredentialsFileSymbolicLinkError:
+        pass
+      finally:
+        os.unlink(SYMFILENAME)
 
   def test_multistore_non_existent_file(self):
     store = multistore_file.get_credential_storage(