From 2a8cc84faad968ef7b72908eb55b45f663b951f9 Mon Sep 17 00:00:00 2001 From: Neil Williams Date: Wed, 29 Aug 2012 10:53:33 -0700 Subject: [PATCH] Refactor user authentication to be more modular. This allows alternate authentication methods to be easily plugged in for custom installs of reddit, such as LDAP on intranets. --- r2/example.ini | 2 + r2/r2/controllers/reddit_base.py | 9 +-- r2/r2/lib/authentication.py | 100 +++++++++++++++++++++++++++ r2/r2/lib/utils/utils.py | 5 +- r2/r2/models/account.py | 22 ------ r2/r2/templates/redditheader.compact | 3 + r2/r2/templates/redditheader.html | 3 +- 7 files changed, 114 insertions(+), 30 deletions(-) create mode 100644 r2/r2/lib/authentication.py diff --git a/r2/example.ini b/r2/example.ini index a90903b12..9802e384e 100644 --- a/r2/example.ini +++ b/r2/example.ini @@ -115,6 +115,8 @@ login_cookie = reddit_session admin_cookie = reddit_admin # name of the otp cookie otp_cookie = reddit_otp +# how to authenticate users. see r2/lib/authentication.py for options +authentication_provider = cookie # the work factor for bcrypt, increment this every time computers double in # speed. don't worry, changing this won't break old passwords bcrypt_work_factor = 12 diff --git a/r2/r2/controllers/reddit_base.py b/r2/r2/controllers/reddit_base.py index 31ecc84dc..4ce165600 100644 --- a/r2/r2/controllers/reddit_base.py +++ b/r2/r2/controllers/reddit_base.py @@ -29,7 +29,7 @@ from r2.lib import pages, utils, filters, amqp, stats from r2.lib.utils import http_utils, is_subdomain, UniqueIterator, is_throttled from r2.lib.cache import LocalCache, make_key, MemcachedError import random as rand -from r2.models.account import valid_cookie, FakeAccount, valid_feed, valid_admin_cookie +from r2.models.account import FakeAccount, valid_feed, valid_admin_cookie from r2.models.subreddit import Subreddit, Frontpage from r2.models import * from errors import ErrorSet, ForbiddenError, errors @@ -39,6 +39,7 @@ from r2.config.extensions import is_api from r2.lib.translation import set_lang from r2.lib.contrib import ipaddress from r2.lib.base import BaseController, proxyurl, abort +from r2.lib.authentication import authenticate_user from Cookie import CookieError from copy import copy @@ -834,11 +835,7 @@ class RedditController(MinimalController): # no logins for RSS feed unless valid_feed has already been called if not c.user: if c.extension != "rss": - session_cookie = c.cookies.get(g.login_cookie) - if session_cookie: - c.user = valid_cookie(session_cookie.value) - if c.user: - c.user_is_loggedin = True + authenticate_user() admin_cookie = c.cookies.get(g.admin_cookie) if c.user_is_loggedin and admin_cookie: diff --git a/r2/r2/lib/authentication.py b/r2/r2/lib/authentication.py new file mode 100644 index 000000000..5d18b6645 --- /dev/null +++ b/r2/r2/lib/authentication.py @@ -0,0 +1,100 @@ +# The contents of this file are subject to the Common Public Attribution +# License Version 1.0. (the "License"); you may not use this file except in +# compliance with the License. You may obtain a copy of the License at +# http://code.reddit.com/LICENSE. The License is based on the Mozilla Public +# License Version 1.1, but Sections 14 and 15 have been added to cover use of +# software over a computer network and provide for limited attribution for the +# Original Developer. In addition, Exhibit A has been modified to be consistent +# with Exhibit B. +# +# Software distributed under the License is distributed on an "AS IS" basis, +# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for +# the specific language governing rights and limitations under the License. +# +# The Original Code is reddit. +# +# The Original Developer is the Initial Developer. The Initial Developer of +# the Original Code is reddit Inc. +# +# All portions of the code written by reddit are Copyright (c) 2006-2012 reddit +# Inc. All Rights Reserved. +############################################################################### +"""Authentication providers for setting c.user on every request. + +This system is intended to allow pluggable authentication for intranets etc. It +is not intended to cover all login/logout functionality and in non-cookie +scenarios those are probably nonsensical to allow user control of (i.e. +single-signon on an intranet doesn't generally allow new account creation on a +single website.) +""" + +from pylons import g, c + +from r2.models import Account, NotFound +from r2.lib.utils import constant_time_compare + + +_AUTHENTICATION_PROVIDERS = {} + + +def authentication_provider(allow_logout): + """Register an authentication provider with the framework. + + Authentication providers should return None if authentication failed or an + Account object if it succeeded. + """ + def authentication_provider_decorator(fn): + _AUTHENTICATION_PROVIDERS[fn.__name__] = fn + fn.allow_logout = allow_logout + return fn + return authentication_provider_decorator + + +@authentication_provider(allow_logout=True) +def cookie(): + """Authenticate the user given a session cookie.""" + session_cookie = c.cookies.get(g.login_cookie) + if not session_cookie: + return None + + cookie = session_cookie.value + try: + uid, timestr, hash = cookie.split(",") + uid = int(uid) + except: + return None + + try: + account = Account._byID(uid, data=True) + except NotFound: + return None + + if not constant_time_compare(cookie, account.make_cookie(timestr)): + return None + return account + + +def _get_authenticator(): + """Return the configured authenticator.""" + return _AUTHENTICATION_PROVIDERS[g.authentication_provider] + + +def user_can_log_out(): + """Return if the configured authenticator allows users to log out.""" + authenticator = _get_authenticator() + return authenticator.allow_logout + + +def authenticate_user(): + """Attempt to authenticate the user using the configured authenticator.""" + + if not g.read_only_mode: + authenticator = _get_authenticator() + c.user = authenticator() + + if c.user and c.user._deleted: + c.user = None + else: + c.user = None + + c.user_is_loggedin = bool(c.user) diff --git a/r2/r2/lib/utils/utils.py b/r2/r2/lib/utils/utils.py index 20b653aa4..b9ba1a5f3 100644 --- a/r2/r2/lib/utils/utils.py +++ b/r2/r2/lib/utils/utils.py @@ -20,6 +20,8 @@ # Inc. All Rights Reserved. ############################################################################### +import base64 + from urllib import unquote_plus from urllib2 import urlopen from urlparse import urlparse, urlunparse @@ -39,6 +41,7 @@ from pylons.i18n import ungettext, _ from r2.lib.filters import _force_unicode, _force_utf8 from mako.filters import url_escape from r2.lib.contrib import ipaddress +from r2.lib.require import require, require_split import snudown from r2.lib.utils._utils import * @@ -1416,7 +1419,7 @@ def parse_http_basic(authorization_header): Raises RequirementException if anything is uncool. """ - auth_scheme, auth_token = require_split(auth, 2) + auth_scheme, auth_token = require_split(authorization_header, 2) require(auth_scheme.lower() == "basic") try: auth_data = base64.b64decode(auth_token) diff --git a/r2/r2/models/account.py b/r2/r2/models/account.py index 2e608293a..70144f896 100644 --- a/r2/r2/models/account.py +++ b/r2/r2/models/account.py @@ -608,28 +608,6 @@ class FakeAccount(Account): pref_no_profanity = True -def valid_cookie(cookie): - try: - uid, timestr, hash = cookie.split(',') - uid = int(uid) - except: - return False - - if g.read_only_mode: - return False - - try: - account = Account._byID(uid, True) - if account._deleted: - return False - except NotFound: - return False - - if constant_time_compare(cookie, account.make_cookie(timestr)): - return account - return False - - def valid_admin_cookie(cookie): if g.read_only_mode: return (False, None) diff --git a/r2/r2/templates/redditheader.compact b/r2/r2/templates/redditheader.compact index cc9a3e56e..2c2357189 100644 --- a/r2/r2/templates/redditheader.compact +++ b/r2/r2/templates/redditheader.compact @@ -23,6 +23,7 @@ <%! from r2.lib.template_helpers import static, s3_https_if_secure from r2.models.subreddit import DefaultSR + from r2.lib import authentication %> <%namespace file="utils.html" import="plain_link, img_link, separator, logout"/> @@ -95,9 +96,11 @@ + %if authentication.user_can_log_out(): + % endif %else: