From 3c49e94f97bbbedcd70e66f6ad4ecf8fa26d1bd5 Mon Sep 17 00:00:00 2001 From: Keith Mitchell Date: Mon, 10 Feb 2014 11:51:12 -0800 Subject: [PATCH] Add app 'type' field to app create form This will allow for differing handling of permissions based on application type. * Web app: Web-hosted application. Can keep a client_secret * Installed app: e.g., android app. Client secret is not so secret See also: https://developers.google.com/accounts/docs/OAuth2InstalledApp * Script: For simple scripts and bots. Client secret is assumed secret. --- r2/r2/controllers/api.py | 27 +++++++++++++--- r2/r2/lib/errors.py | 1 + r2/r2/lib/utils/utils.py | 50 ++++++++++++++++------------- r2/r2/lib/validator/validator.py | 21 ++++++++++-- r2/r2/models/token.py | 1 + r2/r2/public/static/css/reddit.less | 2 +- r2/r2/templates/prefapps.html | 41 ++++++++++++++++++----- 7 files changed, 105 insertions(+), 38 deletions(-) diff --git a/r2/r2/controllers/api.py b/r2/r2/controllers/api.py index 344c71738..f19f8ed0a 100755 --- a/r2/r2/controllers/api.py +++ b/r2/r2/controllers/api.py @@ -88,6 +88,7 @@ from r2.lib.merge import ConflictException import csv from collections import defaultdict from datetime import datetime, timedelta +from urlparse import urlparse import hashlib import re import urllib @@ -3540,19 +3541,36 @@ class ApiController(RedditController, OAuth2ResourceController): docs=dict(name="a name for the app")), about_url=VSanitizedUrl('about_url'), icon_url=VSanitizedUrl('icon_url'), - redirect_uri=VSanitizedUrl('redirect_uri')) + redirect_uri=VRedirectUri('redirect_uri'), + app_type=VOneOf('app_type', ('web', 'installed', 'script'))) @api_doc(api_section.apps) - def POST_updateapp(self, form, jquery, name, about_url, icon_url, redirect_uri): + def POST_updateapp(self, form, jquery, name, about_url, icon_url, + redirect_uri, app_type): if (form.has_errors('name', errors.NO_TEXT) | - form.has_errors('redirect_uri', errors.BAD_URL, errors.NO_URL)): + form.has_errors('redirect_uri', errors.BAD_URL) | + form.has_errors('redirect_uri', errors.NO_URL) | + form.has_errors('app_type', errors.INVALID_OPTION)): return + # Web apps should be redirecting to web + if app_type == 'web': + parsed = urlparse(redirect_uri) + if parsed.scheme not in ('http', 'https'): + c.errors.add(errors.INVALID_SCHEME, field='redirect_uri', + msg_params={"schemes": "http, https"}) + form.has_errors('redirect_uri', errors.INVALID_SCHEME) + return + description = request.POST.get('description', '') client_id = request.POST.get('client_id') if client_id: # client_id was specified, updating existing OAuth2Client client = OAuth2Client.get_token(client_id) + if app_type != client.app_type: + # App type cannot be changed after creation + abort(400, "invalid request") + return if not client: form.set_html('.status', _('invalid client id')) return @@ -3577,7 +3595,8 @@ class ApiController(RedditController, OAuth2ResourceController): client = OAuth2Client._new(name=name, description=description, about_url=about_url or '', - redirect_uri=redirect_uri) + redirect_uri=redirect_uri, + app_type=app_type) client._commit() client.add_developer(c.user) form.set_html('.status', _('application created')) diff --git a/r2/r2/lib/errors.py b/r2/r2/lib/errors.py index 1698b112d..60b977de1 100644 --- a/r2/r2/lib/errors.py +++ b/r2/r2/lib/errors.py @@ -33,6 +33,7 @@ error_list = dict(( ('VERIFIED_USER_REQUIRED', _("you need to set a valid email address to do that.")), ('NO_URL', _('a url is required')), ('BAD_URL', _('you should check that url')), + ('INVALID_SCHEME', _('URI scheme must be one of: %(schemes)s')), ('BAD_CAPTCHA', _('care to try these again?')), ('BAD_USERNAME', _('invalid user name')), ('USERNAME_TAKEN', _('that username is already taken')), diff --git a/r2/r2/lib/utils/utils.py b/r2/r2/lib/utils/utils.py index 0d929a576..eaa17e7b3 100644 --- a/r2/r2/lib/utils/utils.py +++ b/r2/r2/lib/utils/utils.py @@ -249,9 +249,9 @@ def extract_title(data): return title.encode('utf-8').strip() -valid_schemes = ('http', 'https', 'ftp', 'mailto') +VALID_SCHEMES = ('http', 'https', 'ftp', 'mailto') valid_dns = re.compile('\A[-a-zA-Z0-9]+\Z') -def sanitize_url(url, require_scheme = False): +def sanitize_url(url, require_scheme=False, valid_schemes=VALID_SCHEMES): """Validates that the url is of the form scheme://domain/path/to/content#anchor?cruft @@ -262,7 +262,7 @@ def sanitize_url(url, require_scheme = False): otherwise validates""" if not url: - return + return None url = url.strip() if url.lower() == 'self': @@ -275,30 +275,34 @@ def sanitize_url(url, require_scheme = False): url = 'http://' + url u = urlparse(url) except ValueError: - return + return None - if u.scheme and u.scheme in valid_schemes: - # if there is a scheme and no hostname, it is a bad url. - if not u.hostname: - return - if u.username is not None or u.password is not None: - return + if not u.scheme: + return None + if valid_schemes is not None and u.scheme not in valid_schemes: + return None - try: - idna_hostname = u.hostname.encode('idna') - except TypeError as e: - g.log.warning("Bad hostname given [%r]: %s", u.hostname, e) - raise - except UnicodeError: - return + # if there is a scheme and no hostname, it is a bad url. + if not u.hostname: + return None + if u.username is not None or u.password is not None: + return None - for label in idna_hostname.split('.'): - if not re.match(valid_dns, label): - return + try: + idna_hostname = u.hostname.encode('idna') + except TypeError as e: + g.log.warning("Bad hostname given [%r]: %s", u.hostname, e) + raise + except UnicodeError: + return None - if idna_hostname != u.hostname: - url = urlunparse((u[0], idna_hostname, u[2], u[3], u[4], u[5])) - return url + for label in idna_hostname.split('.'): + if not re.match(valid_dns, label): + return None + + if idna_hostname != u.hostname: + url = urlunparse((u[0], idna_hostname, u[2], u[3], u[4], u[5])) + return url def trunc_string(text, length): return text[0:length]+'...' if len(text)>length else text diff --git a/r2/r2/lib/validator/validator.py b/r2/r2/lib/validator/validator.py index e788ab801..d9e8a1442 100644 --- a/r2/r2/lib/validator/validator.py +++ b/r2/r2/lib/validator/validator.py @@ -1299,15 +1299,19 @@ class VSanitizedUrl(Validator): class VUrl(VRequired): - def __init__(self, item, allow_self=True, *a, **kw): + def __init__(self, item, allow_self=True, require_scheme=False, + valid_schemes=utils.VALID_SCHEMES, *a, **kw): self.allow_self = allow_self + self.require_scheme = require_scheme + self.valid_schemes = valid_schemes VRequired.__init__(self, item, errors.NO_URL, *a, **kw) def run(self, url): if not url: return self.error(errors.NO_URL) - url = utils.sanitize_url(url) + url = utils.sanitize_url(url, require_scheme=self.require_scheme, + valid_schemes=self.valid_schemes) if not url: return self.error(errors.BAD_URL) @@ -1323,6 +1327,19 @@ class VUrl(VRequired): return {self.param: "a valid URL"} +class VRedirectUri(VUrl): + def __init__(self, item, valid_schemes=None, *a, **kw): + VUrl.__init__(self, item, allow_self=False, require_scheme=True, + valid_schemes=valid_schemes, *a, **kw) + + def param_docs(self): + doc = "a valid URI" + if self.valid_schemes: + doc += " with one of the following schemes: " + doc += ", ".join(self.valid_schemes) + return {self.param: doc} + + class VShamedDomain(Validator): def run(self, url): if not url: diff --git a/r2/r2/models/token.py b/r2/r2/models/token.py index 735f09a3b..8dfc47340 100644 --- a/r2/r2/models/token.py +++ b/r2/r2/models/token.py @@ -278,6 +278,7 @@ class OAuth2Client(Token): icon_url="", secret="", redirect_uri="", + app_type="web", ) _use_db = True _connection_pool = "main" diff --git a/r2/r2/public/static/css/reddit.less b/r2/r2/public/static/css/reddit.less index 54e8314dd..7bef7e792 100755 --- a/r2/r2/public/static/css/reddit.less +++ b/r2/r2/public/static/css/reddit.less @@ -7553,7 +7553,7 @@ body:not(.gold) .allminus-link { .edit-app-icon-button { display: block; text-align: center; width: 72px; } .edit-app-form, .edit-app-form form { display: inline-block; } .edit-app-form th, .edit-app-icon th { width: 12ex; } -.edit-app-form input, .edit-app-form textarea { margin: 0px; width: 50ex; } +.edit-app-form input.text { margin: 0px; width: 50ex; } .edit-app-form input[name="name"] { width: 20ex !important; } .edit-app-form input[type="file"] { width: auto !important; } .edit-app-form input[type="submit"] { diff --git a/r2/r2/templates/prefapps.html b/r2/r2/templates/prefapps.html index 968ce34a2..e34167a83 100644 --- a/r2/r2/templates/prefapps.html +++ b/r2/r2/templates/prefapps.html @@ -56,6 +56,18 @@ %endif +<%def name="app_type_selector(selection='web')"> +${utils.radio_type('app_type', "web", _("web app"), + _("A web based application"), + selection == "web")} +${utils.radio_type('app_type', "installed", _("installed app"), + _("An app intended for installation, such as on a mobile phone"), + selection == "installed")} +${utils.radio_type('app_type', "script", _("script"), + _("Script for personal use. Will only have access to the developers accounts"), + selection == "script")} + + <%def name="editable_developer(app, dev)">
  • ${dev.name} @@ -83,6 +95,15 @@ ${app.name} %endif +

    + %if app.app_type == 'web': + ${_("web app")} + %elif app.app_type == 'installed': + ${_("installed app")} + %elif app.app_type == 'script': + ${_("personal use script")} + %endif +

    ${app._id}

    ${app.description}
    @@ -105,6 +126,7 @@ onsubmit="${"return post_form(this, 'updateapp', function(x) {return '%s'})" % _("updating...")}"> + @@ -113,7 +135,7 @@ @@ -126,17 +148,18 @@
    ${_("secret")}
    ${_("name")} - + ${error_field("NO_TEXT", "name")}
    ${_("about url")} - + ${error_field("BAD_URL", "about_url")}
    ${_("redirect uri")} - + ${error_field("NO_URL", "redirect_uri")} ${error_field("BAD_URL", "redirect_uri")} + ${error_field("INVALID_SCHEME", "redirect_uri")}
    @@ -160,7 +183,7 @@ onsubmit="${"return post_form(this, 'adddeveloper', function(x) {return '%s'})" % _("adding...")}"> - ${_('add developer')}: + ${_('add developer')}:
    ${error_field('TOO_MANY_DEVELOPERS', '')} ${error_field('OAUTH2_INVALID_CLIENT', 'client_id')} @@ -302,10 +325,11 @@ ${_("name")} - + ${error_field("NO_TEXT", "name")} + ${app_type_selector()} ${_("description")} @@ -315,16 +339,17 @@ ${_("about url")} - + ${error_field("BAD_URL", "about_url")} ${_("redirect uri")} - + ${error_field("NO_URL", "redirect_uri")} ${error_field("BAD_URL", "redirect_uri")} + ${error_field("INVALID_SCHEME", "redirect_uri")}