Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expire user permissions #72

Merged
merged 18 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions hq_superset/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
json_success,
)

from .models import DataSetChange
from .oauth2_server import authorization, require_oauth
from .tasks import process_dataset_change
from hq_superset.models import DataSetChange
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

from hq_superset.oauth2_server import authorization, require_oauth
from hq_superset.tasks import process_dataset_change


class OAuth(BaseApi):
Expand Down
5 changes: 5 additions & 0 deletions hq_superset/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,8 @@
"Datasets": [MENU_ACCESS_PERMISSION],
"ExploreFormDataRestApi": [CAN_READ_PERMISSION]
}

DOMAIN_PREFIX = "hqdomain_"
SESSION_USER_DOMAINS_KEY = "user_hq_domains"
SESSION_OAUTH_RESPONSE_KEY = "oauth_response"
SESSION_DOMAIN_ROLE_LAST_SYNCED_AT = "domain_role_last_synced_at"
83 changes: 78 additions & 5 deletions hq_superset/hq_domain.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,31 @@
from flask import flash, g, redirect, request, session, url_for
from datetime import timedelta

from .utils import SESSION_USER_DOMAINS_KEY
import superset
from flask import current_app, flash, g, redirect, request, session, url_for
from superset.config import USER_DOMAIN_ROLE_EXPIRY
from superset.extensions import cache_manager

from hq_superset.const import (
SESSION_DOMAIN_ROLE_LAST_SYNCED_AT,
SESSION_USER_DOMAINS_KEY,
)
from hq_superset.utils import DomainSyncUtil, datetime_utcnow


def before_request_hook():
return ensure_domain_selected()
"""
Call all hooks functions set in sequence and
if any hook returns a response,
break the chain and return that response
"""
hooks = [
ensure_domain_selected,
sync_user_domain_role
]
for _function in hooks:
response = _function()
if response:
return response


def after_request_hook(response):
Expand All @@ -13,7 +34,7 @@ def after_request_hook(response):
"AuthDBView.login",
"AuthOAuthView.logout",
]
if (request.url_rule and request.url_rule.endpoint in logout_views):
if request.url_rule and (request.url_rule.endpoint in logout_views):
response.set_cookie('hq_domain', '', expires=0)
return response

Expand All @@ -24,6 +45,8 @@ def after_request_hook(response):
'AuthOAuthView.login',
'AuthOAuthView.logout',
'AuthOAuthView.oauth_authorized',
'CurrentUserRestApi.get_me',
'Superset.log',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious what this is for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the UI, this a call made to constantly sync the user record, I assume its getting used in javascript/UI somewhere.
It is a domain independent API call.
I added it to this list so we don't fire the before request hooks on such requests since they are not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one more, b4eb1b3

'DataSetChangeAPI.post_dataset_change',
'OAuth.issue_access_token',
'SelectDomainView.list',
Expand Down Expand Up @@ -55,6 +78,57 @@ def ensure_domain_selected():
return redirect(url_for('SelectDomainView.list', next=request.url))


def sync_user_domain_role():
if is_user_admin() or (
request.url_rule
and request.url_rule.endpoint in DOMAIN_EXCLUDED_VIEWS
):
return
if _domain_role_expired():
# only sync if another sync not in progress
if not _sync_in_progress():
return _perform_sync_domain_role()


def _domain_role_expired():
if not session.get(SESSION_DOMAIN_ROLE_LAST_SYNCED_AT):
return True

time_since_last_sync = datetime_utcnow() - session[SESSION_DOMAIN_ROLE_LAST_SYNCED_AT]
return time_since_last_sync >= timedelta(minutes=USER_DOMAIN_ROLE_EXPIRY)


def _sync_in_progress():
return cache_manager.cache.get(_sync_domain_role_cache_key())


def _sync_domain_role_cache_key():
return f"{g.user.id}_{g.hq_domain}_sync_domain_role"


def _perform_sync_domain_role():
cache_key = _sync_domain_role_cache_key()

# set cache for 30 seconds
cache_manager.cache.set(cache_key, True, timeout=30)
sync_domain_role_response = _sync_domain_role()
cache_manager.cache.delete(cache_key)

return sync_domain_role_response

def _sync_domain_role():
if not DomainSyncUtil(superset.appbuilder.sm).sync_domain_role(g.hq_domain):
error_message = (
f"We couldn't refresh your permissions to access the domain '{g.hq_domain}'. "
f"Please select the project space again or login again to resolve. "
f"If issue persists, please submit a support request."
)
return current_app.response_class(
response=error_message,
status=400,
)


def is_valid_user_domain(hq_domain):
# Admins have access to all domains
return is_user_admin() or hq_domain in user_domains()
Expand All @@ -71,7 +145,6 @@ def user_domains():


