Changeset - aa25ef34ebab
[Not reviewed]
default
0 16 0
Mads Kiilerich - 8 years ago 2018-01-21 02:49:15
mads@kiilerich.com
auth: refactor to introduce @LoginRequired(allow_default_user=True) and deprecate @NotAnonymous()

It was error prone that @LoginRequired defaulted to allow anonymous users (if
'default' user is enabled). See also 245b4e3abf39.

Refactor code to make it more explicit and safe by default: Deprecate
@NotAnonymous by making it the default of @LoginRequired. That will make it
safe by default.

To preserve same functionality, set allow_default_user=True in all the cases
where @LoginRequired was *not* followed by @NotAnonymous or other permission
checks - that was done with some script hacks:
sed -i 's/@LoginRequired(\(..*\))/@LoginRequired(\1, allow_default_user=True)/g' `hg mani`
sed -i 's/@LoginRequired()/@LoginRequired(allow_default_user=True)/g' `hg mani`
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)\n\s*\@NotAnonymous\(\)/\@LoginRequired()/g' `hg mani`
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@Has(Repo)?Permission)/\@LoginRequired()\1/g' `hg mani`

It has been reviewed that all uses of allow_default_user=True are in places
where the there indeed wasn't any checking for default user before. These may
or may not be correct, but now they are explicit and can be spotted and fixed.

The few remaining uses of @NotAnonymous should probably be removed somehow.
16 files changed with 36 insertions and 56 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/admin/admin.py
Show inline comments
 
