From ce0cdea492dead22e23a1888abd9b7ff4da20513 Mon Sep 17 00:00:00 2001 From: "Duong (Danny) Luu" <51145179+lcduong@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:54:13 +0700 Subject: [PATCH] Ensure SSO works on admin level (#237) * Ensure SSO works on admin level and change implementation to be an external plugin * remove unsed template * fix pipeline * using provider name constant instead of text * update transaction for storing key * handle case sso_provider not configured --------- Co-authored-by: odkhang --- MANIFEST.in | 1 + .../templates/eventyay_common/sso/detail.html | 26 ++++++ src/pretalx/eventyay_common/views/auth.py | 22 ++++- src/pretalx/eventyay_common/views/sso.py | 91 +++++++++++++++++++ src/pretalx/orga/forms/sso_client_form.py | 14 +-- src/pretalx/orga/templates/orga/base.html | 3 + .../orga/templates/orga/organiser/detail.html | 11 --- .../orga/organiser/organiser_sso.html | 26 ------ src/pretalx/orga/urls.py | 14 +++ src/pretalx/orga/views/organiser.py | 66 +------------- src/pretalx/settings.py | 5 +- 11 files changed, 158 insertions(+), 121 deletions(-) create mode 100644 src/pretalx/eventyay_common/templates/eventyay_common/sso/detail.html create mode 100644 src/pretalx/eventyay_common/views/sso.py delete mode 100644 src/pretalx/orga/templates/orga/organiser/organiser_sso.html diff --git a/MANIFEST.in b/MANIFEST.in index d1320dc8e..34dfc35f1 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -29,6 +29,7 @@ recursive-include src/pretalx/mail/templates * recursive-include src/pretalx/orga/templates * recursive-include src/pretalx/schedule/templates * recursive-include src/pretalx/sso_provider/templates * +recursive-include src/pretalx/eventyay_common/templates * recursive-exclude src/tests * recursive-include src *.py diff --git a/src/pretalx/eventyay_common/templates/eventyay_common/sso/detail.html b/src/pretalx/eventyay_common/templates/eventyay_common/sso/detail.html new file mode 100644 index 000000000..4915dd784 --- /dev/null +++ b/src/pretalx/eventyay_common/templates/eventyay_common/sso/detail.html @@ -0,0 +1,26 @@ +{% extends "orga/base.html" %} +{% load i18n %} +{% load rules %} +{% block extra_title %}{% translate "SSO settings" %} :: {% endblock extra_title %} +{% block content %} +

{% translate "SSO client settings" %}

+
+ {% csrf_token %} + {{ form }} +
+ + {% if sso_provider %} + + {% translate "Delete key" %} + + {% endif %} + + + + +
+
+{% endblock content %} diff --git a/src/pretalx/eventyay_common/views/auth.py b/src/pretalx/eventyay_common/views/auth.py index a0c7dc996..444fb5c60 100644 --- a/src/pretalx/eventyay_common/views/auth.py +++ b/src/pretalx/eventyay_common/views/auth.py @@ -1,7 +1,9 @@ import logging import os +from allauth.socialaccount.models import SocialApp from django.conf import settings +from django.contrib import messages from django.contrib.auth import login from django.shortcuts import redirect from django.urls import reverse @@ -19,9 +21,16 @@ def oauth2_login_view(request, *args, **kwargs): + sso_provider = SocialApp.objects.filter( + provider=settings.EVENTYAY_SSO_PROVIDER + ).first() + if not sso_provider: + messages.error(request, "SSO not configured yet, please contact the " + "administrator or come back later.") + return redirect(reverse("orga:login")) # Create an OAuth2 session using the client ID and redirect URI oauth2_session = OAuth2Session( - client_id=settings.OAUTH2_PROVIDER["CLIENT_ID"], + client_id=sso_provider.client_id, redirect_uri=settings.OAUTH2_PROVIDER["REDIRECT_URI"], scope=settings.OAUTH2_PROVIDER["SCOPE"], ) @@ -39,8 +48,15 @@ def oauth2_login_view(request, *args, **kwargs): def oauth2_callback(request): + sso_provider = SocialApp.objects.filter( + provider=settings.EVENTYAY_SSO_PROVIDER + ).first() + if not sso_provider: + messages.error(request, "SSO not configured yet, please contact the " + "administrator or come back later.") + return redirect(reverse("orga:login")) oauth2_session = OAuth2Session( - settings.OAUTH2_PROVIDER["CLIENT_ID"], + sso_provider.client_id, redirect_uri=settings.OAUTH2_PROVIDER["REDIRECT_URI"], state=request.session.get("oauth2_state"), ) @@ -49,7 +65,7 @@ def oauth2_callback(request): # Fetch the token using the authorization code oauth2_session.fetch_token( settings.OAUTH2_PROVIDER["ACCESS_TOKEN_URL"], - client_secret=settings.OAUTH2_PROVIDER["CLIENT_SECRET"], + client_secret=sso_provider.secret, authorization_response=request.build_absolute_uri(), scope=settings.OAUTH2_PROVIDER["SCOPE"], ) diff --git a/src/pretalx/eventyay_common/views/sso.py b/src/pretalx/eventyay_common/views/sso.py new file mode 100644 index 000000000..46f059fa6 --- /dev/null +++ b/src/pretalx/eventyay_common/views/sso.py @@ -0,0 +1,91 @@ +from allauth.socialaccount.models import SocialApp +from django.conf import settings +from django.contrib import messages +from django.contrib.sites.models import Site +from django.db import transaction +from django.http import HttpResponseRedirect +from django.shortcuts import redirect +from django.urls import reverse +from django.utils.translation import gettext_lazy as _ +from django.views.generic import DetailView + +from pretalx.common.text.phrases import phrases +from pretalx.common.views import CreateOrUpdateView +from pretalx.common.views.mixins import ActionConfirmMixin, PermissionRequired +from pretalx.orga.forms.sso_client_form import SSOClientForm + + +class SSOConfigureView(PermissionRequired, CreateOrUpdateView): + template_name = "eventyay_common/sso/detail.html" + permission_required = "person.is_administrator" + form_class = SSOClientForm + model = SocialApp + + def get_object(self): + """ + Get the SocialApp instance for the 'eventyay' provider if it exists. + If not, return None to create a new instance. + Note: "eventyay" is the provider name for the Eventyay Ticket Provider. + """ + return SocialApp.objects.filter(provider=settings.EVENTYAY_SSO_PROVIDER).first() + + def get_success_url(self): + messages.success(self.request, phrases.base.saved) + return self.request.path + + def form_valid(self, form): + """ + Handle the form submission and save the instance. + """ + instance = form.save(commit=False) + instance.provider = settings.EVENTYAY_SSO_PROVIDER + instance.name = "Eventyay Ticket Provider" + with transaction.atomic(): + instance.save() + site = Site.objects.get(pk=settings.SITE_ID) + instance.sites.add(site) + return redirect(self.get_success_url()) + + def form_invalid(self, form): + """ + Handle invalid form submissions. + """ + messages.error( + self.request, + "There was an error updating the Eventyay Ticket " + "Provider configuration.", + ) + return self.render_to_response(self.get_context_data(form=form)) + + def get_context_data(self, **kwargs): + """ + Add additional context to the template if necessary. + """ + context = super().get_context_data(**kwargs) + context["sso_provider"] = self.get_object() + return context + + +class SSODeleteView(PermissionRequired, ActionConfirmMixin, DetailView): + permission_required = "person.is_administrator" + model = SocialApp + action_text = ( + _("You will not able to login with eventyay-tickets account.") + + " " + + phrases.base.delete_warning + ) + + def get_object(self, queryset=None): + return SocialApp.objects.filter(provider=settings.EVENTYAY_SSO_PROVIDER).first() + + def action_object_name(self): + return _("SSO Provider") + f": {self.get_object().name}" + + @property + def action_back_url(self): + return reverse("orga:admin.sso.settings") + + def post(self, *args, **kwargs): + sso_provider = self.get_object() + sso_provider.delete() + return HttpResponseRedirect(reverse("orga:admin.sso.settings")) diff --git a/src/pretalx/orga/forms/sso_client_form.py b/src/pretalx/orga/forms/sso_client_form.py index a47c6b742..b31ae879b 100644 --- a/src/pretalx/orga/forms/sso_client_form.py +++ b/src/pretalx/orga/forms/sso_client_form.py @@ -1,22 +1,12 @@ from allauth.socialaccount.models import SocialApp from django import forms -from django.conf import settings -from django.contrib.sites.models import Site class SSOClientForm(forms.ModelForm): - def __init__(self, provider_id, *args, **kwargs): - social_app = SocialApp.objects.filter(provider=provider_id).first() - kwargs["instance"] = social_app + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields["secret"].required = True # Secret is required + self.fields["secret"].required = True class Meta: model = SocialApp fields = ["client_id", "secret"] - - def save(self, organiser=None): - self.instance.name = organiser - self.instance.provider = organiser - super().save() - self.instance.sites.add(Site.objects.get(pk=settings.SITE_ID)) diff --git a/src/pretalx/orga/templates/orga/base.html b/src/pretalx/orga/templates/orga/base.html index 5e825a4d6..f2fb95445 100644 --- a/src/pretalx/orga/templates/orga/base.html +++ b/src/pretalx/orga/templates/orga/base.html @@ -474,6 +474,9 @@ {% translate "Users" %} + + {% translate "SSO settings" %} + {% endif %} diff --git a/src/pretalx/orga/templates/orga/organiser/detail.html b/src/pretalx/orga/templates/orga/organiser/detail.html index c479499dd..c27f21cf9 100644 --- a/src/pretalx/orga/templates/orga/organiser/detail.html +++ b/src/pretalx/orga/templates/orga/organiser/detail.html @@ -1,17 +1,12 @@ {% extends "orga/base.html" %} - {% load i18n %} {% load rules %} - {% block extra_title %}{% translate "Organiser" %} :: {{ request.organiser.name }} :: {% endblock extra_title %} - {% block content %}

{% translate "Settings" %}

{% csrf_token %} - {{ form }} -
{% has_perm "person.is_administrator" request.user request.organiser as can_delete_event %} @@ -28,11 +23,5 @@

{% translate "Settings" %}
- - {% if request.organiser %} -
- {% include "orga/organiser/organiser_sso.html" %} -
- {% endif %}
{% endblock content %} diff --git a/src/pretalx/orga/templates/orga/organiser/organiser_sso.html b/src/pretalx/orga/templates/orga/organiser/organiser_sso.html deleted file mode 100644 index d31291f2c..000000000 --- a/src/pretalx/orga/templates/orga/organiser/organiser_sso.html +++ /dev/null @@ -1,26 +0,0 @@ -{% load i18n %} -{% load rules %} - -{% block content %} -
-
- {% translate "SSO with Eventyay-ticket" %} - {% csrf_token %} - {{ sso_client_form.client_id.as_field_group }} - {{ sso_client_form.secret.as_field_group }} -
- - - - - -
-
-
-{% endblock %} diff --git a/src/pretalx/orga/urls.py b/src/pretalx/orga/urls.py index 28ab048a5..ab4f14d02 100644 --- a/src/pretalx/orga/urls.py +++ b/src/pretalx/orga/urls.py @@ -1,6 +1,7 @@ from django.urls import include, path from django.views.generic.base import RedirectView +from pretalx.eventyay_common.views import sso from pretalx.orga.views import ( admin, auth, @@ -39,6 +40,19 @@ name="admin.user.delete", ), path("admin/users/", admin.AdminUserList.as_view(), name="admin.user.list"), + path( + "admin/sso/", + include( + [ + path( + "settings", + sso.SSOConfigureView.as_view(), + name="admin.sso.settings", + ), + path("delete", sso.SSODeleteView.as_view(), name="admin.sso.delete"), + ] + ), + ), path("me", event.UserSettings.as_view(), name="user.view"), path("me/subuser", person.SubuserView.as_view(), name="user.subuser"), path( diff --git a/src/pretalx/orga/views/organiser.py b/src/pretalx/orga/views/organiser.py index 6e8f15e87..408ee9115 100644 --- a/src/pretalx/orga/views/organiser.py +++ b/src/pretalx/orga/views/organiser.py @@ -1,6 +1,5 @@ import logging -from allauth.socialaccount.models import SocialApp from django.contrib import messages from django.db import transaction from django.db.models import Count, Q @@ -16,7 +15,7 @@ from pretalx.common.exceptions import SendMailException from pretalx.common.text.phrases import phrases -from pretalx.common.views import CreateOrUpdateView, is_form_bound +from pretalx.common.views import CreateOrUpdateView from pretalx.common.views.mixins import ( ActionConfirmMixin, Filterable, @@ -32,7 +31,6 @@ TeamInvite, check_access_permissions, ) -from pretalx.orga.forms.sso_client_form import SSOClientForm from pretalx.person.forms import UserSpeakerFilterForm from pretalx.person.models import User from pretalx.submission.models.submission import SubmissionStates @@ -319,68 +317,6 @@ def get_success_url(self): messages.success(self.request, phrases.base.saved) return self.request.path - @context - @cached_property - def sso_client_form(self): - organiser = self.kwargs.get("organiser", None) - if self.request.POST.get("form") == "remove_sso_client": - bind = is_form_bound(self.request, "remove_sso_client") - else: - bind = is_form_bound(self.request, "sso_client") - return SSOClientForm( - provider_id=organiser, - data=self.request.POST if bind else None, - ) - - def save_sso_client(self, request, *args, **kwargs): - try: - self.sso_client_form.save(organiser=self.kwargs.get("organiser", None)) - except Exception as e: - logger.error( - f"Error saving SSO client for organiser {self.kwargs.get('organiser', None)}: {e}" - ) - messages.error(request, _("An error occurred: ") + str(e)) - return redirect(self.request.path) - return redirect(self.get_success_url()) - - def post(self, request, *args, **kwargs): - try: - if self.is_remove_sso_client_request(request): - return self.handle_remove_sso_client(request) - elif self.is_sso_client_request(request): - return self.handle_sso_client(request, *args, **kwargs) - else: - self.set_object() - return super().post(request, *args, **kwargs) - except Exception as e: - messages.error(request, _("An error occurred: ") + str(e)) - return redirect(self.request.path) - - def is_remove_sso_client_request(self, request): - return ( - is_form_bound(self.request, "remove_sso_client") - and request.POST.get("form") == "remove_sso_client" - ) - - def handle_remove_sso_client(self, request): - provider_id = self.kwargs.get("organiser") - try: - social_app = SocialApp.objects.get(provider=provider_id) - social_app.delete() - except SocialApp.DoesNotExist: - messages.error(request, _("The key does not exist.")) - return redirect(self.request.path) - - def is_sso_client_request(self, request): - return ( - is_form_bound(self.request, "sso_client") - and request.POST.get("form") == "sso_client" - and self.sso_client_form.is_valid() - ) - - def handle_sso_client(self, request, *args, **kwargs): - return self.save_sso_client(request, *args, **kwargs) - class OrganiserDelete(PermissionRequired, ActionConfirmMixin, DetailView): permission_required = "person.is_administrator" diff --git a/src/pretalx/settings.py b/src/pretalx/settings.py index 22bb381a4..851ec1c6e 100644 --- a/src/pretalx/settings.py +++ b/src/pretalx/settings.py @@ -702,11 +702,7 @@ def merge_csp(*options, config=None): ACCOUNT_DEFAULT_HTTP_PROTOCOL = "https" # OAuth2 Client settings -SSO_CLIENT_ID = config.get("sso", "client_id", fallback="") -SSO_CLIENT_SECRET = config.get("sso", "client_secret", fallback="") OAUTH2_PROVIDER = { - "CLIENT_ID": SSO_CLIENT_ID, - "CLIENT_SECRET": SSO_CLIENT_SECRET, "AUTHORIZE_URL": "/".join([EVENTYAY_TICKET_BASE_PATH, "control/oauth2/authorize/"]), "ACCESS_TOKEN_URL": "/".join([EVENTYAY_TICKET_BASE_PATH, "control/oauth2/token/"]), "REDIRECT_URI": "/".join( @@ -736,3 +732,4 @@ def merge_csp(*options, config=None): LOGIN_URL = BASE_PATH + "/login/" CORS_ORIGIN_WHITELIST = [EVENTYAY_TICKET_BASE_PATH] +EVENTYAY_SSO_PROVIDER = "eventyay"