Skip to content

Commit

Permalink
Feature/quicksight embed permission (#1396)
Browse files Browse the repository at this point in the history
* Enable embedded Quicksight for user

Add a Permission object to the user model to
represent access to the embedded Quicksight
Update the user detail page to allow enabling
of both versions of Quicksight
  • Loading branch information
michaeljcollinsuk authored Nov 27, 2024
1 parent d09451b commit 157dacf
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 26 deletions.
21 changes: 21 additions & 0 deletions controlpanel/api/migrations/0046_alter_user_options.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Generated by Django 5.1.2 on 2024-11-26 11:26

# Third-party
from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("api", "0045_task_retried_at"),
]

operations = [
migrations.AlterModelOptions(
name="user",
options={
"ordering": ("username",),
"permissions": [("quicksight_embed_access", "Can access the embedded Quicksight")],
},
),
]
2 changes: 1 addition & 1 deletion controlpanel/api/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
from controlpanel.api.models.s3bucket import S3Bucket
from controlpanel.api.models.task import Task
from controlpanel.api.models.tool import HomeDirectory, Tool, ToolDeployment
from controlpanel.api.models.user import User
from controlpanel.api.models.user import QUICKSIGHT_EMBED_PERMISSION, User
from controlpanel.api.models.userapp import UserApp
from controlpanel.api.models.users3bucket import UserS3Bucket
3 changes: 3 additions & 0 deletions controlpanel/api/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from controlpanel.api.signals import prometheus_login_event
from controlpanel.utils import sanitize_dns_label

QUICKSIGHT_EMBED_PERMISSION = "quicksight_embed_access"


class User(AbstractUser):

Expand Down Expand Up @@ -48,6 +50,7 @@ class User(AbstractUser):
class Meta:
db_table = "control_panel_api_user"
ordering = ("username",)
permissions = [(QUICKSIGHT_EMBED_PERMISSION, "Can access the embedded Quicksight")]

def __repr__(self):
return f"<User: {self.username} ({self.auth0_id})>"
Expand Down
1 change: 0 additions & 1 deletion controlpanel/api/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ def is_app_admin(user, obj):
add_perm("api.remove_app_bucket", is_authenticated & is_superuser) # TODO change to is_app_admin
add_perm("api.view_app_logs", is_authenticated & is_app_admin)
add_perm("api.manage_groups", is_authenticated & is_superuser)
add_perm("api.quicksight_embed_access", is_authenticated & is_superuser)
add_perm("api.create_policys3bucket", is_authenticated & is_superuser)
add_perm("api.update_app_settings", is_authenticated & is_app_admin)
add_perm("api.update_app_ip_allowlists", is_authenticated & is_app_admin)
Expand Down
38 changes: 37 additions & 1 deletion controlpanel/frontend/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

# Third-party
from django import forms
from django.contrib.auth.models import Permission
from django.contrib.postgres.forms import SimpleArrayField
from django.core.exceptions import ValidationError
from django.core.validators import RegexValidator, validate_email
Expand All @@ -12,7 +13,14 @@
from controlpanel.api.cluster import AWSRoleCategory
from controlpanel.api.cluster import S3Folder as ClusterS3Folder
from controlpanel.api.github import GithubAPI, RepositoryNotFound, extract_repo_info_from_url
from controlpanel.api.models import App, S3Bucket, Tool, User, UserS3Bucket
from controlpanel.api.models import (
QUICKSIGHT_EMBED_PERMISSION,
App,
S3Bucket,
Tool,
User,
UserS3Bucket,
)
from controlpanel.api.models.access_to_s3bucket import S3BUCKET_PATH_REGEX
from controlpanel.api.models.iam_managed_policy import POLICY_NAME_REGEX
from controlpanel.api.models.ip_allowlist import IPAllowlist
Expand Down Expand Up @@ -606,3 +614,31 @@ class CreateParameterForm(forms.Form):
],
)
value = forms.CharField(widget=forms.PasswordInput)


class QuicksightAccessForm(forms.Form):
QUICKSIGHT_LEGACY = "quicksight_legacy"
QUICKSIGHT_COMPUTE = "quicksight_compute"

enable_quicksight = forms.MultipleChoiceField(
choices=[
(QUICKSIGHT_LEGACY, "Legacy"),
(QUICKSIGHT_COMPUTE, "Compute"),
],
widget=forms.CheckboxSelectMultiple,
required=False,
)

def __init__(self, *args, **kwargs):
self.user = kwargs.pop("user")
super().__init__(*args, **kwargs)

