-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed bug w/ not checking permissions in copy #174
Changes from 3 commits
c4e22f6
585af53
dbf1327
8b75c7a
accbeeb
135f3e8
f89a312
b0b855c
77e7d79
030e424
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -173,7 +173,7 @@ def post(self, library): | |
- admin | ||
- write | ||
""" | ||
|
||
# Get the user requesting this from the header | ||
try: | ||
user_editing = self.helper_get_user_id() | ||
|
@@ -203,22 +203,8 @@ def post(self, library): | |
current_app.logger.error('Wrong type passed for POST: {0} [{1}]' | ||
.format(request.data, error)) | ||
return err(WRONG_TYPE_ERROR) | ||
|
||
if data['action'] in ['union', 'intersection', 'difference']: | ||
if 'libraries' not in data: | ||
return err(NO_LIBRARY_SPECIFIED_ERROR) | ||
if 'name' not in data: | ||
data['name'] = 'Untitled {0}.'.format(get_date().isoformat()) | ||
if 'public' not in data: | ||
data['public'] = False | ||
|
||
if data['action'] == 'copy': | ||
if 'libraries' not in data: | ||
return err(NO_LIBRARY_SPECIFIED_ERROR) | ||
if len(data['libraries']) > 1: | ||
return err(TOO_MANY_LIBRARIES_SPECIFIED_ERROR) | ||
|
||
lib_names = [] | ||
check_access = [] | ||
with current_app.session_scope() as session: | ||
primary = session.query(Library).filter_by(id=library_uuid).one() | ||
lib_names.append(primary.name) | ||
|
@@ -230,6 +216,32 @@ def post(self, library): | |
return err(BAD_LIBRARY_ID_ERROR) | ||
secondary = session.query(Library).filter_by(id=secondary_uuid).one() | ||
lib_names.append(secondary.name) | ||
check_access.append(secondary) | ||
|
||
if data['action'] in ['union', 'intersection', 'difference']: | ||
if 'libraries' not in data: | ||
return err(NO_LIBRARY_SPECIFIED_ERROR) | ||
for lib in check_access: | ||
lib = session.merge(lib) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Totally get it! The reason I used merge was because the libraries were getting detached from the session. I'm sure there's some other way around it, but I couldn't think of any off the top of my head. If you're okay with holding off on this until tomorrow, I can try to come up with something else. |
||
if lib.public or not self.read_access(service_uid=user_editing_uid, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops my comment wasn't very clear - this should be |
||
library_id=lib.id): | ||
return err(NO_PERMISSION_ERROR) | ||
if 'name' not in data: | ||
data['name'] = 'Untitled {0}.'.format(get_date().isoformat()) | ||
if 'public' not in data: | ||
data['public'] = False | ||
|
||
if data['action'] == 'copy': | ||
if 'libraries' not in data: | ||
return err(NO_LIBRARY_SPECIFIED_ERROR) | ||
if len(data['libraries']) > 1: | ||
return err(TOO_MANY_LIBRARIES_SPECIFIED_ERROR) | ||
# Check the permissions of the user | ||
if not self.write_access(service_uid=user_editing_uid, | ||
library_id=check_access[0].id): | ||
return err(NO_PERMISSION_ERROR) | ||
|
||
|
||
|
||
if data['action'] == 'union': | ||
bib_union = self.setops_libraries( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it matters for the test, but did you mean for these to both be the same library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it wasn't relevant so I added lib2. But I changed it now to lib1, just because that's more consistent with how it was before.