address more reivew comments

ephios/plugins/federation/templates/federation/event_detail.html

@@ -5,6 +5,6 @@
 {% block messages %}
     <div class="alert alert-info" role="alert">
         {% blocktranslate trimmed %}You are currently viewing an event at {{ organization_name }}.{% endblocktranslate %}
-        <a href="{{ guest.url }}">{% blocktranslate with name=guest.name trimmed %}Back to {{ name }}{% endblocktranslate %}</a>
+        <a href="{{ federation_guest.url }}">{% blocktranslate with name=federation_guest.name trimmed %}Back to {{ name }}{% endblocktranslate %}</a>
     </div>
 {% endblock %}

ephios/plugins/federation/templates/federation/external_event_list.html

@@ -18,10 +18,15 @@ <h1>{% translate "External events" %}</h1>
                href="{{ event.signup_url }}?referrer={{ SITE_URL }}">
                 <div class="grid-wrapper m-0 py-0 ps-2 pe-3">
                     <div class="grid-title">
-                        <h5 class="mb-0 text-break">
-                            {{ event.title }}
-
-                        </h5>
+                        <div>
+                            <h5 class="mb-0 text-break d-inline-block">
+                                {{ event.title }}
+                            </h5>
+                            <small class="text-muted ms-1">
+                                <span class="fas fa-share"></span>
+                                <span class="">{{ event.host }}</span>
+                            </small>
+                        </div>
                         <span class="w-100 text-muted text-break">
                         {{ event.location }}
                     </span>
@@ -30,7 +35,6 @@ <h5 class="mb-0 text-break">
                             class="badge bg-primary">{{ event.type }}</span>
                     </div>
                     <div class="grid-signup">
-                        <span class="fas fa-share"></span><span class="text-nowrap">{{ event.host }}</span>
                     </div>
                     <div class="grid-time">
                         {{ event.start_time|date:"D" }},

ephios/plugins/federation/templates/federation/federation_settings.html

@@ -16,7 +16,7 @@ <h3>{% translate "ephios instances that you share events with" %}</h3>
         </tr>
         </thead>
         <tbody>
-        {% for guest in guests %}
+        {% for guest in federation_guests %}
             <tr>
                 <td>{{ guest.name }}</td>
                 <td>{{ guest.url }}</td>
@@ -26,7 +26,7 @@ <h3>{% translate "ephios instances that you share events with" %}</h3>
         </tbody>
     </table>
 
-    {% if invites %}
+    {% if federation_invites %}
         <h4>{% translate "Pending invites" %}</h4>
         <table class="table table-striped display mt-2">
         <thead>
@@ -36,7 +36,7 @@ <h4>{% translate "Pending invites" %}</h4>
         </tr>
         </thead>
         <tbody>
-        {% for invite in invites %}
+        {% for invite in federation_invites %}
             <tr>
                 <td>{{ invite.url }}</td>
                 <td><a class="btn btn-secondary" href="{% url "federation:reveal_invite_code" invite.pk %}">{% translate "Show invite code" %}</a></td>
@@ -57,7 +57,7 @@ <h3>{% translate "ephios instances that share events with you" %}</h3>
         </tr>
         </thead>
         <tbody>
-        {% for host in hosts %}
+        {% for host in federation_hosts %}
             <tr>
                 <td>{{ host.name }}</td>
                 <td>{{ host.url }}</td>

ephios/plugins/federation/views/api.py

@@ -2,6 +2,7 @@
 
 import requests
 from django.conf import settings
+from django.core.exceptions import MultipleObjectsReturned
 from django.shortcuts import redirect
 from django.urls import reverse
 from django.views import View
@@ -21,13 +22,21 @@
 
 
 class RedeemInviteCodeView(CreateAPIView):
+    """
+    API view that accepts an InviteCode and creates a FederatedGuest (to start sharing events with that instance).
+    """
+
     serializer_class = FederatedGuestCreateSerializer
     queryset = FederatedGuest.objects.all()
     authentication_classes = []
     permission_classes = [AllowAny]
 
 
 class SharedEventListView(ListAPIView):
+    """
+    API view that lists all events that are shared with the instance corresponding to the access token.
+    """
+
     serializer_class = SharedEventSerializer
     permission_classes = [TokenHasScope]
     required_scopes = []
@@ -42,20 +51,26 @@ def get_queryset(self):
 
 
 class FederationOAuthView(View):
