Refactor oauth2 scopes and add subreddits to them.

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")

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()

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__()
 

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."""
 

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;

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
   <div class="access">
     <h2>${_("Allow %(app_name)s to:") % dict(app_name=thing.client.name)}</h2>
-    <ul>
-      %for scope_info in thing.scope_details:
-        <li>${scope_info["description"]}</li>
-      %endfor
-    </ul>
+    ${scope_details(thing.scope)}
     <p class="notice">${_("%(app_name)s will not be able to access your reddit password.") % dict(app_name=thing.client.name)}</p>
     <form method="post" action="/api/v1/authorize" class="pretty-form">
       <input type="hidden" name="client_id" value="${thing.client._id}" />
       <input type="hidden" name="redirect_uri" value="${thing.redirect_uri}" />
-      <input type="hidden" name="scope" value="${','.join(scope['id'] for scope in thing.scope_details)}" />
+      <input type="hidden" name="scope" value="${str(thing.scope)}" />
       <input type="hidden" name="state" value="${thing.state}" />
       <input type="hidden" name="uh" value="${c.modhash}"/>
       <div>

r2/r2/templates/prefapps.html

@@ -165,7 +165,35 @@
   </li>
 </%def>
 
-<%def name="authorized_app(app, scopes)">
+<%def name="scope_details(scope, compact=False)">
+  <div class="app-permissions">
+    <ul>
+      %for name, scope_info in scope.details():
+        <li>
+          %if compact:
+            ${scope_info['name']}
+            <span class="app-scope">${scope_info['description']}</span>
+          %else:
+            ${scope_info['description']}
+          %endif
+        </li>
+      %endfor
+    </ul>
+    %if scope.subreddit_only:
+      <span class="app-permissions-subreddits">
+        ${_("Only on:")}&#32;
+        %for i, name in enumerate(sorted(scope.subreddits)):
+          %if i:
+            ,&#32;
+          %endif
+          <a href="/r/${name}">/r/${name}</a>
+        %endfor
+      </span>
+    %endif
+  </div>
+</%def>
+
+<%def name="authorized_app(app, scope)">
   <div id="authorized-app-${app._id}" class="authorized-app rounded">
     ${icon(app)}
     <div class="app-details">
@@ -176,14 +204,7 @@
         ${app.name}
       %endif
       </h2>
-      <ul class="app-permissions">
-        %for scope_info in scopes:
-          <li>
-            ${scope_info['name']}
-            <span class="app-scope">${scope_info['description']}</span>
-          </li>
-        %endfor
-      </ul>
+      ${scope_details(scope, compact=True)}
     </div>
     <div class="app-description">${app.description}</div>
     ${developers(app)}
@@ -199,8 +220,8 @@
 %if thing.my_apps:
   <h1>${_("authorized applications")}</h1>
 
-  %for app, scopes in thing.my_apps:
-    ${authorized_app(app, scopes)}
+  %for app, scope in thing.my_apps:
+    ${authorized_app(app, scope)}
   %endfor
 %endif
 

r2/r2/templates/useriphistory.html

@@ -79,8 +79,8 @@
   <div id="account-activity-apps" class="instructions">
     <h1>${_("Apps you have authorized")}</h1>
     <p>${strings.account_activity_apps_blurb}</p>
-    %for app, scopes in thing.my_apps:
-      ${authorized_app(app, scopes)}
+    %for app, scope in thing.my_apps:
+      ${authorized_app(app, scope)}
     %endfor
   </div>
 %endif