Skip to content

Commit

Permalink
POST_comment: More granular scope checking.
Browse files Browse the repository at this point in the history
The `submit` scope was required to reply to messages, which instead
should have been the job of `privatemessages`.
  • Loading branch information
kjoconnor committed Aug 6, 2015
1 parent c362f1c commit 9ccda2c
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
14 changes: 10 additions & 4 deletions r2/r2/controllers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@ def POST_editusertext(self, form, jquery, item, text):
jquery(".content").replace_things(item, True, True, wrap = wrapper)
jquery(".content .link .rank").hide()

@require_oauth2_scope("submit")
@allow_oauth2_access
@validatedForm(
VUser(),
VModhash(),
Expand All @@ -1812,9 +1812,9 @@ def POST_comment(self, commentform, jquery, parent, comment):
`parent` is the fullname of the thing being replied to. Its value
changes the kind of object created by this request:
* the fullname of a Link: a top-level comment in that Link's thread.
* the fullname of a Comment: a comment reply to that comment.
* the fullname of a Message: a message reply to that message.
* the fullname of a Link: a top-level comment in that Link's thread. (requires `submit` scope)
* the fullname of a Comment: a comment reply to that comment. (requires `submit` scope)
* the fullname of a Message: a message reply to that message. (requires `privatemessages` scope)
`text` should be the raw markdown body of the comment or message.
Expand All @@ -1825,13 +1825,19 @@ def POST_comment(self, commentform, jquery, parent, comment):
#check the parent type here cause we need that for the
#ratelimit checks
if isinstance(parent, Message):
if (c.oauth_user and not
c.oauth_scope.has_any_scope({'privatemessages', 'submit'})):
abort(403, 'forbidden')
if not getattr(parent, "repliable", True):
abort(403, 'forbidden')
if not parent.can_view_slow():
abort(403, 'forbidden')
is_message = True
should_ratelimit = False
else:
if (c.oauth_user and not
c.oauth_scope.has_access(c.site.name, {'submit'})):
abort(403, 'forbidden')
is_message = False
if isinstance(parent, Link):
link = parent
Expand Down
3 changes: 3 additions & 0 deletions r2/r2/models/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,9 @@ def has_access(self, subreddit, required_scopes):
return False
return (self.scopes >= required_scopes)

def has_any_scope(self, required_scopes):
return bool(self.scopes & required_scopes)

def is_valid(self):
return all(scope in self.scope_info for scope in self.scopes)

Expand Down

0 comments on commit 9ccda2c

Please sign in to comment.