Fix config permissions on the config directory and the config file

st2client/st2client/base.py

@@ -225,7 +225,7 @@ def _get_cached_auth_token(self, client, username, password):
         :rtype: ``str``
         """
         if not os.path.isdir(ST2_CONFIG_DIRECTORY):
-            os.makedirs(ST2_CONFIG_DIRECTORY)
+            os.makedirs(ST2_CONFIG_DIRECTORY, mode=0o770)
 
         cached_token_path = self._get_cached_token_path_for_user(username=username)
 
@@ -253,7 +253,7 @@ def _get_cached_auth_token(self, client, username, password):
         file_st_mode = oct(os.stat(cached_token_path).st_mode & 0o777)
         others_st_mode = int(file_st_mode[-1])
 
-        if others_st_mode >= 4:
+        if others_st_mode >= 2:
             # Every user has access to this file which is dangerous
             message = ('Permissions (%s) for cached token file "%s" are to permissive. Please '
                        'restrict the permissions and make sure only your own user can read '
@@ -290,7 +290,7 @@ def _cache_auth_token(self, token_obj):
         :type token_obj: ``object``
         """
         if not os.path.isdir(ST2_CONFIG_DIRECTORY):
-            os.makedirs(ST2_CONFIG_DIRECTORY)
+            os.makedirs(ST2_CONFIG_DIRECTORY, mode=0o770)
 
         username = token_obj.user
         cached_token_path = self._get_cached_token_path_for_user(username=username)
@@ -326,9 +326,10 @@ def _cache_auth_token(self, token_obj):
         # open + chmod are two operations which means that during a short time frame (between
         # open and chmod) when file can potentially be read by other users if the default
         # permissions used during create allow that.
-        fd = os.open(cached_token_path, os.O_WRONLY | os.O_CREAT, 0o600)
+        fd = os.open(cached_token_path, os.O_WRONLY | os.O_CREAT, 0o660)
         with os.fdopen(fd, 'w') as fp:
             fp.write(data)
+        os.chmod(cached_token_path, 0o660)
 
         self.LOG.debug('Token has been cached in "%s"' % (cached_token_path))
         return True

st2client/st2client/commands/auth.py

@@ -140,6 +140,7 @@ def run(self, args, **kwargs):
 
         with open(config_file, 'w') as cfg_file_out:
             config.write(cfg_file_out)
+        os.chmod(config_file, mode=0o660)
 
         return manager
 

st2client/st2client/config_parser.py

@@ -20,6 +20,7 @@
 from __future__ import absolute_import
 
 import os
+import warnings
 
 from collections import defaultdict
 
@@ -138,6 +139,20 @@ def parse(self):
             # Config doesn't exist, return the default values
             return CONFIG_DEFAULT_VALUES
 
+        config_dir_path = os.path.dirname(self.config_file_path)
+
+        if bool(os.stat(config_dir_path).st_mode ^ 0o770):
+            warnings.warn('Setting StackStorm config directory permissions '
+                          '(%s) to 0770' % config_dir_path)
+            os.chmod(config_dir_path, 0o770)
+            if not bool(os.stat(config_dir_path).st_mode ^ 0o770):
+                raise ValueError('Unable to set config directory permssions')
+
+        if bool(os.stat(self.config_file_path).st_mode ^ 0o660):
+            warnings.warn('Setting StackStorm config file permissions (%s) '
+                          'to 0660' % self.config_file_path)
+            os.chmod(self.config_file_path, 0o660)
+
         config = ConfigParser()
         with io.open(self.config_file_path, 'r', encoding='utf8') as fp:
             config.readfp(fp)

st2client/tests/unit/test_auth.py

@@ -61,6 +61,7 @@ class TestLoginBase(base.BaseCLITestCase):
 
     DOTST2_PATH = os.path.expanduser('~/.st2/')
     CONFIG_FILE = tempfile.mkstemp(suffix='st2.conf')
