diff --git a/r2/r2/controllers/oauth2.py b/r2/r2/controllers/oauth2.py index 32f72891f..67ff42a97 100644 --- a/r2/r2/controllers/oauth2.py +++ b/r2/r2/controllers/oauth2.py @@ -31,36 +31,14 @@ from r2.lib.base import abort from reddit_base import RedditController, MinimalController, require_https from r2.lib.db.thing import NotFound from r2.models import Account -from r2.models.token import OAuth2Client, OAuth2AuthorizationCode, OAuth2AccessToken +from r2.models.token import ( + OAuth2Client, OAuth2AuthorizationCode, OAuth2AccessToken, OAuth2Scope) from r2.controllers.errors import ForbiddenError, errors from validator import validate, VRequired, VOneOf, VUser, VModhash, VOAuth2ClientID, VOAuth2Scope from r2.lib.pages import OAuth2AuthorizationPage from r2.lib.require import RequirementException, require, require_split from r2.lib.utils import parse_http_basic -scope_info = { - "identity": { - "id": "identity", - "name": _("My Identity"), - "description": _("Access my reddit username and signup date."), - }, - "comment": { - "id": "comment", - "name": _("Commenting"), - "description": _("Submit comments from my account."), - }, - "moderateflair": { - "id": "moderateflair", - "name": _("Moderating Flair"), - "description": _("Manage flair in subreddits I moderate."), - }, - "myreddits": { - "id": "myreddits", - "name": _("My Subscriptions"), - "description": _("Access my list of subreddits."), - }, -} - class OAuth2FrontendController(RedditController): def pre(self): RedditController.pre(self) @@ -231,9 +209,11 @@ class OAuth2ResourceController(MinimalController): if handler: oauth2_perms = getattr(handler, "oauth2_perms", None) if oauth2_perms: - granted_scopes = set(access_token.scope_list) + grant = OAuth2Scope(access_token.scope) + if grant.subreddit_only and c.site.name not in grant.subreddits: + self._auth_error(403, "insufficient_scope") required_scopes = set(oauth2_perms['allowed_scopes']) - if not (granted_scopes >= required_scopes): + if not (grant.scopes >= required_scopes): self._auth_error(403, "insufficient_scope") else: self._auth_error(400, "invalid_request") diff --git a/r2/r2/controllers/validator/validator.py b/r2/r2/controllers/validator/validator.py index 165b51450..947f265b6 100644 --- a/r2/r2/controllers/validator/validator.py +++ b/r2/r2/controllers/validator/validator.py @@ -1890,11 +1890,10 @@ class VOAuth2Scope(VRequired): VRequired.__init__(self, param, errors.OAUTH2_INVALID_SCOPE, *a, **kw) def run(self, scope): - from r2.controllers.oauth2 import scope_info scope = VRequired.run(self, scope) if scope: - scope_list = scope.split(',') - if all(scope in scope_info for scope in scope_list): - return scope_list + parsed_scope = OAuth2Scope(scope) + if parsed_scope.is_valid(): + return parsed_scope else: self.error() diff --git a/r2/r2/lib/pages/pages.py b/r2/r2/lib/pages/pages.py index 88f2a7ef9..90cacb36c 100755 --- a/r2/r2/lib/pages/pages.py +++ b/r2/r2/lib/pages/pages.py @@ -700,9 +700,7 @@ class PrefApps(Templated): """Preference form for managing authorized third-party applications.""" def __init__(self, my_apps, developed_apps): - from r2.controllers.oauth2 import scope_info - self.my_apps = [(app, [scope_info[scope] for scope in scopes]) - for app, scopes in my_apps] + self.my_apps = my_apps self.developed_apps = developed_apps super(PrefApps, self).__init__() @@ -864,12 +862,10 @@ class Register(Login): pass class OAuth2AuthorizationPage(BoringPage): - def __init__(self, client, redirect_uri, scopes, state): - from r2.controllers.oauth2 import scope_info - scope_details = [scope_info[scope] for scope in scopes] + def __init__(self, client, redirect_uri, scope, state): content = OAuth2Authorization(client=client, redirect_uri=redirect_uri, - scope_details=scope_details, + scope=scope, state=state) BoringPage.__init__(self, _("request for permission"), show_sidebar=False, content=content) @@ -3662,9 +3658,7 @@ class AccountActivityPage(BoringPage): class UserIPHistory(Templated): def __init__(self): - from r2.controllers.oauth2 import scope_info - self.my_apps = [(app, [scope_info[scope] for scope in scopes]) - for app, scopes in OAuth2Client._by_user(c.user)] + self.my_apps = OAuth2Client._by_user(c.user) self.ips = ips_by_account_id(c.user._id) super(UserIPHistory, self).__init__() diff --git a/r2/r2/models/token.py b/r2/r2/models/token.py index 0b98bd66d..26a4d5beb 100644 --- a/r2/r2/models/token.py +++ b/r2/r2/models/token.py @@ -25,11 +25,12 @@ from base64 import urlsafe_b64encode from pycassa.system_manager import ASCII_TYPE, DATE_TYPE, UTF8_TYPE +from pylons.i18n import _ + from r2.lib.db import tdb_cassandra from r2.lib.db.thing import NotFound from r2.models.account import Account - def generate_token(size): return urlsafe_b64encode(urandom(size)).rstrip("=") @@ -93,6 +94,62 @@ class ConsumableToken(Token): self._commit() +class OAuth2Scope: + scope_info = { + "identity": { + "id": "identity", + "name": _("My Identity"), + "description": _("Access my reddit username and signup date."), + }, + "comment": { + "id": "comment", + "name": _("Commenting"), + "description": _("Submit comments from my account."), + }, + "moderateflair": { + "id": "moderateflair", + "name": _("Moderating Flair"), + "description": _("Manage flair in subreddits I moderate."), + }, + "myreddits": { + "id": "myreddits", + "name": _("My Subscriptions"), + "description": _("Access my list of subreddits."), + }, + } + + def __init__(self, scope_str=None): + if scope_str: + self._parse_scope_str(scope_str) + else: + self.subreddit_only = False + self.subreddits = set() + self.scopes = set() + + def _parse_scope_str(self, scope_str): + srs, sep, scopes = scope_str.rpartition(':') + if sep: + self.subreddit_only = True + self.subreddits = set(srs.split('+')) + else: + self.subreddit_only = False + self.subreddits = set() + self.scopes = set(scopes.split(',')) + + def __str__(self): + if self.subreddit_only: + sr_part = '+'.join(sorted(self.subreddits)) + ':' + else: + sr_part = '' + return sr_part + ','.join(sorted(self.scopes)) + + def is_valid(self): + return all(scope in self.scope_info for scope in self.scopes) + + def details(self): + return [(scope, self.scope_info[scope]) for scope in self.scopes] + + class OAuth2Client(Token): """A client registered for OAuth2 access""" max_developers = 20 @@ -202,16 +259,11 @@ class OAuth2Client(Token): def _by_user(cls, account): """Returns a (possibly empty) list of client-scope pairs for which Account has outstanding access tokens.""" - client_ids = set() - client_id_to_scope = {} - for token in OAuth2AccessToken._by_user(account): - if token.check_valid(): - client_id_to_scope.setdefault(token.client_id, set()).update( - token.scope_list) - - clients = cls._byID(client_id_to_scope.keys()) - return [(client, list(client_id_to_scope.get(client_id, []))) - for client_id, client in clients.iteritems()] + tokens = [token for token in OAuth2AccessToken._by_user(account) + if token.check_valid()] + clients = cls._byID([token.client_id for token in tokens]) + return [(clients[token.client_id], OAuth2Scope(token.scope)) + for token in tokens] def revoke(self, account): """Revoke all of the outstanding OAuth2AccessTokens associated with this client and user Account.""" @@ -244,13 +296,12 @@ class OAuth2AuthorizationCode(ConsumableToken): _connection_pool = "main" @classmethod - def _new(cls, client_id, redirect_uri, user_id, scope_list): - scope = ','.join(scope_list) + def _new(cls, client_id, redirect_uri, user_id, scope): return super(OAuth2AuthorizationCode, cls)._new( client_id=client_id, redirect_uri=redirect_uri, user_id=user_id, - scope=scope) + scope=str(scope)) @classmethod def use_token(cls, _id, client_id, redirect_uri): @@ -278,7 +329,7 @@ class OAuth2AccessToken(Token): return super(OAuth2AccessToken, cls)._new( client_id=client_id, user_id=user_id, - scope=scope) + scope=str(scope)) def _on_create(self): """Updates the OAuth2AccessTokensByUser index upon creation.""" @@ -345,10 +396,6 @@ class OAuth2AccessToken(Token): tokens = cls._byID(tba._values().keys()) return [token for token in tokens.itervalues() if token.check_valid()] - @property - def scope_list(self): - return self.scope.split(',') - class OAuth2AccessTokensByUser(tdb_cassandra.View): """Index listing the outstanding access tokens for an account.""" diff --git a/r2/r2/public/static/css/reddit.css b/r2/r2/public/static/css/reddit.css index dd1a5308b..2b8107f8c 100755 --- a/r2/r2/public/static/css/reddit.css +++ b/r2/r2/public/static/css/reddit.css @@ -6146,6 +6146,8 @@ tr.gold-accent + tr > td { } .app-permissions li { position: relative; } +.app-permissions-subreddits { display:block; margin-top: 1em; } + .app-scope { display: none; position: absolute; diff --git a/r2/r2/templates/oauth2authorization.html b/r2/r2/templates/oauth2authorization.html index 89ef70212..7a89cba61 100644 --- a/r2/r2/templates/oauth2authorization.html +++ b/r2/r2/templates/oauth2authorization.html @@ -23,6 +23,7 @@ <%! from r2.lib.template_helpers import static, s3_https_if_secure %> +<%namespace file="prefapps.html" import="scope_details" /> <%namespace file="utils.html" import="_md" /> <% if thing.client.icon_url: @@ -45,16 +46,12 @@ %endif

${_("Allow %(app_name)s to:") % dict(app_name=thing.client.name)}

- + ${scope_details(thing.scope)}

${_("%(app_name)s will not be able to access your reddit password.") % dict(app_name=thing.client.name)}

- +
diff --git a/r2/r2/templates/prefapps.html b/r2/r2/templates/prefapps.html index 6730c6c21..4304883b0 100644 --- a/r2/r2/templates/prefapps.html +++ b/r2/r2/templates/prefapps.html @@ -165,7 +165,35 @@ -<%def name="authorized_app(app, scopes)"> +<%def name="scope_details(scope, compact=False)"> +
+
    + %for name, scope_info in scope.details(): +
  • + %if compact: + ${scope_info['name']} + ${scope_info['description']} + %else: + ${scope_info['description']} + %endif +
  • + %endfor +
+ %if scope.subreddit_only: + + ${_("Only on:")} + %for i, name in enumerate(sorted(scope.subreddits)): + %if i: + , + %endif + /r/${name} + %endfor + + %endif +
+ + +<%def name="authorized_app(app, scope)">
${icon(app)}
@@ -176,14 +204,7 @@ ${app.name} %endif -
    - %for scope_info in scopes: -
  • - ${scope_info['name']} - ${scope_info['description']} -
  • - %endfor -
+ ${scope_details(scope, compact=True)}
${app.description}
${developers(app)} @@ -199,8 +220,8 @@ %if thing.my_apps:

${_("authorized applications")}

- %for app, scopes in thing.my_apps: - ${authorized_app(app, scopes)} + %for app, scope in thing.my_apps: + ${authorized_app(app, scope)} %endfor %endif diff --git a/r2/r2/templates/useriphistory.html b/r2/r2/templates/useriphistory.html index 93ce99e75..b867be7c6 100644 --- a/r2/r2/templates/useriphistory.html +++ b/r2/r2/templates/useriphistory.html @@ -79,8 +79,8 @@

${_("Apps you have authorized")}

${strings.account_activity_apps_blurb}

- %for app, scopes in thing.my_apps: - ${authorized_app(app, scopes)} + %for app, scope in thing.my_apps: + ${authorized_app(app, scope)} %endfor
%endif