Catch error when no user is set on request

[Charl1996]

When trying to access https://commcare-analytics-staging.dimagi.com/ the server responds with a 500. The logs show the error at the bottom.

The issue seems to be the fact that security_manager.is_admin() doesn't handle no user being set on the request. This PR is really only to catch this error and return False. I'm not sure why this wasn't picked up before.

Traceback (most recent call last):
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/flask/app.py", line 1482, in full_dispatch_request
    rv = self.preprocess_request()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/flask/app.py", line 1974, in preprocess_request
    rv = self.ensure_sync(before_func)()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/hq_superset/hq_domain.py", line 7, in before_request_hook
    return ensure_domain_selected()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/hq_superset/hq_domain.py", line 44, in ensure_domain_selected
    if is_user_admin() or (
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/hq_superset/hq_domain.py", line 38, in is_user_admin
    return security_manager.is_admin()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/superset/security/manager.py", line 2340, in is_admin
    role.name for role in self.get_user_roles()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/superset/security/manager.py", line 2025, in get_user_roles
    if user.is_anonymous:
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/werkzeug/local.py", line 311, in __get__
    obj = instance._get_current_object()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/werkzeug/local.py", line 515, in _get_current_object
    return get_name(local())  # type: ignore
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/flask_login/utils.py", line 25, in <lambda>
    current_user = LocalProxy(lambda: _get_user())
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/flask_login/utils.py", line 370, in _get_user
    current_app.login_manager._load_user()
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/flask_login/login_manager.py", line 364, in _load_user
    user = self._user_callback(user_id)
  File "/home/ubuntu/www/.virtualenvs/superset/lib/python3.10/site-packages/flask_appbuilder/security/manager.py", line 2158, in load_user
    if user.is_active:
AttributeError: 'NoneType' object has no attribute 'is_active'

[kaapstorm]

It bothers me that we have to catch this error, because it's not in our codebase.

What if we first check the user, like so?

Suggested change

	try:
	from superset import security_manager
	return security_manager.is_admin()
	except AttributeError:
	return False
	from superset import security_manager
	from flask import g
	return g.user and security_manager.is_admin()

[Charl1996]

@kaapstorm

It bothers me that we have to catch this error, because it's not in our codebase.

Yeah, but at this stage I'm more prone getting it to work in our code.

The g.user is the part where it fails hard. Just running g.user will raise the AttributeError.

[mkangia]

Does security manager provide another method on it that we can call to check if there is a user or not before calling is_admin?

[mkangia]

Can we instead at least do this

try:
    # Check if there is a user first before checking for admin via security manager
    g.user
except AttributeError:
    return False
return security_manager.is_admin()

[mkangia]

Does security manager provide another method on it that we can call to check if there is a user or not before calling is_admin?

[Charl1996]

@mkangia

Does security manager provide another method on it that we can call to check if there is a user or not before calling is_admin?

There is a get_user_by_username method where we can try and retrieve the admin user by username, but I don't think that a good solution as we'd have to store the admin user's username in an env file. And what if there's two admin users? It just becomes messy.

[Charl1996]

Update: the issue was related to the SECRET_KEY not being property configured, hence the user id in the session was wrong.

[mkangia]

Update: the issue was related to the SECRET_KEY not being property configured, hence the user id in the session was wrong.

Glad that Norman and I pushing back on the change, motivated you to look deeper and find this route cause @Charl1996 🥇

[Charl1996]

@mkangia

True. Sometimes it's worth spending that extra bit of time, even though it's important to get solutions out. Priority != sloppy solution.

Anycase, thanks!