+    CONFIG_DIR = tempfile.mkdtemp()
     CONFIG_CONTENTS = """
     [credentials]
     username = olduser
@@ -102,7 +103,7 @@ def tearDown(self):
 
 class TestLoginPasswordAndConfig(TestLoginBase):
 
-    CONFIG_FILE = '/tmp/logintest.cfg'
+    CONFIG_FILE = os.path.join(TestLoginBase.CONFIG_DIR, 'logintest.cfg')
 
     TOKEN = {
         'user': 'st2admin',
@@ -141,7 +142,7 @@ def runTest(self):
 
 class TestLoginIntPwdAndConfig(TestLoginBase):
 
-    CONFIG_FILE = '/tmp/logintest.cfg'
+    CONFIG_FILE = os.path.join(TestLoginBase.CONFIG_DIR, 'logintest.cfg')
 
     TOKEN = {
         'user': 'st2admin',
@@ -175,6 +176,9 @@ def runTest(self, mock_gp):
         }
         requests.post.assert_called_with('http://127.0.0.1:9100/tokens', '{}', **expected_kwargs)
 
+        # Check file permissions
+        self.assertEqual(os.stat(self.CONFIG_FILE).st_mode & 0o777, 0o660)
+
         with open(self.CONFIG_FILE, 'r') as config_file:
             for line in config_file.readlines():
                 # Make sure certain values are not present
@@ -203,7 +207,7 @@ def runTest(self, mock_gp):
 
 class TestLoginWritePwdOkay(TestLoginBase):
 
-    CONFIG_FILE = '/tmp/logintest.cfg'
+    CONFIG_FILE = os.path.join(TestLoginBase.CONFIG_DIR, 'logintest.cfg')
 
     TOKEN = {
         'user': 'st2admin',
@@ -244,7 +248,7 @@ def runTest(self, mock_gp):
 
 class TestLoginUncaughtException(TestLoginBase):
 
-    CONFIG_FILE = '/tmp/logintest.cfg'
+    CONFIG_FILE = os.path.join(TestLoginBase.CONFIG_DIR, 'logintest.cfg')
 
     TOKEN = {
         'user': 'st2admin',

st2client/tests/unit/test_config_parser.py

@@ -16,6 +16,8 @@
 
 from __future__ import absolute_import
 import os
+import shutil
+import warnings
 
 import six
 import unittest2
@@ -37,6 +39,44 @@ def test_constructor(self):
         self.assertRaises(ValueError, CLIConfigParser, config_file_path='doestnotexist',
                           validate_config_exists=True)
 
+    def test_fix_permssions_on_existing_config(self):
+        TEMP_FILE_PATH = os.path.join('st2config', '.st2', 'config')
+        TEMP_CONFIG_DIR = os.path.dirname(TEMP_FILE_PATH)
+
+        self.assertFalse(os.path.exists(TEMP_CONFIG_DIR))
+
+        try:
+            os.makedirs(TEMP_CONFIG_DIR)
+
+            shutil.copyfile(CONFIG_FILE_PATH_FULL, TEMP_FILE_PATH)
+
+            self.assertNotEqual(os.stat(TEMP_FILE_PATH).st_mode & 0o777, 0o770)
+
+            parser = CLIConfigParser(config_file_path=TEMP_FILE_PATH, validate_config_exists=True)
+
+            with warnings.catch_warnings(record=True) as warnings_list:
+                result = parser.parse()
+
+                self.assertEquals(
+                    "Setting StackStorm config directory permissions ({}) to 0770".format(TEMP_CONFIG_DIR),
+                    str(warnings_list[0].message))
+
+                self.assertEqual(
+                    "Setting StackStorm config file permissions ({}) to 0660".format(TEMP_FILE_PATH),
+                    str(warnings_list[1].message))
+
+            self.assertTrue(os.path.exists(TEMP_FILE_PATH))
+            self.assertEqual(os.stat(TEMP_FILE_PATH).st_mode & 0o777, 0o660)
+
+            self.assertTrue(os.path.exists(TEMP_CONFIG_DIR))
+            self.assertEqual(os.stat(TEMP_CONFIG_DIR).st_mode & 0o777, 0o770)
+        finally:
+            if os.path.exists(TEMP_FILE_PATH):
+                os.remove(TEMP_FILE_PATH)
+                os.removedirs(TEMP_CONFIG_DIR)
+
+            self.assertFalse(os.path.exists(TEMP_FILE_PATH))
+
     def test_parse(self):
         # File doesn't exist
         parser = CLIConfigParser(config_file_path='doesnotexist', validate_config_exists=False)