Using new UserClass instances everywhere:

- No more direct access to g.current_user, unless unavoidable.
  - Using pillar.auth.current_user instead of g.current_user or
    flask_login.current_user.
  - p.a.current_user is never checked against None.
  - p.a.current_user.is_authenticated or is_anonymous is used, and never
    together with a negation (instead of 'not is_anon' use 'is_auth').
  - No more accessing current_user a a dict.
  - No more checks for admin role, use capability check instead.
This commit is contained in:
Sybren A. Stüvel 2017-08-29 11:34:39 +02:00
parent 86e76aaa5f
commit bdd603fb17
12 changed files with 113 additions and 87 deletions

View File

@ -1,7 +1,8 @@
import logging
from flask import g, request, current_app
from flask import request, current_app
from pillar.api.utils import gravatar
from pillar.auth import current_user
log = logging.getLogger(__name__)
@ -30,7 +31,7 @@ def notification_parse(notification):
object_name = ''
object_id = activity['object']
if node['parent']['user'] == g.current_user['user_id']:
if node['parent']['user'] == current_user.user_id:
owner = "your {0}".format(node['parent']['node_type'])
else:
parent_comment_user = users_collection.find_one(
@ -52,7 +53,7 @@ def notification_parse(notification):
action = activity['verb']
lookup = {
'user': g.current_user['user_id'],
'user': current_user.user_id,
'context_object_type': 'node',
'context_object': context_object_id,
}

View File

@ -4,7 +4,7 @@ import logging
import datetime
from bson import ObjectId, tz_util
from eve.methods.get import get
from flask import Blueprint, g, current_app, request
from flask import Blueprint, current_app, request
from pillar.api import utils
from pillar.api.utils import authentication, authorization
from werkzeug import exceptions as wz_exceptions
@ -201,8 +201,10 @@ def home_project():
Eve projections are supported, but at least the following fields must be present:
'permissions', 'category', 'user'
"""
user_id = g.current_user['user_id']
roles = g.current_user.get('roles', ())
from pillar.auth import current_user
user_id = current_user.user_id
roles = current_user.roles
log.debug('Possibly creating home project for user %s with roles %s', user_id, roles)
if HOME_PROJECT_USERS and not HOME_PROJECT_USERS.intersection(roles):

View File

@ -3,12 +3,14 @@ import logging
from eve.methods.get import get
from eve.utils import config as eve_config
from flask import Blueprint, request, current_app, g
from flask import Blueprint, request, current_app
from werkzeug.datastructures import MultiDict
from werkzeug.exceptions import InternalServerError
from pillar.api import utils
from pillar.api.utils.authentication import current_user_id
from pillar.api.utils.authorization import require_login
from werkzeug.datastructures import MultiDict
from werkzeug.exceptions import InternalServerError
from pillar.auth import current_user
FIRST_ADDON_VERSION_WITH_HDRI = (1, 4, 0)
TL_PROJECTION = utils.dumps({'name': 1, 'url': 1, 'permissions': 1,})
@ -25,8 +27,8 @@ log = logging.getLogger(__name__)
def keep_fetching_texture_libraries(proj_filter):
groups = g.current_user['groups']
user_id = g.current_user['user_id']
groups = current_user.group_ids
user_id = current_user.user_id
page = 1
max_page = float('inf')

View File

@ -18,7 +18,6 @@ import werkzeug.datastructures
from bson import ObjectId
from flask import Blueprint
from flask import current_app
from flask import g
from flask import jsonify
from flask import request
from flask import send_from_directory
@ -34,6 +33,7 @@ from pillar.api.utils.cdn import hash_file_path
from pillar.api.utils.encoding import Encoder
from pillar.api.utils.imaging import generate_local_thumbnails
from pillar.api.file_storage_backends import default_storage_backend, Bucket
from pillar.auth import current_user
log = logging.getLogger(__name__)
@ -387,10 +387,11 @@ def before_returning_file(response):
def strip_link_and_variations(response):
# Check the access level of the user.
if g.current_user is None:
if current_user.is_anonymous:
has_full_access = False
else:
user_roles = g.current_user['roles']
user_roles = current_user.roles
# TODO: convert to a capability and check for that.
access_roles = current_app.config['FULL_FILE_ACCESS_ROLES']
has_full_access = bool(user_roles.intersection(access_roles))
@ -598,12 +599,10 @@ def create_file_doc(name, filename, content_type, length, project,
if backend is None:
backend = current_app.config['STORAGE_BACKEND']
current_user = g.get('current_user')
file_doc = {'name': name,
'filename': filename,
'file_path': '',
'user': current_user['user_id'],
'user': current_user.user_id,
'backend': backend,
'md5': '',
'content_type': content_type,
@ -666,7 +665,7 @@ def assert_file_size_allowed(file_size: int):
filesize_limit_mb = filesize_limit / 2.0 ** 20
log.info('User %s tried to upload a %.3f MiB file, but is only allowed '
'%.3f MiB.',
authentication.current_user_id(), file_size / 2.0 ** 20,
current_user.user_id, file_size / 2.0 ** 20,
filesize_limit_mb)
raise wz_exceptions.RequestEntityTooLarge(
'To upload files larger than %i MiB, subscribe to Blender Cloud' %
@ -685,7 +684,7 @@ def stream_to_storage(project_id: str):
raise wz_exceptions.NotFound('Project %s does not exist' % project_id)
log.info('Streaming file to bucket for project=%s user_id=%s', project_id,
authentication.current_user_id())
current_user.user_id)
log.info('request.headers[Origin] = %r', request.headers.get('Origin'))
log.info('request.content_length = %r', request.content_length)

View File

@ -6,7 +6,7 @@ import urllib.parse
import pymongo.errors
import werkzeug.exceptions as wz_exceptions
from bson import ObjectId
from flask import current_app, g, Blueprint, request
from flask import current_app, Blueprint, request
import pillar.markdown
from pillar.api.activities import activity_subscribe, activity_object_add
@ -205,6 +205,8 @@ def before_inserting_nodes(items):
"""Before inserting a node in the collection we check if the user is allowed
and we append the project id to it.
"""
from pillar.auth import current_user
nodes_collection = current_app.data.driver.db['nodes']
def find_parent_project(node):
@ -226,7 +228,7 @@ def before_inserting_nodes(items):
item['project'] = project['_id']
# Default the 'user' property to the current user.
item.setdefault('user', g.current_user['user_id'])
item.setdefault('user', current_user.user_id)
def after_inserting_nodes(items):

View File

@ -2,11 +2,13 @@ import json
import logging
from bson import ObjectId
from flask import Blueprint, g, request, current_app, make_response, url_for
from flask import Blueprint, request, current_app, make_response, url_for
from werkzeug import exceptions as wz_exceptions
from pillar.api.utils import authorization, jsonify, str2id
from pillar.api.utils import mongo
from pillar.api.utils.authorization import require_login, check_permissions
from werkzeug import exceptions as wz_exceptions
from pillar.auth import current_user
from . import utils
@ -24,7 +26,7 @@ def create_project(overrides=None):
project_name = request.json['name']
else:
project_name = request.form['project_name']
user_id = g.current_user['user_id']
user_id = current_user.user_id
project = utils.create_new_project(project_name, user_id, overrides)
@ -62,7 +64,7 @@ def project_manage_users():
project_id = str2id(data['project_id'])
target_user_id = str2id(data['user_id'])
action = data['action']
current_user_id = g.current_user['user_id']
current_user_id = current_user.user_id
project = projects_collection.find_one({'_id': project_id})

View File

@ -3,12 +3,12 @@ import json
import bson
from eve.utils import parse_request
from flask import g
from werkzeug import exceptions as wz_exceptions
from pillar import current_app
from pillar.api.users.routes import log
from pillar.api.utils.authorization import user_has_role
import pillar.auth
USER_EDITABLE_FIELDS = {'full_name', 'username', 'email', 'settings'}
@ -33,7 +33,7 @@ def before_replacing_user(request, lookup):
# Reset fields that shouldn't be edited to their original values. This is only
# needed when users are editing themselves; admins are allowed to edit much more.
if not user_has_role('admin'):
if not pillar.auth.current_user.has_cap('admin'):
for db_key, db_value in original.items():
if db_key[0] == '_' or db_key in USER_EDITABLE_FIELDS:
continue
@ -89,33 +89,31 @@ def send_blinker_signal_roles_changed(user, original):
def check_user_access(request, lookup):
"""Modifies the lookup dict to limit returned user info."""
# No access when not logged in.
current_user = g.get('current_user')
current_user_id = current_user['user_id'] if current_user else None
user = pillar.auth.get_current_user()
# Admins can do anything and get everything, except the 'auth' block.
if user_has_role('admin'):
if user.has_cap('admin'):
return
if not lookup and not current_user:
if not lookup and user.is_anonymous:
raise wz_exceptions.Forbidden()
# Add a filter to only return the current user.
if '_id' not in lookup:
lookup['_id'] = current_user['user_id']
lookup['_id'] = user.user_id
def check_put_access(request, lookup):
"""Only allow PUT to the current user, or all users if admin."""
if user_has_role('admin'):
user = pillar.auth.get_current_user()
if user.has_cap('admin'):
return
current_user = g.get('current_user')
if not current_user:
if user.is_anonymous:
raise wz_exceptions.Forbidden()
if str(lookup['_id']) != str(current_user['user_id']):
if str(lookup['_id']) != str(user.user_id):
raise wz_exceptions.Forbidden()
@ -124,15 +122,14 @@ def after_fetching_user(user):
# custom end-points.
user.pop('auth', None)
current_user = g.get('current_user')
current_user_id = current_user['user_id'] if current_user else None
current_user = pillar.auth.get_current_user()
# Admins can do anything and get everything, except the 'auth' block.
if user_has_role('admin'):
if current_user.has_cap('admin'):
return
# Only allow full access to the current user.
if str(user['_id']) == str(current_user_id):
if current_user.is_authenticated and str(user['_id']) == str(current_user.user_id):
return
# Remove all fields except public ones.

View File

@ -1,9 +1,11 @@
import logging
from eve.methods.get import get
from flask import g, Blueprint
from flask import Blueprint
from pillar.api.utils import jsonify
from pillar.api.utils.authorization import require_login
from pillar.auth import current_user
log = logging.getLogger(__name__)
blueprint_api = Blueprint('users_api', __name__)
@ -12,7 +14,7 @@ blueprint_api = Blueprint('users_api', __name__)
@blueprint_api.route('/me')
@require_login()
def my_info():
eve_resp, _, _, status, _ = get('users', {'_id': g.current_user['user_id']})
eve_resp, _, _, status, _ = get('users', {'_id': current_user.user_id})
resp = jsonify(eve_resp['_items'][0], status=status)
return resp

View File

@ -113,6 +113,8 @@ def validate_token():
@returns True iff the user is logged in with a valid Blender ID token.
"""
from pillar.auth import force_logout_user
if request.authorization:
token = request.authorization.username
oauth_subclient = request.authorization.password
@ -129,7 +131,7 @@ def validate_token():
# If no authorization headers are provided, we are getting a request
# from a non logged in user. Proceed accordingly.
log.debug('No authentication headers, so not logged in.')
g.current_user = None
force_logout_user()
return False
return validate_this_token(token, oauth_subclient) is not None
@ -142,9 +144,9 @@ def validate_this_token(token, oauth_subclient=None):
:rtype: dict
"""
from pillar.auth import UserClass
from pillar.auth import UserClass, force_logout_user
g.current_user = None
force_logout_user()
_delete_expired_tokens()
# Check the users to see if there is one with this Blender ID token.

View File

@ -45,6 +45,8 @@ def compute_allowed_methods(collection_name, resource, check_node_type=None):
:rtype: set
"""
import pillar.auth
# Check some input values.
if collection_name not in CHECK_PERMISSIONS_IMPLEMENTED_FOR:
raise ValueError('compute_allowed_methods only implemented for %s, not for %s',
@ -62,18 +64,18 @@ def compute_allowed_methods(collection_name, resource, check_node_type=None):
# Accumulate allowed methods from the user, group and world level.
allowed_methods = set()
current_user = getattr(g, 'current_user', None)
user = pillar.auth.get_current_user()
if current_user:
user_is_admin = is_admin(current_user)
if user.is_authenticated:
user_is_admin = is_admin(user)
# If the user is authenticated, proceed to compare the group permissions
for permission in computed_permissions.get('groups', ()):
if user_is_admin or permission['group'] in current_user['groups']:
if user_is_admin or permission['group'] in user.group_ids:
allowed_methods.update(permission['methods'])
for permission in computed_permissions.get('users', ()):
if user_is_admin or current_user['user_id'] == permission['user']:
if user_is_admin or user.user_id == permission['user']:
allowed_methods.update(permission['methods'])
# Check if the node is public or private. This must be set for non logged
@ -298,21 +300,24 @@ def require_login(require_roles=set(),
def decorator(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
if g.current_user is None:
import pillar.auth
current_user = pillar.auth.get_current_user()
if current_user.is_anonymous:
# We don't need to log at a higher level, as this is very common.
# Many browsers first try to see whether authentication is needed
# at all, before sending the password.
log.debug('Unauthenticated access to %s attempted.', func)
abort(403)
if require_roles and not g.current_user.matches_roles(require_roles, require_all):
if require_roles and not current_user.matches_roles(require_roles, require_all):
log.warning('User %s is authenticated, but does not have required roles %s to '
'access %s', g.current_user['user_id'], require_roles, func)
'access %s', current_user.user_id, require_roles, func)
abort(403)
if require_cap and not g.current_user.has_cap(require_cap):
if require_cap and not current_user.has_cap(require_cap):
log.warning('User %s is authenticated, but does not have required capability %s to '
'access %s', g.current_user.user_id, require_cap, func)
'access %s', current_user.user_id, require_cap, func)
abort(403)
return func(*args, **kwargs)
@ -356,16 +361,17 @@ def ab_testing(require_roles=set(),
def user_has_role(role, user=None):
"""Returns True iff the user is logged in and has the given role."""
from pillar.auth import UserClass
import pillar.auth
if user is None:
user = g.get('current_user')
if user is not None and not isinstance(user, UserClass):
raise TypeError(f'g.current_user should be instance of UserClass, not {type(user)}')
elif not isinstance(user, UserClass):
user = pillar.auth.get_current_user()
if user is not None and not isinstance(user, pillar.auth.UserClass):
raise TypeError(f'pillar.auth.current_user should be instance of UserClass, '
f'not {type(user)}')
elif not isinstance(user, pillar.auth.UserClass):
raise TypeError(f'user should be instance of UserClass, not {type(user)}')
if user is None:
if user.is_anonymous:
return False
return user.has_role(role)
@ -374,17 +380,14 @@ def user_has_role(role, user=None):
def user_has_cap(capability: str, user=None) -> bool:
"""Returns True iff the user is logged in and has the given capability."""
from pillar.auth import UserClass
import pillar.auth
assert capability
if user is None:
user = g.get('current_user')
user = pillar.auth.get_current_user()
if user is None:
return False
if not isinstance(user, UserClass):
if not isinstance(user, pillar.auth.UserClass):
raise TypeError(f'user should be instance of UserClass, not {type(user)}')
return user.has_cap(capability)
@ -402,19 +405,16 @@ def user_matches_roles(require_roles=set(),
returning True.
"""
from pillar.auth import UserClass
import pillar.auth
current_user: UserClass = g.get('current_user')
if current_user is None:
return False
user = pillar.auth.get_current_user()
if not isinstance(user, pillar.auth.UserClass):
raise TypeError(f'user should be instance of UserClass, not {type(user)}')
if not isinstance(current_user, UserClass):
raise TypeError(f'g.current_user should be instance of UserClass, not {type(current_user)}')
return current_user.matches_roles(require_roles, require_all)
return user.matches_roles(require_roles, require_all)
def is_admin(user):
"""Returns True iff the given user has the admin role."""
"""Returns True iff the given user has the admin capability."""
return user_has_role('admin', user)
return user_has_cap('admin', user)

View File

@ -12,9 +12,6 @@ from pillar import current_app
import bson
from ..api import utils
from ..api.utils import authentication
log = logging.getLogger(__name__)
# Mapping from user role to capabilities obtained by users with that role.
@ -48,6 +45,8 @@ class UserClass(flask_login.UserMixin):
def construct(cls, token: str, db_user: dict) -> 'UserClass':
"""Constructs a new UserClass instance from a Mongo user document."""
from ..api import utils
user = cls(token)
user.user_id = db_user['_id']
@ -170,6 +169,8 @@ def _load_user(token) -> typing.Union[UserClass, AnonymousUser]:
:returns: returns a UserClass instance if logged in, or an AnonymousUser() if not.
"""
from ..api.utils import authentication
db_user = authentication.validate_this_token(token)
if not db_user:
return AnonymousUser()
@ -206,6 +207,15 @@ def login_user(oauth_token: str, *, load_from_db=False):
g.current_user = user
def force_logout_user():
"""Resets the current user to an AnonymousUser instance."""
from flask import g
flask_login.logout_user()
g.current_user = flask_login.current_user._get_current_object()
def get_blender_id_oauth_token():
"""Returns a tuple (token, ''), for use with flask_oauthlib."""
@ -221,10 +231,15 @@ def get_blender_id_oauth_token():
return None
def _get_current_user() -> UserClass:
def get_current_user() -> UserClass:
"""Returns the current user as a UserClass instance.
Never returns None; returns an AnonymousUser() instance instead.
This function is intended to be used when pillar.auth.current_user is
accessed many times in the same scope. Calling this function is then
more efficient, since it doesn't have to resolve the LocalProxy for
each access to the returned object.
"""
from ..api.utils.authentication import current_user
@ -232,5 +247,5 @@ def _get_current_user() -> UserClass:
return current_user()
current_user: UserClass = LocalProxy(_get_current_user)
current_user: UserClass = LocalProxy(get_current_user)
"""The current user."""

View File

@ -2,9 +2,9 @@ import json
import logging
from werkzeug import exceptions as wz_exceptions
from flask import abort, Blueprint, current_app, flash, redirect, render_template, request, session,\
from flask import abort, Blueprint, flash, redirect, render_template, request, session,\
url_for
from flask_login import login_required, logout_user, current_user
from flask_login import login_required, logout_user
from pillarsdk import exceptions as sdk_exceptions
from pillarsdk.users import User
@ -15,6 +15,7 @@ from pillar.web import system_util
from pillar.api.local_auth import generate_and_store_token, get_local_user
from pillar.api.utils.authentication import find_user_in_db, upsert_user
from pillar.api.blender_cloud.subscription import update_subscription
from pillar.auth import current_user
from pillar.auth.oauth import OAuthSignIn, ProviderConfigurationMissing, ProviderNotImplemented, \
OAuthCodeNotProvided
from . import forms
@ -47,8 +48,9 @@ def oauth_authorize(provider):
@blueprint.route('/oauth/<provider>/authorized')
def oauth_callback(provider):
if not current_user.is_anonymous:
if current_user.is_authenticated:
return redirect(url_for('main.homepage'))
oauth = OAuthSignIn.get_provider(provider)
try:
oauth_user = oauth.callback()
@ -68,7 +70,7 @@ def oauth_callback(provider):
# Login user
pillar.auth.login_user(token['token'], load_from_db=True)
if provider == 'blender-id' and current_user is not None:
if provider == 'blender-id' and current_user.is_authenticated:
# Check with the store for user roles. If the user has an active subscription, we apply
# the 'subscriber' role
update_subscription()