def grant_access(self):
quicksight_access = self.cleaned_data["enable_quicksight"]
self.user.set_quicksight_access(enable=self.QUICKSIGHT_LEGACY in quicksight_access)

permission = Permission.objects.get(codename=QUICKSIGHT_EMBED_PERMISSION)
if self.QUICKSIGHT_COMPUTE in quicksight_access:
self.user.user_permissions.add(permission)
else:
self.user.user_permissions.remove(permission)
2 changes: 1 addition & 1 deletion controlpanel/frontend/jinja2/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
"active": page_name == "parameters",
},
{
"hide": not request.user.is_superuser,
"hide": not request.user.has_perm("api.quicksight_embed_access"),
"text": "QuickSight",
"href": url("quicksight"),
"active": page_name == "quicksight",
Expand Down
16 changes: 12 additions & 4 deletions controlpanel/frontend/jinja2/user-detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

{% set page_title = user_name(user) %}
{% set pronoun = "Your" if user == request.user else "User's" %}
{% set env = settings.ENV %}

{% block content %}
<span class="govuk-caption-xl">User</span>
Expand Down Expand Up @@ -109,21 +110,28 @@ <h2 class="govuk-heading-m govuk-error-summary-heading">
"name": "enable_quicksight",
"fieldset": {
"legend": {
"text": "Enable Quicksight",
"text": "Quicksight access",
"classes": "govuk-fieldset__legend--m",
},
},
"hint": {
"text": "Toggle access to Quicksight for a user."
"text": "Update Quicksight access for a user."
},
"items": [
{
"value": "True",
"text": "Quicksight Enabled",
"value": "quicksight_legacy",
"text": "Legacy Quicksight access enabled (running in the data-" + env + " account)",
"checked": user.is_quicksight_enabled
},
{
"value": "quicksight_compute",
"text": "Embedded Quicksight enabled (running in the compute-" + env + " account)",
"checked": user.has_perm("api.quicksight_embed_access"),
"disabled": user.is_superuser
},
]
}) }}
<p class="govuk-body">Note: all super admins have access to the embedded Quicksight.</p>
<button class="govuk-button govuk-button--secondary">
Save changes
</button>
Expand Down
25 changes: 18 additions & 7 deletions controlpanel/frontend/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,22 @@

# Third-party
from django.contrib import messages
from django.contrib.auth.models import Permission
from django.forms import BaseModelForm
from django.http import HttpResponse, HttpResponseRedirect
from django.shortcuts import get_object_or_404
from django.urls import reverse_lazy
from django.views import View
from django.views.generic import FormView
from django.views.generic.base import RedirectView
from django.views.generic.detail import DetailView, SingleObjectMixin
from django.views.generic.edit import DeleteView, UpdateView
from django.views.generic.edit import DeleteView, FormView, UpdateView
from django.views.generic.list import ListView
from rules.contrib.views import PermissionRequiredMixin

# First-party/Local
from controlpanel.api.cluster import User as ClusterUser
from controlpanel.api.models import User
from controlpanel.frontend import forms
from controlpanel.frontend.mixins import PolicyAccessMixin
from controlpanel.oidc import OIDCLoginRequiredMixin

Expand Down Expand Up @@ -111,15 +112,25 @@ class EnableBedrockUser(PolicyAccessMixin, UpdateView):
method_name = "set_bedrock_access"


class SetQuicksightAccess(OIDCLoginRequiredMixin, PermissionRequiredMixin, View):
class SetQuicksightAccess(OIDCLoginRequiredMixin, PermissionRequiredMixin, FormView):
permission_required = "api.add_superuser"
http_method_names = ["post"]
form_class = forms.QuicksightAccessForm

def post(self, request, *args, **kwargs):
user = get_object_or_404(User, pk=kwargs["pk"])
user.set_quicksight_access(enable="enable_quicksight" in request.POST)
def get_form_kwargs(self):
kwargs = super().get_form_kwargs()
user = get_object_or_404(User, pk=self.kwargs["pk"])
kwargs.update({"user": user})
return kwargs

def form_valid(self, form):
form.grant_access()
messages.success(self.request, "Successfully updated Quicksight access")
return HttpResponseRedirect(reverse_lazy("manage-user", kwargs={"pk": user.auth0_id}))
return HttpResponseRedirect(reverse_lazy("manage-user", kwargs={"pk": form.user.auth0_id}))

def form_invalid(self, form):
messages.error(self.request, "Failed to update Quicksight access")
return HttpResponseRedirect(reverse_lazy("manage-user", kwargs={"pk": form.user.auth0_id}))