@@ -120,7 +120,7 @@ def _journal_filter(user_log, search_ter
 

	
 
class AdminController(BaseController):
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def _before(self, *args, **kwargs):
 
        super(AdminController, self)._before(*args, **kwargs)
 

	
kallithea/controllers/admin/gists.py
Show inline comments
 
@@ -41,7 +41,7 @@ from kallithea.model.meta import Session
 
from kallithea.model.db import Gist, User
 
from kallithea.lib import helpers as h
 
from kallithea.lib.base import BaseController, render, jsonify
 
from kallithea.lib.auth import LoginRequired, NotAnonymous
 
from kallithea.lib.auth import LoginRequired
 
from kallithea.lib.utils2 import safe_int, safe_unicode, time_to_datetime
 
from kallithea.lib.page import Page
 
from sqlalchemy.sql.expression import or_
 
@@ -65,7 +65,7 @@ class GistsController(BaseController):
 
            c.lifetime_values.append(extra_values)
 
        c.lifetime_options = [(c.lifetime_values, _("Lifetime"))]
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def index(self):
 
        not_default_user = not request.authuser.is_default_user
 
        c.show_private = request.GET.get('private') and not_default_user
 
@@ -100,7 +100,6 @@ class GistsController(BaseController):
 
        return render('admin/gists/index.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def create(self):
 
        self.__load_defaults()
 
        gist_form = GistForm([x[0] for x in c.lifetime_values])()
 
@@ -143,13 +142,11 @@ class GistsController(BaseController):
 
        raise HTTPFound(location=url('gist', gist_id=new_gist_id))
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def new(self, format='html'):
 
        self.__load_defaults()
 
        return render('admin/gists/new.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def delete(self, gist_id):
 
        gist = GistModel().get_gist(gist_id)
 
        owner = gist.owner_id == request.authuser.user_id
 
@@ -162,7 +159,7 @@ class GistsController(BaseController):
 

	
 
        raise HTTPFound(location=url('gists'))
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def show(self, gist_id, revision='tip', format='html', f_path=None):
 
        c.gist = Gist.get_or_404(gist_id)
 

	
 
@@ -183,7 +180,6 @@ class GistsController(BaseController):
 
        return render('admin/gists/show.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def edit(self, gist_id, format='html'):
 
        c.gist = Gist.get_or_404(gist_id)
 

	
 
@@ -242,7 +238,6 @@ class GistsController(BaseController):
 
        return rendered
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @jsonify
 
    def check_revision(self, gist_id):
 
        c.gist = Gist.get_or_404(gist_id)
kallithea/controllers/admin/my_account.py
Show inline comments
 
@@ -38,7 +38,7 @@ from webob.exc import HTTPFound
 
from kallithea.config.routing import url
 
from kallithea.lib import helpers as h
 
from kallithea.lib import auth_modules
 
from kallithea.lib.auth import LoginRequired, NotAnonymous, AuthUser
 
from kallithea.lib.auth import LoginRequired, AuthUser
 
from kallithea.lib.base import BaseController, render
 
from kallithea.lib.utils2 import generate_api_key, safe_int
 
from kallithea.model.db import Repository, UserEmailMap, User, UserFollowing
 
@@ -59,7 +59,6 @@ class MyAccountController(BaseController
 
    #         path_prefix='/admin', name_prefix='admin_')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def _before(self, *args, **kwargs):
 
        super(MyAccountController, self)._before(*args, **kwargs)
 

	
kallithea/controllers/admin/notifications.py
Show inline comments
 
@@ -35,7 +35,7 @@ from webob.exc import HTTPBadRequest, HT
 
from kallithea.model.db import Notification
 
from kallithea.model.notification import NotificationModel
 
from kallithea.model.meta import Session
 
from kallithea.lib.auth import LoginRequired, NotAnonymous
 
from kallithea.lib.auth import LoginRequired
 
from kallithea.lib.base import BaseController, render
 
from kallithea.lib import helpers as h
 
from kallithea.lib.page import Page
 
@@ -53,7 +53,6 @@ class NotificationsController(BaseContro
 
    #         path_prefix='/_admin', name_prefix='_admin_')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def _before(self, *args, **kwargs):
 
        super(NotificationsController, self)._before(*args, **kwargs)
 

	
kallithea/controllers/admin/repo_groups.py
Show inline comments
 
@@ -58,7 +58,7 @@ log = logging.getLogger(__name__)
 

	
 
class RepoGroupsController(BaseController):
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def _before(self, *args, **kwargs):
 
        super(RepoGroupsController, self)._before(*args, **kwargs)
 

	
kallithea/controllers/admin/repos.py
Show inline comments
 
@@ -60,7 +60,7 @@ class ReposController(BaseRepoController
 
    # file has a resource setup:
 
    #     map.resource('repo', 'repos')
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def _before(self, *args, **kwargs):
 
        super(ReposController, self)._before(*args, **kwargs)
 

	
 
@@ -169,7 +169,6 @@ class ReposController(BaseRepoController
 
            force_defaults=False)
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def repo_creating(self, repo_name):
 
        c.repo = repo_name
 
        c.task_id = request.GET.get('task_id')
 
@@ -178,7 +177,6 @@ class ReposController(BaseRepoController
 
        return render('admin/repos/repo_creating.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @jsonify
 
    def repo_check(self, repo_name):
 
        c.repo = repo_name
kallithea/controllers/admin/settings.py
Show inline comments
 
@@ -59,7 +59,7 @@ class SettingsController(BaseController)
 
    #     map.resource('setting', 'settings', controller='admin/settings',
 
    #         path_prefix='/admin', name_prefix='admin_')
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def _before(self, *args, **kwargs):
 
        super(SettingsController, self)._before(*args, **kwargs)
 

	
kallithea/controllers/admin/user_groups.py
Show inline comments
 
@@ -63,7 +63,7 @@ log = logging.getLogger(__name__)
 
class UserGroupsController(BaseController):
 
    """REST Controller styled on the Atom Publishing Protocol"""
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def _before(self, *args, **kwargs):
 
        super(UserGroupsController, self)._before(*args, **kwargs)
 
        c.available_permissions = config['available_permissions']
kallithea/controllers/changeset.py
Show inline comments
 
@@ -37,8 +37,7 @@ from kallithea.lib.vcs.exceptions import
 
    ChangesetDoesNotExistError, EmptyRepositoryError
 

	
 
import kallithea.lib.helpers as h
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator, \
 
    NotAnonymous
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator
 
from kallithea.lib.base import BaseRepoController, render, jsonify
 
from kallithea.lib.utils import action_logger
 
from kallithea.lib.compat import OrderedDict
 
@@ -348,7 +347,6 @@ class ChangesetController(BaseRepoContro
 
        return self._index(revision, method='download')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def comment(self, repo_name, revision):
 
@@ -399,7 +397,6 @@ class ChangesetController(BaseRepoContro
 
        return data
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def delete_comment(self, repo_name, comment_id):
kallithea/controllers/forks.py
Show inline comments
 
@@ -38,7 +38,7 @@ import kallithea.lib.helpers as h
 

	
 
from kallithea.config.routing import url
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator, \
 
    NotAnonymous, HasRepoPermissionLevel, HasPermissionAnyDecorator, HasPermissionAny
 
    HasRepoPermissionLevel, HasPermissionAnyDecorator, HasPermissionAny
 
from kallithea.lib.base import BaseRepoController, render
 
from kallithea.lib.page import Page
 
from kallithea.lib.utils2 import safe_int
 
@@ -123,7 +123,6 @@ class ForksController(BaseRepoController
 
        return render('/forks/forks.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasPermissionAnyDecorator('hg.admin', 'hg.fork.repository')
 
    @HasRepoPermissionLevelDecorator('read')
 
    def fork(self, repo_name):
 
@@ -141,7 +140,6 @@ class ForksController(BaseRepoController
 
            force_defaults=False)
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasPermissionAnyDecorator('hg.admin', 'hg.fork.repository')
 
    @HasRepoPermissionLevelDecorator('read')
 
    def fork_create(self, repo_name):
kallithea/controllers/home.py
Show inline comments
 
@@ -50,7 +50,7 @@ class HomeController(BaseController):
 
    def about(self):
 
        return render('/about.html')
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def index(self):
 
        c.group = None
 

	
 
@@ -63,7 +63,7 @@ class HomeController(BaseController):
 

	
 
        return render('/index.html')
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    @jsonify
 
    def repo_switcher_data(self):
 
        # wrapper for conditional cache
 
@@ -145,7 +145,7 @@ class HomeController(BaseController):
 
        }
 
        return data
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    @jsonify
 
    def users_and_groups_data(self):
 
        """
kallithea/controllers/journal.py
Show inline comments
 
@@ -46,7 +46,7 @@ from kallithea.model.db import UserLog, 
 
from kallithea.model.meta import Session
 
from kallithea.model.repo import RepoModel
 
import kallithea.lib.helpers as h
 
from kallithea.lib.auth import LoginRequired, NotAnonymous
 
from kallithea.lib.auth import LoginRequired
 
from kallithea.lib.base import BaseController, render
 
from kallithea.lib.page import Page
 
from kallithea.lib.utils2 import safe_int, AttributeDict
 
@@ -191,7 +191,6 @@ class JournalController(BaseController):
 
        return feed.writeString('utf-8')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def index(self):
 
        # Return a rendered template
 
        p = safe_int(request.GET.get('page'), 1)
 
@@ -223,7 +222,6 @@ class JournalController(BaseController):
 
        return render('journal/journal.html')
 

	
 
    @LoginRequired(api_access=True)
 
    @NotAnonymous()
 
    def journal_atom(self):
 
        """
 
        Produce an atom-1.0 feed via feedgenerator module
 
@@ -235,7 +233,6 @@ class JournalController(BaseController):
 
        return self._atom_feed(following, public=False)
 

	
 
    @LoginRequired(api_access=True)
 
    @NotAnonymous()
 
    def journal_rss(self):
 
        """
 
        Produce an rss feed via feedgenerator module
 
@@ -247,7 +244,6 @@ class JournalController(BaseController):
 
        return self._rss_feed(following, public=False)
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def toggle_following(self):
 
        user_id = request.POST.get('follows_user_id')
 
        if user_id:
 
@@ -273,7 +269,7 @@ class JournalController(BaseController):
 

	
 
        raise HTTPBadRequest()
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def public_journal(self):
 
        # Return a rendered template
 
        p = safe_int(request.GET.get('page'), 1)
 
@@ -294,7 +290,7 @@ class JournalController(BaseController):
 

	
 
        return render('journal/public_journal.html')
 

	
 
    @LoginRequired(api_access=True)
 
    @LoginRequired(api_access=True, allow_default_user=True)
 
    def public_journal_atom(self):
 
        """
 
        Produce an atom-1.0 feed via feedgenerator module
 
@@ -306,7 +302,7 @@ class JournalController(BaseController):
 

	
 
        return self._atom_feed(c.following)
 

	
 
    @LoginRequired(api_access=True)
 
    @LoginRequired(api_access=True, allow_default_user=True)
 
    def public_journal_rss(self):
 
        """
 
        Produce an rss2 feed via feedgenerator module
kallithea/controllers/pullrequests.py
Show inline comments
 
@@ -36,8 +36,7 @@ from webob.exc import HTTPFound, HTTPNot
 
from kallithea.config.routing import url
 
from kallithea.lib import helpers as h
 
from kallithea.lib import diffs
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator, \
 
    NotAnonymous
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator
 
from kallithea.lib.base import BaseRepoController, render, jsonify
 
from kallithea.lib.page import Page
 
from kallithea.lib.utils import action_logger
 
@@ -218,7 +217,6 @@ class PullrequestsController(BaseRepoCon
 
        return render('/pullrequests/pullrequest_show_all.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def show_my(self):
 
        c.closed = request.GET.get('closed') or ''
 

	
 
@@ -244,7 +242,6 @@ class PullrequestsController(BaseRepoCon
 
        return render('/pullrequests/pullrequest_show_my.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    def index(self):
 
        org_repo = c.db_repo
 
@@ -300,7 +297,6 @@ class PullrequestsController(BaseRepoCon
 
        return render('/pullrequests/pullrequest.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def repo_info(self, repo_name):
 
@@ -313,7 +309,6 @@ class PullrequestsController(BaseRepoCon
 
            }
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    def create(self, repo_name):
 
        repo = c.db_repo
 
@@ -383,7 +378,6 @@ class PullrequestsController(BaseRepoCon
 

	
 
    # pullrequest_post for PR editing
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    def post(self, repo_name, pull_request_id):
 
        pull_request = PullRequest.get_or_404(pull_request_id)
 
@@ -440,7 +434,6 @@ class PullrequestsController(BaseRepoCon
 
        raise HTTPFound(location=pull_request.url())
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def delete(self, repo_name, pull_request_id):
 
@@ -633,7 +626,6 @@ class PullrequestsController(BaseRepoCon
 
        return render('/pullrequests/pullrequest_show.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def comment(self, repo_name, pull_request_id):
 
@@ -718,7 +710,6 @@ class PullrequestsController(BaseRepoCon
 
        return data
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def delete_comment(self, repo_name, comment_id):
kallithea/controllers/search.py
Show inline comments
 
@@ -49,7 +49,7 @@ log = logging.getLogger(__name__)
 

	
 
class SearchController(BaseRepoController):
 

	
 
    @LoginRequired()
 
    @LoginRequired(allow_default_user=True)
 
    def index(self, repo_name=None):
 
        c.repo_name = repo_name
 
        c.formated_results = []
kallithea/controllers/summary.py
Show inline comments
 
@@ -43,8 +43,7 @@ from kallithea.lib.vcs.exceptions import
 
from kallithea.config.conf import ALL_READMES, ALL_EXTS, LANGUAGES_EXTENSIONS_MAP
 
from kallithea.model.db import Statistics, CacheInvalidation, User
 
from kallithea.lib.utils2 import safe_int, safe_str
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator, \
 
    NotAnonymous
 
from kallithea.lib.auth import LoginRequired, HasRepoPermissionLevelDecorator
 
from kallithea.lib.base import BaseRepoController, render, jsonify
 
from kallithea.lib.vcs.backends.base import EmptyChangeset
 
from kallithea.lib.markup_renderer import MarkupRenderer
 
@@ -162,7 +161,6 @@ class SummaryController(BaseRepoControll
 
        return render('summary/summary.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionLevelDecorator('read')
 
    @jsonify
 
    def repo_size(self, repo_name):
kallithea/lib/auth.py
Show inline comments
 
@@ -752,16 +752,20 @@ def _redirect_to_login(message=None):
 

	
 
# Use as decorator
 
class LoginRequired(object):
 
    """Client must be logged in as a valid User (but the "default" user,
 
    if enabled, is considered valid), or we'll redirect to the login page.
 
    """Client must be logged in as a valid User, or we'll redirect to the login
 
    page.
 

	
 
    If the "default" user is enabled and allow_default_user is true, that is
 
    considered valid too.
 

	
 
    Also checks that IP address is allowed, and if using API key instead
 
    of regular cookie authentication, checks that API key access is allowed
 
    (based on `api_access` parameter and the API view whitelist).
 
    """
 

	
 
    def __init__(self, api_access=False):
 
    def __init__(self, api_access=False, allow_default_user=False):
 
        self.api_access = api_access
 
        self.allow_default_user = allow_default_user
 

	
 
    def __call__(self, func):
 
        return decorator(self.__wrapper, func)
 
@@ -801,12 +805,17 @@ class LoginRequired(object):
 
                raise HTTPForbidden()
 

	
 
        # regular user authentication
 
        if user.is_authenticated or user.is_default_user:
 
        if user.is_authenticated:
 
            log.info('user %s authenticated with regular auth @ %s', user, loc)
 
            return func(*fargs, **fkwargs)
 
        elif user.is_default_user:
 
            if self.allow_default_user:
 
                log.info('default user @ %s', loc)
 
                return func(*fargs, **fkwargs)
 
            log.info('default user is not accepted here @ %s', loc)
 
        else:
 
            log.warning('user %s NOT authenticated with regular auth @ %s', user, loc)
 
            raise _redirect_to_login()
 
        raise _redirect_to_login()
 

	
 

	
 
# Use as decorator
0 comments (0 inline, 0 general)