Do not allow credentials files to be symlinks.
Reviewed in https://codereview.appspot.com/6476062/.
diff --git a/oauth2client/file.py b/oauth2client/file.py
index 1abc6d2..1895f94 100644
--- a/oauth2client/file.py
+++ b/oauth2client/file.py
@@ -29,6 +29,10 @@
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 @@
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 @@
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 @@
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()
diff --git a/oauth2client/locked_file.py b/oauth2client/locked_file.py
index 8f35c90..1cfe532 100644
--- a/oauth2client/locked_file.py
+++ b/oauth2client/locked_file.py
@@ -28,11 +28,20 @@
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 @@
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 @@
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 @@
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:
diff --git a/oauth2client/multistore_file.py b/oauth2client/multistore_file.py
index e190c6a..c919573 100644
--- a/oauth2client/multistore_file.py
+++ b/oauth2client/multistore_file.py
@@ -76,7 +76,7 @@
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(
diff --git a/tests/test_oauth2client_file.py b/tests/test_oauth2client_file.py
index 596a4b6..7c65677 100644
--- a/tests/test_oauth2client_file.py
+++ b/tests/test_oauth2client_file.py
@@ -32,12 +32,13 @@
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 @@
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 @@
# 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 @@
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 @@
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 @@
credentials = AccessTokenCredentials(access_token, user_agent)
- s = Storage(FILENAME)
+ s = file.Storage(FILENAME)
credentials = s.put(credentials)
credentials = s.get()
@@ -188,6 +202,22 @@
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(