+    """
+    View that handles the OAuth2 flow for federated users from another instance.
+    """
+
     def get(self, request, *args, **kwargs):
         try:
             self.guest = (
-                FederatedGuest.objects.get(pk=self.request.session["guest"])
-                if "guest" in self.request.session.keys()
+                FederatedGuest.objects.get(pk=self.request.session["federation_guest_pk"])
+                if "federation_guest_pk" in self.request.session.keys()
                 else FederatedGuest.objects.get(url=self.request.GET["referrer"])
             )
-        except (KeyError, FederatedGuest.DoesNotExist) as exc:
+        except (KeyError, FederatedGuest.DoesNotExist, MultipleObjectsReturned) as exc:
             raise PermissionDenied from exc
         if "error" in request.GET.keys():
             return redirect(urljoin(self.guest.url, reverse("federation:external_event_list")))
         elif "code" in request.GET.keys():
             self._oauth_callback()
-            return redirect("federation:event_detail", pk=self.request.session.pop("event"))
+            return redirect(
+                "federation:event_detail", pk=self.request.session.pop("federation_event")
+            )
         else:
             return redirect(self._get_authorization_url())
 
@@ -68,8 +83,8 @@ def _get_authorization_url(self):
         )
         verifier = oauth_client.create_code_verifier(64)
         self.request.session["code_verifier"] = verifier
-        self.request.session["event"] = self.kwargs["pk"]
-        self.request.session["guest"] = self.guest.pk
+        self.request.session["federation_event"] = self.kwargs["pk"]
+        self.request.session["federation_guest_pk"] = self.guest.pk
         challenge = oauth_client.create_code_challenge(verifier, "S256")
         authorization_url, _ = oauth.authorization_url(
             urljoin(self.guest.url, "api/oauth/authorize/"),
@@ -89,7 +104,7 @@ def _oauth_callback(self):
             client_secret=self.guest.client_secret,
             code_verifier=self.request.session["code_verifier"],
         )
-        self.request.session["access_token"] = token["access_token"]
+        self.request.session["federation_access_token"] = token["access_token"]
         self.request.session.set_expiry(token["expires_in"])
         user_data = requests.get(
             urljoin(self.guest.url, "api/users/me/"),
@@ -101,27 +116,31 @@ def _oauth_callback(self):
                 federated_instance=self.guest, email=user_data.json()["email"]
             )
         except FederatedUser.DoesNotExist:
-            user = FederatedUser.objects.create(
-                federated_instance=self.guest,
-                email=user_data.json()["email"],
-                first_name=user_data.json()["first_name"],
-                last_name=user_data.json()["last_name"],
-                date_of_birth=user_data.json()["date_of_birth"],
-            )
-            for qualification in user_data.json()["qualifications"]:
-                try:
-                    # Note that we assign the qualification on the host instance without further checks.
-                    # This may lead to incorrect qualifications if the inclusions for the qualification
-                    # are defined differently on the guest instance. We are accepting this as it should
-                    # not happen with the pre-defined qualifications as we are displaying a warning if
-                    # the user adapt these and custom qualifications will have different uuids anyway.
-                    user.qualifications.add(Qualification.objects.get(uuid=qualification["uuid"]))
-                except Qualification.DoesNotExist:
-                    for included_qualification in qualification["includes"]:
-                        try:
-                            user.qualifications.add(
-                                Qualification.objects.get(uuid=included_qualification["uuid"])
-                            )
-                        except Qualification.DoesNotExist:
-                            continue
-        self.request.session["federated_user"] = user.pk
+            user = self._create_user(user_data)
+        self.request.session["federation_user"] = user.pk
+
+    def _create_user(self, user_data):
+        user = FederatedUser.objects.create(
+            federated_instance=self.guest,
+            email=user_data.json()["email"],
+            first_name=user_data.json()["first_name"],
+            last_name=user_data.json()["last_name"],
+            date_of_birth=user_data.json()["date_of_birth"],
+        )
+        for qualification in user_data.json()["qualifications"]:
+            try:
+                # Note that we assign the qualification on the host instance without further checks.
+                # This may lead to incorrect qualifications if the inclusions for the qualification
+                # are defined differently on the guest instance. We are accepting this as it should
+                # not happen with the pre-defined qualifications as we are displaying a warning if
+                # the user adapt these and custom qualifications will have different uuids anyway.
+                user.qualifications.add(Qualification.objects.get(uuid=qualification["uuid"]))
+            except Qualification.DoesNotExist:
+                for included_qualification in qualification["includes"]:
+                    try:
+                        user.qualifications.add(
+                            Qualification.objects.get(uuid=included_qualification["uuid"])
+                        )
+                    except Qualification.DoesNotExist:
+                        continue
+        return user

ephios/plugins/federation/views/frontend.py