def add_domain_links(active_domain, domains):
import superset
for domain in domains:
superset.appbuilder.menu.add_link(domain, category=active_domain, href=url_for('SelectDomainView.select', hq_domain=domain))

Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
"""
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa

from alembic import op

# revision identifiers, used by Alembic.
revision: str = '56d0467ff6ff'
Expand Down
10 changes: 7 additions & 3 deletions hq_superset/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@
from cryptography.fernet import MultiFernet
from superset import db

from .const import OAUTH2_DATABASE_NAME
from .exceptions import TableMissing
from .utils import cast_data_for_table, get_fernet_keys, get_hq_database
from hq_superset.const import OAUTH2_DATABASE_NAME
from hq_superset.exceptions import TableMissing
from hq_superset.utils import (
cast_data_for_table,
get_fernet_keys,
get_hq_database,
)


@dataclass
Expand Down
4 changes: 2 additions & 2 deletions hq_superset/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
from requests.exceptions import HTTPError
from superset.security import SupersetSecurityManager

from .exceptions import OAuthSessionExpired
from .utils import (
from hq_superset.const import (
SESSION_OAUTH_RESPONSE_KEY,
SESSION_USER_DOMAINS_KEY,
)
from hq_superset.exceptions import OAuthSessionExpired

logger = logging.getLogger(__name__)

Expand Down
2 changes: 1 addition & 1 deletion hq_superset/oauth2_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
)
from authlib.oauth2.rfc6749 import grants

from .models import OAuth2Client, OAuth2Token, db
from hq_superset.models import OAuth2Client, OAuth2Token, db


def save_token(token: dict, request: FlaskOAuth2Request) -> None:
Expand Down
10 changes: 5 additions & 5 deletions hq_superset/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@
from superset.extensions import cache_manager
from superset.sql_parse import Table

from .exceptions import HQAPIException
from .hq_requests import HQRequest
from .hq_url import (
from hq_superset.exceptions import HQAPIException
from hq_superset.hq_requests import HQRequest
from hq_superset.hq_url import (
datasource_details,
datasource_export,
datasource_subscribe,
datasource_unsubscribe,
)
from .models import OAuth2Client
from .utils import (
from hq_superset.models import OAuth2Client
from hq_superset.utils import (
convert_to_array,
generate_secret,
get_column_dtypes,
Expand Down
6 changes: 3 additions & 3 deletions hq_superset/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

from superset.extensions import celery_app

from .exceptions import TableMissing
from .models import DataSetChange
from .services import AsyncImportHelper, refresh_hq_datasource
from hq_superset.exceptions import TableMissing
from hq_superset.models import DataSetChange
from hq_superset.services import AsyncImportHelper, refresh_hq_datasource


@celery_app.task(name='refresh_hq_datasource_task')
Expand Down
36 changes: 34 additions & 2 deletions hq_superset/tests/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
"""
Base TestCase class
"""

import os
import shutil

import jwt
from flask_testing import TestCase
from sqlalchemy.sql import text
from superset.app import create_app

from hq_superset.utils import DOMAIN_PREFIX, get_hq_database
from hq_superset.const import DOMAIN_PREFIX
from hq_superset.tests.utils import OAuthMock
from hq_superset.utils import get_hq_database

superset_test_home = os.path.join(os.path.dirname(__file__), ".test_superset")
shutil.rmtree(superset_test_home, ignore_errors=True)
Expand Down Expand Up @@ -48,3 +50,33 @@ def tearDown(self):
sql = "; ".join(domain_schemas) + ";"
connection.execute(text(sql))
super(HQDBTestCase, self).tearDown()


class LoginUserTestMixin(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Seems useful.

"""
Use this mixin by calling login function with client
& then logout once done for clearing the session
"""
def login(self, client):
self._setup_user()
# bypass oauth-workflow by skipping login and oauth flow
with client.session_transaction() as session_:
session_["oauth_state"] = "mock_state"
state = jwt.encode({}, "mock_state", algorithm="HS256")
return client.get(f"/oauth-authorized/commcare?state={state}", follow_redirects=True)

def _setup_user(self):
self.app.appbuilder.add_permissions(update_perms=True)
self.app.appbuilder.sm.sync_role_definitions()

self.oauth_mock = OAuthMock()
self.app.appbuilder.sm.oauth_remotes = {"commcare": self.oauth_mock}

gamma_role = self.app.appbuilder.sm.find_role('Gamma')
self.user = self.app.appbuilder.sm.find_user(self.oauth_mock.user_json['username'])
if not self.user:
self.user = self.app.appbuilder.sm.add_user(**self.oauth_mock.user_json, role=[gamma_role])

@staticmethod
def logout(client):
return client.get("/logout/")
1 change: 1 addition & 0 deletions hq_superset/tests/config_for_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,4 @@
# CommCare Analytics extensions
FLASK_APP_MUTATOR = flask_app_mutator
CUSTOM_SECURITY_MANAGER = oauth.CommCareSecurityManager
USER_DOMAIN_ROLE_EXPIRY = 60 # minutes
13 changes: 13 additions & 0 deletions hq_superset/tests/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,16 @@
"id": "test1_ucr1",
"resource_uri": "/a/demo/api/v0.5/ucr_data_source/52a134da12c9b801bd85d2122901b30c/"
}

TEST_UCR_CSV_V1 = """\
doc_id,inserted_at,data_visit_date_eaece89e,data_visit_number_33d63739,data_lmp_date_5e24b993,data_visit_comment_fb984fda
a1, 2021-12-20, 2022-01-19, 100, 2022-02-20, some_text
a2, 2021-12-22, 2022-02-19, 10, 2022-03-20, some_other_text
"""

TEST_UCR_CSV_V2 = """\
doc_id,inserted_at,data_visit_date_eaece89e,data_visit_number_33d63739,data_lmp_date_5e24b993,data_visit_comment_fb984fda
a1, 2021-12-20, 2022-01-19, 100, 2022-02-20, some_text
a2, 2021-12-22, 2022-02-19, 10, 2022-03-20, some_other_text
a3, 2021-11-22, 2022-01-19, 10, 2022-03-20, some_other_text2
"""
5 changes: 2 additions & 3 deletions hq_superset/tests/test_hq_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from flask import g

from hq_superset.const import SESSION_USER_DOMAINS_KEY
from hq_superset.hq_domain import (
DOMAIN_EXCLUDED_VIEWS,
after_request_hook,
Expand All @@ -10,15 +11,13 @@
is_valid_user_domain,
user_domains,
)
from hq_superset.tests.base_test import HQDBTestCase, SupersetTestCase
from hq_superset.utils import (
SESSION_USER_DOMAINS_KEY,
DomainSyncUtil,
get_hq_database,
get_schema_name_for_domain,
)

from .base_test import HQDBTestCase, SupersetTestCase

MOCK_DOMAIN_SESSION = {
SESSION_USER_DOMAINS_KEY:[
{
Expand Down
9 changes: 4 additions & 5 deletions hq_superset/tests/test_oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@

from flask import session

from hq_superset.exceptions import OAuthSessionExpired
from hq_superset.oauth import get_valid_cchq_oauth_token
from hq_superset.utils import (
from hq_superset.const import (
SESSION_OAUTH_RESPONSE_KEY,
SESSION_USER_DOMAINS_KEY,
)

from .base_test import SupersetTestCase
from hq_superset.exceptions import OAuthSessionExpired
from hq_superset.oauth import get_valid_cchq_oauth_token
from hq_superset.tests.base_test import SupersetTestCase


class MockResponse:
Expand Down
32 changes: 27 additions & 5 deletions hq_superset/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import doctest
from unittest.mock import patch

from hq_superset.utils import get_column_dtypes, DomainSyncUtil
from .base_test import SupersetTestCase
from .const import TEST_DATASOURCE
from hq_superset.const import READ_ONLY_ROLE_NAME
from flask import session

from hq_superset.const import READ_ONLY_ROLE_NAME, SESSION_DOMAIN_ROLE_LAST_SYNCED_AT
from hq_superset.tests.base_test import LoginUserTestMixin, SupersetTestCase
from hq_superset.tests.const import TEST_DATASOURCE
from hq_superset.utils import DomainSyncUtil, get_column_dtypes


def test_get_column_dtypes():
Expand All @@ -28,7 +30,7 @@ def test_doctests():
assert results.failed == 0


class TestDomainSyncUtil(SupersetTestCase):
class TestDomainSyncUtil(LoginUserTestMixin, SupersetTestCase):
PLATFORM_ROLE_NAMES = ["Gamma", "sql_lab", "dataset_editor"]

@patch.object(DomainSyncUtil, "_get_domain_access")
Expand Down Expand Up @@ -94,6 +96,26 @@ def test_permissions_change_updates_user_role(self, get_domain_access_mock):
additional_roles = DomainSyncUtil(security_manager)._get_additional_user_roles("test-domain")
assert not additional_roles

@patch('hq_superset.utils.datetime_utcnow')
@patch.object(DomainSyncUtil, "_get_domain_access")
def test_sync_domain_role(self, get_domain_access_mock, utcnow_mock):
client = self.app.test_client()
self.login(client)

utcnow_mock_return = "2024-11-01 14:30:04.323000+00:00"
utcnow_mock.return_value = utcnow_mock_return
get_domain_access_mock.return_value = self._to_permissions_response(
can_write=False,
can_read=True,
roles=[],
)
security_manager = self.app.appbuilder.sm

self.assertIsNone(session.get(SESSION_DOMAIN_ROLE_LAST_SYNCED_AT))
DomainSyncUtil(security_manager).sync_domain_role("test-domain")
self.assertEqual(session[SESSION_DOMAIN_ROLE_LAST_SYNCED_AT], utcnow_mock_return)
self.logout(client)

def _ensure_platform_roles_exist(self, sm):
for role_name in self.PLATFORM_ROLE_NAMES:
sm.add_role(role_name)
Expand Down
Loading
Loading