Set file permissions on credentials files.

Reviewed in http://codereview.appspot.com/5540053/.

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()

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 '

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()