@@ -27,6 +27,10 @@
 
 
 class ExternalEventListView(LoginRequiredMixin, TemplateView):
+    """
+    View that lists all events that are shared with this instance.
+    """
+
     template_name = "federation/external_event_list.html"
 
     def get_context_data(self, **kwargs):
@@ -56,55 +60,74 @@ def get_context_data(self, **kwargs):
 
 class CheckFederatedAccessTokenMixin:
     def dispatch(self, request, *args, **kwargs):
-        if "access_token" not in request.session.keys():
+        if "federation_access_token" not in request.session.keys():
             return FederationOAuthView.as_view()(request, *args, **kwargs)
         return super().dispatch(request, *args, **kwargs)
 
     def get_context_data(self, object):
         context = super().get_context_data()
-        context["guest"] = FederatedGuest.objects.get(pk=self.request.session["guest"])
+        context["federation_guest"] = FederatedGuest.objects.get(
+            pk=self.request.session["federation_guest_pk"]
+        )
         return context
 
 
 class FederatedEventDetailView(CheckFederatedAccessTokenMixin, DetailView):
+    """
+    View that displays a shared event to a federated user from another instance
+    """
+
     model = Event
     template_name = "federation/event_detail.html"
 
     def get_object(self, queryset=None):
         obj = super().get_object(queryset)
         try:
-            guest = self.request.session["guest"]
+            guest = self.request.session["federation_guest_pk"]
             FederatedEventShare.objects.get(
                 event=obj,
                 shared_with__in=[FederatedGuest.objects.get(pk=guest)],
             )
         except (KeyError, FederatedEventShare.DoesNotExist) as exc:
+            self.request.session.flush()
             raise PermissionDenied from exc
         return obj
 
 
 class FederatedUserShiftActionView(CheckFederatedAccessTokenMixin, BaseShiftActionView):
+    """
+    View that allows a federated user from another instanceto sign up for a shift
+    """
+
     def get_participant(self):
         try:
             return FederatedUser.objects.get(
-                pk=self.request.session["federated_user"]
+                pk=self.request.session["federation_user"]
             ).as_participant()
         except FederatedUser.DoesNotExist as e:
             raise PermissionDenied from e
 
 
 class FederationSettingsView(StaffRequiredMixin, TemplateView):
+    """
+    View that displays the federation settings page where new instances can be connected
+    """
+
     template_name = "federation/federation_settings.html"
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
-        context["guests"] = FederatedGuest.objects.all()
-        context["hosts"] = FederatedHost.objects.all()
-        context["invites"] = InviteCode.objects.all()
+        context["federation_guests"] = FederatedGuest.objects.all()
+        context["federation_hosts"] = FederatedHost.objects.all()
+        context["federation_invites"] = InviteCode.objects.all()
         return context
 
 
 class CreateInviteCodeView(StaffRequiredMixin, CreateView):
+    """
+    View that allows staff users to create new invite codes (to share events with another instance)
+    """
+
     model = InviteCode
     form_class = InviteCodeForm
     success_url = reverse_lazy("federation:settings")
@@ -114,6 +137,10 @@ def get_success_url(self):
 
 
 class InviteCodeRevealView(StaffRequiredMixin, TemplateView):
+    """
+    View that displays an invite code to a staff user
+    """
+
     template_name = "federation/invitecode_reveal.html"
 
     def get(self, request, *args, **kwargs):
@@ -126,6 +153,10 @@ def get(self, request, *args, **kwargs):
 
 
 class RedeemInviteCodeView(StaffRequiredMixin, FormView):
+    """
+    View that allows staff users to redeem an invite code (to receive events from another instance)
+    """
+
     form_class = RedeemInviteCodeForm
     template_name = "federation/redeem_invite_code.html"
 
@@ -146,6 +177,10 @@ def form_valid(self, form):
 
 
 class FederatedGuestDeleteView(StaffRequiredMixin, SuccessMessageMixin, DeleteView):
+    """
+    View that allows staff users to remove a guest instance (to stop sharing events with another instance)
+    """
+
     model = FederatedGuest
     success_url = reverse_lazy("federation:settings")
     success_message = _("You are no longer sharing events with this instance.")
@@ -158,6 +193,10 @@ def form_valid(self, form):
 
 
 class FederatedHostDeleteView(StaffRequiredMixin, SuccessMessageMixin, DeleteView):
+    """
+    View that allows staff users to remove a host instance (to stop receiving events from another instance)
+    """
+
     model = FederatedHost
     success_url = reverse_lazy("federation:settings")
     success_message = _("You are no longer receiving events from this instance.")