diff --git a/nbgrader/converters/assign.py b/nbgrader/converters/assign.py index db21cd36d..717d6562f 100644 --- a/nbgrader/converters/assign.py +++ b/nbgrader/converters/assign.py @@ -42,7 +42,7 @@ class Assign(BaseConverter): @default("permissions") def _permissions_default(self): - return 664 if self.groupshared else 644 + return 664 if self.coursedir.groupshared else 644 @property def _input_directory(self): diff --git a/nbgrader/converters/base.py b/nbgrader/converters/base.py index 1ec717fc0..c0010a338 100644 --- a/nbgrader/converters/base.py +++ b/nbgrader/converters/base.py @@ -43,20 +43,9 @@ class BaseConverter(LoggingConfigurable): ) ).tag(config=True) - groupshared = Bool( - False, - help=dedent( - """ - Be less strict about user permissions (instructor files are by - default group writeable. Requires that admins ensure that primary - groups are correct! - """ - ) - ).tag(config=True) - @default("permissions") def _permissions_default(self): - return 664 if self.groupshared else 644 + return 664 if self.coursedir.groupshared else 644 coursedir = Instance(CourseDirectory, allow_none=True) @@ -261,7 +250,7 @@ def set_permissions(self, assignment_id, student_id): for filename in filenames: os.chmod(os.path.join(dirname, filename), permissions) # If groupshared, set dir permissions - see comment below. - if self.groupshared and os.stat(dirname).st_uid == os.getuid(): + if self.coursedir.groupshared and os.stat(dirname).st_uid == os.getuid(): #for subdirname in subdirnames: # subdirname = os.path.join(dirname, subdirname) try: @@ -273,7 +262,7 @@ def set_permissions(self, assignment_id, student_id): # nbconvert.writer, (unless there are supplementary files) with a # default mode of 755 and there is no way to pass the mode arguments # all the way to there! So we have to walk and fix. - if self.groupshared: + if self.coursedir.groupshared: # Root may be created in this step, and is not included above. rootdir = self.coursedir.format_path(self._output_directory, '.', '.') # Add 2770 to existing dir permissions (don't unconditionally override) diff --git a/nbgrader/coursedir.py b/nbgrader/coursedir.py index d3c72d8f6..2b4704f08 100644 --- a/nbgrader/coursedir.py +++ b/nbgrader/coursedir.py @@ -4,7 +4,7 @@ from textwrap import dedent from traitlets.config import LoggingConfigurable -from traitlets import Unicode, List, default, validate, TraitError +from traitlets import Unicode, List, Bool, default, validate, TraitError from .utils import full_split, parse_utc @@ -201,6 +201,17 @@ def _db_url_default(self): ) ).tag(config=True) + groupshared = Bool( + False, + help=dedent( + """ + Be less strict about user permissions (instructor files are by + default group writeable. Requires that admins ensure that primary + groups are correct! + """ + ) + ).tag(config=True) + @default("root") def _root_default(self): return os.getcwd() diff --git a/nbgrader/exchange/exchange.py b/nbgrader/exchange/exchange.py index 046d361fb..bfe107b9f 100644 --- a/nbgrader/exchange/exchange.py +++ b/nbgrader/exchange/exchange.py @@ -76,17 +76,6 @@ def _cache_default(self): coursedir = Instance(CourseDirectory, allow_none=True) - groupshared = Bool( - False, - help=dedent( - """ - Be less strict about user permissions (instructor files are by - default group writeable. Requires that admins ensure that primary - groups are correct! - """ - ) - ).tag(config=True) - def __init__(self, coursedir=None, **kwargs): self.coursedir = coursedir @@ -135,7 +124,7 @@ def do_copy(self, src, dest): shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*self.coursedir.ignore)) # copytree copies access mode too - so we must add go+rw back to it if # we are in groupshared. - if self.groupshared: + if self.coursedir.groupshared: for dirname, _, filenames in os.walk(dest): # dirs become ug+rwx try: @@ -154,7 +143,7 @@ def start(self): if sys.platform == 'win32': self.fail("Sorry, the exchange is not available on Windows.") - if not self.groupshared: + if not self.coursedir.groupshared: # This just makes sure that directory is o+rwx. In group shared # case, it is up to admins to ensure that instructors can write # there. diff --git a/nbgrader/exchange/release.py b/nbgrader/exchange/release.py index 50ae4373d..e3dd13b94 100644 --- a/nbgrader/exchange/release.py +++ b/nbgrader/exchange/release.py @@ -18,7 +18,7 @@ class ExchangeRelease(Exchange): force = Bool(False, help="Force overwrite existing files in the exchange.").tag(config=True) def ensure_root(self): - perms = S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH|((S_IWGRP|S_ISGID) if self.groupshared else 0) + perms = S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH|((S_IWGRP|S_ISGID)if self.coursedir.groupshared else 0) # if root doesn't exist, create it and set permissions if not os.path.exists(self.root): @@ -66,19 +66,19 @@ def init_dest(self): # groupshared: +2040 self.ensure_directory( self.course_path, - S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.groupshared else 0) + S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.coursedir.groupshared else 0) ) # 0755 # groupshared: +2040 self.ensure_directory( self.outbound_path, - S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.groupshared else 0) + S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.coursedir.groupshared else 0) ) # 0733 with set GID so student submission will have the instructors group # groupshared: +0040 self.ensure_directory( self.inbound_path, - S_ISGID|S_IRUSR|S_IWUSR|S_IXUSR|S_IWGRP|S_IXGRP|S_IWOTH|S_IXOTH|(S_IRGRP if self.groupshared else 0) + S_ISGID|S_IRUSR|S_IWUSR|S_IXUSR|S_IWGRP|S_IXGRP|S_IWOTH|S_IXOTH|(S_IRGRP if self.coursedir.groupshared else 0) ) def ensure_directory(self, path, mode): @@ -89,7 +89,7 @@ def ensure_directory(self, path, mode): # so we have to create and then chmod. os.chmod(path, mode) else: - if not self.groupshared and not self_owned(path): + if not self.coursedir.groupshared and not self_owned(path): self.fail("You don't own the directory: {}".format(path)) def copy_files(self): @@ -108,6 +108,6 @@ def copy_files(self): self.do_copy(self.src_path, self.dest_path) self.set_perms( self.dest_path, - fileperms=(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH|(S_IWGRP if self.groupshared else 0)), - dirperms=(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.groupshared else 0))) + fileperms=(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH|(S_IWGRP if self.coursedir.groupshared else 0)), + dirperms=(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.coursedir.groupshared else 0))) self.log.info("Released as: {} {}".format(self.course_id, self.coursedir.assignment_id))