class ReinitialiseUser(OIDCLoginRequiredMixin, PermissionRequiredMixin, View):
Expand Down
2 changes: 1 addition & 1 deletion tests/api/views/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def ExtendedAuth0():
def test_list(client, users):
response = client.get(reverse("user-list"))
assert response.status_code == status.HTTP_200_OK
assert len(response.data["results"]) == 4
assert len(response.data["results"]) == 5


def test_detail(client, users):
Expand Down
19 changes: 18 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import pytest
import yaml
from django.conf import settings
from django.contrib.auth.models import Permission
from model_bakery import baker

# First-party/Local
from controlpanel.api import auth0
from controlpanel.api.models import QUICKSIGHT_EMBED_PERMISSION
from controlpanel.utils import load_app_conf_from_file
from tests.api.fixtures.aws import *
from tests.api.fixtures.helm_mojanalytics_index import HELM_MOJANALYTICS_INDEX
Expand Down Expand Up @@ -99,7 +101,21 @@ def superuser(db, slack_WebClient, iam, managed_policy, airflow_dev_policy, airf


@pytest.fixture
def users(db, superuser, iam, managed_policy, airflow_dev_policy, airflow_prod_policy):
def quicksight_user(db):
user = baker.make(
"api.User",
auth0_id="github|user_5",
username="foobar",
is_superuser=False,
)
user.user_permissions.add(Permission.objects.get(codename=QUICKSIGHT_EMBED_PERMISSION))
return user


@pytest.fixture
def users(
db, superuser, iam, managed_policy, airflow_dev_policy, airflow_prod_policy, quicksight_user
):
return {
"superuser": superuser,
"normal_user": baker.make(
Expand All @@ -120,6 +136,7 @@ def users(db, superuser, iam, managed_policy, airflow_dev_policy, airflow_prod_p
is_superuser=False,
is_database_admin=True,
),
"quicksight_user": quicksight_user,
}


Expand Down
3 changes: 1 addition & 2 deletions tests/frontend/views/test_quicksight.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ def quicksight(client):
(quicksight, "superuser", status.HTTP_200_OK),
(quicksight, "database_user", status.HTTP_403_FORBIDDEN),
(quicksight, "normal_user", status.HTTP_403_FORBIDDEN),
(quicksight, "quicksight_user", status.HTTP_200_OK),
],
)
def test_permission(client, users, view, user, expected_status):
for key, val in users.items():
client.force_login(val)
client.force_login(users[user])
response = view(client)
assert response.status_code == expected_status
23 changes: 16 additions & 7 deletions tests/frontend/views/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

# First-party/Local
from controlpanel.api import cluster
from controlpanel.api.models import QUICKSIGHT_EMBED_PERMISSION


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -111,7 +112,7 @@ def test_permission(client, users, view, user, expected_status):
@pytest.mark.parametrize(
"view,user,expected_count",
[
(list, "superuser", 4),
(list, "superuser", 5),
],
)
def test_list(client, users, view, user, expected_count):
Expand Down Expand Up @@ -139,15 +140,19 @@ def test_grant_superuser_access(client, users, slack):


@pytest.mark.parametrize(
"data, attach",
"data, legacy_access, compute_access",
[
({"enable_quicksight": True}, True),
({}, False),
({"enable_quicksight": ["quicksight_legacy"]}, True, False),
({"enable_quicksight": ["quicksight_compute"]}, False, True),
({"enable_quicksight": ["quicksight_legacy", "quicksight_compute"]}, True, True),
({}, False, False),
],
ids=["attach", "remove"],
ids=["legacy enabled", "compute enabled", "both enabled", "no access"],
)
@patch("controlpanel.api.models.user.cluster.User.update_policy_attachment")
def test_enable_quicksight_access(update_policy_attachment, data, attach, client, users):
def test_enable_quicksight_access(
update_policy_attachment, data, legacy_access, compute_access, client, users
):
request_user = users["superuser"]
user = users["other_user"]
url = reverse("set-quicksight", kwargs={"pk": user.auth0_id})
Expand All @@ -158,5 +163,9 @@ def test_enable_quicksight_access(update_policy_attachment, data, attach, client
assert response.status_code == status.HTTP_302_FOUND
update_policy_attachment.assert_called_once_with(
policy=cluster.User.QUICKSIGHT_POLICY_NAME,
attach=attach,
attach=legacy_access,
)
assert (
user.user_permissions.filter(codename=QUICKSIGHT_EMBED_PERMISSION).exists()
is compute_access
)

0 comments on commit 157dacf

Please sign in to comment.