diff --git a/oauth2client/file.py b/oauth2client/file.py index d20cf6e..d71e888 100644 --- a/oauth2client/file.py +++ b/oauth2client/file.py @@ -20,6 +20,8 @@ credentials. __author__ = 'jcgregorio@google.com (Joe Gregorio)' +import os +import stat import threading from anyjson import simplejson @@ -56,7 +58,7 @@ class Storage(BaseStorage): """ credentials = None try: - f = open(self._filename, 'r') + f = open(self._filename, 'rb') content = f.read() f.close() except IOError: @@ -70,12 +72,28 @@ class Storage(BaseStorage): return credentials + def _create_file_if_needed(self): + """Create an empty file if necessary. + + This method will not initialize the file. Instead it implements a + simple version of "touch" to ensure the file has been created. + """ + if not os.path.exists(self._filename): + old_umask = os.umask(0177) + try: + open(self._filename, 'a+b').close() + finally: + os.umask(old_umask) + + def locked_put(self, credentials): """Write Credentials to file. Args: credentials: Credentials, the credentials to store. """ - f = open(self._filename, 'w') + + self._create_file_if_needed() + f = open(self._filename, 'wb') f.write(credentials.to_json()) f.close() diff --git a/oauth2client/multistore_file.py b/oauth2client/multistore_file.py index b6126f5..d9b89c8 100644 --- a/oauth2client/multistore_file.py +++ b/oauth2client/multistore_file.py @@ -32,6 +32,7 @@ The format of the stored data is like so: __author__ = 'jbeda@google.com (Joe Beda)' import base64 +import errno import fcntl import logging import os @@ -167,7 +168,7 @@ class _MultiStore(object): if not os.path.exists(self._filename): old_umask = os.umask(0177) try: - open(self._filename, 'a+').close() + open(self._filename, 'a+b').close() finally: os.umask(old_umask) @@ -175,12 +176,13 @@ class _MultiStore(object): """Lock the entire multistore.""" self._thread_lock.acquire() # Check to see if the file is writeable. - if os.access(self._filename, os.W_OK): - self._file_handle = open(self._filename, 'r+') + try: + self._file_handle = open(self._filename, 'r+b') fcntl.lockf(self._file_handle.fileno(), fcntl.LOCK_EX) - else: - # Cannot open in read/write mode. Open only in read mode. - self._file_handle = open(self._filename, 'r') + except IOError, e: + if e.errno != errno.EACCES: + raise e + self._file_handle = open(self._filename, 'rb') self._read_only = True if self._warn_on_readonly: logger.warn('The credentials file (%s) is not writable. Opening in ' diff --git a/tests/test_oauth2client_file.py b/tests/test_oauth2client_file.py index a34cce7..7ff1eeb 100644 --- a/tests/test_oauth2client_file.py +++ b/tests/test_oauth2client_file.py @@ -27,6 +27,7 @@ import datetime import httplib2 import os import pickle +import stat import tempfile import unittest @@ -134,6 +135,39 @@ class OAuth2ClientFileTests(unittest.TestCase): self.assertNotEquals(None, credentials) self.assertEquals('foo', credentials.access_token) + mode = os.stat(FILENAME).st_mode + + if os.name == 'posix': + self.assertEquals('0600', oct(stat.S_IMODE(os.stat(FILENAME).st_mode))) + + def test_read_only_file_fail_lock(self): + access_token = 'foo' + client_secret = 'cOuDdkfjxxnv+' + refresh_token = '1/0/a.df219fjls0' + token_expiry = datetime.datetime.utcnow() + token_uri = 'https://www.google.com/accounts/o8/oauth2/token' + user_agent = 'refresh_checker/1.0' + client_id = 'some_client_id' + + credentials = OAuth2Credentials( + access_token, client_id, client_secret, + refresh_token, token_expiry, token_uri, + user_agent) + + open(FILENAME, 'a+b').close() + os.chmod(FILENAME, 0400) + + store = multistore_file.get_credential_storage( + FILENAME, + credentials.client_id, + credentials.user_agent, + ['some-scope', 'some-other-scope']) + + store.put(credentials) + if os.name == 'posix': + self.assertTrue(store._multistore._read_only) + os.chmod(FILENAME, 0600) + def test_multistore_non_existent_file(self): store = multistore_file.get_credential_storage( @@ -171,5 +205,8 @@ class OAuth2ClientFileTests(unittest.TestCase): self.assertNotEquals(None, credentials) self.assertEquals('foo', credentials.access_token) + if os.name == 'posix': + self.assertEquals('0600', oct(stat.S_IMODE(os.stat(FILENAME).st_mode))) + if __name__ == '__main__': unittest.main()