# HG changeset patch # User Mads Kiilerich # Date 2018-01-21 02:49:15 # Node ID aa25ef34ebab2f48c85420e6c0ef1fc0a7d4ef96 # Parent 422671dd32df4868c49d45e3a3b8e884def57bf4 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. diff --git a/kallithea/controllers/admin/admin.py b/kallithea/controllers/admin/admin.py --- a/kallithea/controllers/admin/admin.py +++ b/kallithea/controllers/admin/admin.py @@ -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) diff --git a/kallithea/controllers/admin/gists.py b/kallithea/controllers/admin/gists.py --- a/kallithea/controllers/admin/gists.py +++ b/kallithea/controllers/admin/gists.py @@ -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) diff --git a/kallithea/controllers/admin/my_account.py b/kallithea/controllers/admin/my_account.py --- a/kallithea/controllers/admin/my_account.py +++ b/kallithea/controllers/admin/my_account.py @@ -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) diff --git a/kallithea/controllers/admin/notifications.py b/kallithea/controllers/admin/notifications.py --- a/kallithea/controllers/admin/notifications.py +++ b/kallithea/controllers/admin/notifications.py @@ -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) diff --git a/kallithea/controllers/admin/repo_groups.py b/kallithea/controllers/admin/repo_groups.py --- a/kallithea/controllers/admin/repo_groups.py +++ b/kallithea/controllers/admin/repo_groups.py @@ -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) diff --git a/kallithea/controllers/admin/repos.py b/kallithea/controllers/admin/repos.py --- a/kallithea/controllers/admin/repos.py +++ b/kallithea/controllers/admin/repos.py @@ -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 diff --git a/kallithea/controllers/admin/settings.py b/kallithea/controllers/admin/settings.py --- a/kallithea/controllers/admin/settings.py +++ b/kallithea/controllers/admin/settings.py @@ -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) diff --git a/kallithea/controllers/admin/user_groups.py b/kallithea/controllers/admin/user_groups.py --- a/kallithea/controllers/admin/user_groups.py +++ b/kallithea/controllers/admin/user_groups.py @@ -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'] diff --git a/kallithea/controllers/changeset.py b/kallithea/controllers/changeset.py --- a/kallithea/controllers/changeset.py +++ b/kallithea/controllers/changeset.py @@ -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): diff --git a/kallithea/controllers/forks.py b/kallithea/controllers/forks.py --- a/kallithea/controllers/forks.py +++ b/kallithea/controllers/forks.py @@ -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): diff --git a/kallithea/controllers/home.py b/kallithea/controllers/home.py --- a/kallithea/controllers/home.py +++ b/kallithea/controllers/home.py @@ -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): """ diff --git a/kallithea/controllers/journal.py b/kallithea/controllers/journal.py --- a/kallithea/controllers/journal.py +++ b/kallithea/controllers/journal.py @@ -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 diff --git a/kallithea/controllers/pullrequests.py b/kallithea/controllers/pullrequests.py --- a/kallithea/controllers/pullrequests.py +++ b/kallithea/controllers/pullrequests.py @@ -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): diff --git a/kallithea/controllers/search.py b/kallithea/controllers/search.py --- a/kallithea/controllers/search.py +++ b/kallithea/controllers/search.py @@ -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 = [] diff --git a/kallithea/controllers/summary.py b/kallithea/controllers/summary.py --- a/kallithea/controllers/summary.py +++ b/kallithea/controllers/summary.py @@ -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): diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -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