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

fix: update permissions on classes with missing / unclear permissions #4667

Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions api/integrations/slack/permissions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.shortcuts import get_object_or_404
from rest_framework.permissions import IsAuthenticated

from environments.models import Environment
Expand All @@ -12,3 +13,15 @@ def has_permission(self, request, view):
api_key=view.kwargs.get("environment_api_key")
)
return request.user.is_environment_admin(environment)


class SlackGetChannelPermissions(IsAuthenticated):
def has_permission(self, request, view):
if not super().has_permission(request, view): # pragma: no cover
return False

environment = get_object_or_404(
Environment.objects.select_related("project"),
api_key=view.kwargs.get("environment_api_key"),
)
return request.user.is_project_admin(environment.project)
3 changes: 2 additions & 1 deletion api/integrations/slack/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@
InvalidStateError,
SlackConfigurationDoesNotExist,
)
from .permissions import OauthInitPermission
from .permissions import OauthInitPermission, SlackGetChannelPermissions

signer = TimestampSigner()


class SlackGetChannelsViewSet(GenericViewSet):
serializer_class = SlackChannelListSerializer
pagination_class = None # set here to ensure documentation is correct
permission_classes = [SlackGetChannelPermissions]

def get_api_token(self) -> str:
environment = Environment.objects.get(
Expand Down
5 changes: 2 additions & 3 deletions api/sales_dashboard/urls.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.contrib.admin.views.decorators import staff_member_required
from django.urls import path

from . import views
Expand All @@ -7,7 +6,7 @@


urlpatterns = [
path("", staff_member_required(views.OrganisationList.as_view()), name="index"),
path("", views.OrganisationList.as_view(), name="index"),
path(
"organisations/<int:organisation_id>",
views.organisation_info,
Expand Down Expand Up @@ -40,7 +39,7 @@
),
path(
"email-usage/",
staff_member_required(views.EmailUsage.as_view()),
views.EmailUsage.as_view(),
name="email-usage",
),
path(
Expand Down
13 changes: 13 additions & 0 deletions api/sales_dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from django.template import loader
from django.urls import reverse, reverse_lazy
from django.utils import timezone
from django.utils.decorators import method_decorator
from django.utils.safestring import mark_safe
from django.views.generic import ListView
from django.views.generic.edit import FormView
Expand Down Expand Up @@ -54,6 +55,10 @@
email_regex = re.compile(r"^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$")


@method_decorator(
name="get",
decorator=staff_member_required(),
)
class OrganisationList(ListView):
model = Organisation
paginate_by = OBJECTS_PER_PAGE
Expand Down Expand Up @@ -261,6 +266,14 @@ def migrate_identities_to_edge(request, project_id):
return HttpResponseRedirect(reverse("sales_dashboard:index"))


@method_decorator(
name="get",
decorator=staff_member_required(),
)
@method_decorator(
name="post",
decorator=staff_member_required(),
)
class EmailUsage(FormView):
form_class = EmailUsageForm
template_name = "sales_dashboard/email-usage.html"
Expand Down
19 changes: 19 additions & 0 deletions api/tests/integration/slack/test_slack_get_channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,25 @@

from django.urls import reverse
from rest_framework import status
from rest_framework.test import APIClient

from environments.models import Environment


def test_get_channels_fails_if_user_has_no_permission(
staff_client: APIClient, environment: Environment, environment_api_key: str
) -> None:
# Given
url = reverse(
"api-v1:environments:integrations-slack-channels-list",
args=[environment_api_key],
)

# When
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_403_FORBIDDEN


def test_get_channels_returns_400_when_slack_project_config_does_not_exist(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,57 @@ def test_list_organisations_filter_plan(
# Then
assert response.status_code == 200
assert list(response.context_data["organisation_list"]) == [organisation]


def test_list_organisations_fails_if_not_staff(
organisation: Organisation,
client: Client,
) -> None:
# Given
user = FFAdminUser.objects.create(email="[email protected]")
client.force_login(user)

url = reverse("sales_dashboard:index")

# When
response = client.get(url)

# Then
assert response.status_code == 302
assert response.url == "/admin/login/?next=/sales-dashboard/"


def test_get_email_usage_fails_if_not_staff(
organisation: Organisation,
client: Client,
) -> None:
# Given
user = FFAdminUser.objects.create(email="[email protected]")
client.force_login(user)

url = reverse("sales_dashboard:email-usage")

# When
response = client.get(url)

# Then
assert response.status_code == 302
assert response.url == "/admin/login/?next=/sales-dashboard/email-usage/"


def test_post_email_usage_fails_if_not_staff(
organisation: Organisation,
client: Client,
) -> None:
# Given
user = FFAdminUser.objects.create(email="[email protected]")
client.force_login(user)

url = reverse("sales_dashboard:email-usage")

# When
response = client.post(url)

# Then
assert response.status_code == 302
assert response.url == "/admin/login/?next=/sales-dashboard/email-usage/"
Loading