Skip to content

Commit

Permalink
only set the first parent dir as writable for user data, which is the…
Browse files Browse the repository at this point in the history
… minimum requirement
  • Loading branch information
cwcummings committed Aug 8, 2023
1 parent d647323 commit d84271e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 16 deletions.
22 changes: 12 additions & 10 deletions cowbird/handlers/impl/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,17 @@ def update_secure_data_proxy_path_perms(self, src_path: str, user_name: str) ->
return access_allowed

@staticmethod
def create_hardlink_path(src_path: str, hardlink_path: str, access_allowed: bool, dir_perms: int = 0o775) -> None:
def create_hardlink_path(src_path: str, hardlink_path: str, access_allowed: bool,
is_parent_dir_writable: bool = False) -> None:
"""
Creates a hardlink path from a source file, if the user has access rights.
"""
if access_allowed:
# set umask to allow all permissions, else, mode is ignored when making directories
previous_umask = os.umask(0)
os.makedirs(os.path.dirname(hardlink_path), mode=dir_perms, exist_ok=True)
os.umask(previous_umask) # reset umask to default
os.makedirs(os.path.dirname(hardlink_path), exist_ok=True)
apply_new_path_permissions(os.path.dirname(hardlink_path),
is_readable=True,
is_writable=is_parent_dir_writable,
is_executable=True)
LOGGER.debug("Creating hardlink from file `%s` to the path `%s`", src_path, hardlink_path)
try:
os.link(src_path, hardlink_path)
Expand All @@ -230,9 +232,9 @@ def _create_wpsoutputs_hardlink(self, src_path: str, overwrite: bool = False,
if not process_user_files:
return

# User workspace directories require `rwx` permissions to allow file modifications via JupyterLab for
# User workspace directories require write permissions to allow file modifications via JupyterLab for
# files with write permissions
dir_perms = 0o777
is_parent_dir_writable = True
magpie_handler = HandlerFactory().get_handler("Magpie")
user_name = magpie_handler.get_user_name_from_user_id(int(regex_match.group(2)))
hardlink_path = self.get_user_hardlink(src_path=src_path,
Expand All @@ -249,7 +251,7 @@ def _create_wpsoutputs_hardlink(self, src_path: str, overwrite: bool = False,
else: # public files
if not process_public_files:
return
dir_perms = 0o775
is_parent_dir_writable = False # public files are read-only
hardlink_path = self._get_public_hardlink(src_path)

if os.path.exists(hardlink_path):
Expand All @@ -261,7 +263,7 @@ def _create_wpsoutputs_hardlink(self, src_path: str, overwrite: bool = False,
"created file.", hardlink_path)
os.remove(hardlink_path)

self.create_hardlink_path(src_path, hardlink_path, access_allowed, dir_perms=dir_perms)
self.create_hardlink_path(src_path, hardlink_path, access_allowed, is_parent_dir_writable)

def on_created(self, path: str) -> None:
"""
Expand Down Expand Up @@ -393,7 +395,7 @@ def _update_permissions_on_filesystem(self, permission: Permission) -> None:
# Resync hardlink path
if os.path.exists(hardlink_path):
os.remove(hardlink_path)
self.create_hardlink_path(user_path, hardlink_path, access_allowed, dir_perms=0o777)
self.create_hardlink_path(user_path, hardlink_path, access_allowed, is_parent_dir_writable=True)

def permission_created(self, permission: Permission) -> None:
self._update_permissions_on_filesystem(permission)
Expand Down
9 changes: 3 additions & 6 deletions tests/test_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def test_public_wps_output_created(self):
TestFileSystem.check_created_test_cases(output_file, hardlink_path)

# Check that the hardlink's parent directory have the right permissions
utils.check_path_permissions(os.path.join(filesystem_handler.get_wps_outputs_public_dir(), bird_name), 0o775)
utils.check_path_permissions(os.path.dirname(hardlink_path), 0o775)

# A create event on a folder should not be processed (no corresponding target folder created)
target_dir = os.path.join(filesystem_handler.get_wps_outputs_public_dir(), bird_name)
Expand Down Expand Up @@ -464,11 +464,8 @@ def test_user_wps_output_created(self):

TestFileSystem.check_created_test_cases(self.output_file, self.hardlink_path)

# Check that the hardlink's parent directories have the right permissions
path_to_check = self.hardlink_path
while not path_to_check.endswith(USER_WPS_OUTPUTS_USER_DIR_NAME):
path_to_check = os.path.dirname(path_to_check)
utils.check_path_permissions(path_to_check, 0o777)
# Check that the hardlink's parent directory has the right permissions
utils.check_path_permissions(os.path.dirname(self.hardlink_path), 0o777)

# A create event on a folder should not be processed (no corresponding target folder created)
src_dir = os.path.join(self.wpsoutputs_dir, f"{self.bird_name}/users/{self.user_id}/{self.job_id}")
Expand Down

0 comments on commit d84271e

Please sign in to comment.