Story 2584: Webpage Integration Account Connection Card #2635
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,13 +2,17 @@ | |
| from textwrap import dedent | ||
|
|
||
| from allauth.account import app_settings | ||
| from allauth.socialaccount.adapter import get_adapter, DefaultSocialAccountAdapter | ||
| from allauth.socialaccount.forms import DisconnectForm | ||
| from allauth.socialaccount.models import SocialApp | ||
| from django.contrib import messages | ||
| from django.contrib.auth.mixins import LoginRequiredMixin | ||
| from django.contrib import auth | ||
| from django.contrib.messages.views import SuccessMessageMixin | ||
| from django.http import HttpResponseRedirect, JsonResponse | ||
| from django.utils.http import url_has_allowed_host_and_scheme | ||
| from django.urls import reverse, reverse_lazy | ||
| from django.views.generic import DetailView, FormView, View | ||
| from django.views.generic.base import TemplateView | ||
| from django.utils import timezone | ||
| from django.conf import settings | ||
|
|
@@ -172,6 +176,42 @@ def get_v3_edit_context(self, form=None): | |
| ) | ||
| saved_section = self.request.GET.get("saved") | ||
| user = self.request.user | ||
| is_gh_conn: bool = user.is_github_connected | ||
| is_go_conn: bool = user.is_google_connected | ||
|
|
||
| def _get_connection_context_data(platform: str, connected: bool) -> dict | None: | ||
| adapter: DefaultSocialAccountAdapter = get_adapter(self.request) | ||
| try: | ||
| provider = adapter.get_provider(self.request, platform) | ||
| except SocialApp.DoesNotExist: | ||
| return None | ||
|
Comment on lines
+179
to
+187
Collaborator
There was a problem hiding this comment. I think this is causing 4 queries where we could be making only 1, by fetching all the connected providers at once and compiling this payload.
Collaborator
There was a problem hiding this comment. Maybe adapting |
||
| if platform not in ["github", "google"]: | ||
| return None | ||
| label = "" | ||
| if platform == "github": | ||
| label = "GitHub" | ||
| else: | ||
| label = platform.capitalize() | ||
| return { | ||
| "platform": platform, | ||
| "label": label, | ||
| "status_text": "Connected" if connected else "Not Connected", | ||
| "action_label": "Manage" if connected else "Connect", | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| "connected": connected, | ||
| # If not connected, we provide the login url for the chosen platform | ||
| # else, this points to the name of the disconnect modal associated with this platform | ||
| "action_url": ( | ||
|
julhoang marked this conversation as resolved.
|
||
| provider.get_login_url( | ||
| self.request, | ||
| **{auth.REDIRECT_FIELD_NAME: self.request.get_full_path()}, | ||
| process="connect", | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| if not connected | ||
| else f"#disconnect-{platform}" | ||
| ), | ||
| "disconnect_text": f"This will remove the link between your Boost account and {label}. You can reconnect at any time.", | ||
| } | ||
|
|
||
| ctx = { | ||
| "user_profile_form": form, | ||
| "SLACK_PROFILE_URL_PREFIX": SLACK_PROFILE_URL_PREFIX, | ||
|
|
@@ -188,24 +228,7 @@ def get_v3_edit_context(self, form=None): | |
| {"tier": "4", "name": "Platinum"}, | ||
| {"tier": "5", "name": "Diamond"}, | ||
| ], | ||
| "account_connections": [], | ||
| } | ||
| # Delete-account card: the modal schedules deletion, and once | ||
| # scheduled the card swaps to a single "Cancel deletion" control. | ||
|
|
@@ -223,6 +246,13 @@ def get_v3_edit_context(self, form=None): | |
| ) | ||
| ctx["profile_delete_url"] = reverse("profile-delete") | ||
| ctx["postorius_url"] = settings.POSTORIUS_URL | ||
|
|
||
| gh_conn_data = _get_connection_context_data("github", is_gh_conn) | ||
| go_conn_data = _get_connection_context_data("google", is_go_conn) | ||
| if gh_conn_data: | ||
| ctx["account_connections"].append(gh_conn_data) | ||
| if go_conn_data: | ||
| ctx["account_connections"].append(go_conn_data) | ||
| return ctx | ||
|
|
||
| def get_v3_context_data(self, **kwargs): | ||
|
|
@@ -535,7 +565,10 @@ def get_context_data(self, **kwargs): | |
| def get_social_accounts(self): | ||
| account_data = [] | ||
| for account in SocialAccount.objects.filter(user=self.request.user): | ||
| try: | ||
| provider_account = account.get_provider_account() | ||
| except SocialApp.DoesNotExist: | ||
| continue | ||
| account_data.append( | ||
| { | ||
| "id": account.pk, | ||
|
|
@@ -1083,3 +1116,43 @@ def form_valid(self, form): | |
| user.delete_account(extended_scrub=flag_is_active(self.request, "v3")) | ||
| auth.logout(self.request) | ||
| return super().form_valid(form) | ||
|
|
||
|
|
||
| class DisconnectSocialAccountView(LoginRequiredMixin, View): | ||
| def post(self, *args, **kwargs): | ||
| redirect_url = self.request.GET.get("redirect_url", "").strip("'") | ||
|
Collaborator
There was a problem hiding this comment. can we remove the redirect_url = self.request.GET.get("redirect_url", "").strip("'") or reverse("home")(a slight change will be needed here if you start building the redirect url on the backend, from my other comment) |
||
| if not url_has_allowed_host_and_scheme(redirect_url, allowed_hosts=None): | ||
| messages.error( | ||
| self.request, "An internal error has occurred. Please contact an admin." | ||
| ) | ||
| return HttpResponseRedirect(reverse("home")) | ||
|
|
||
| platform = kwargs.get("platform") | ||
| if not platform: | ||
| messages.error(self.request, "Platform must be specified.") | ||
| return HttpResponseRedirect(redirect_url) | ||
|
|
||
| if not redirect_url: | ||
| redirect_url = reverse("home") | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| user = self.request.user | ||
| try: | ||
| sa = SocialAccount.objects.get(user=user, provider=platform) | ||
| except SocialAccount.DoesNotExist: | ||
| messages.error( | ||
| self.request, | ||
| "No social account between this user and platform exists on Boost.", | ||
| ) | ||
| return HttpResponseRedirect(redirect_url) | ||
|
|
||
| form = DisconnectForm(request=self.request, data={"account": sa.pk}) | ||
| if form.is_valid(): | ||
| form.save() | ||
|
Comment on lines
+1139
to
+1150
There was a problem hiding this comment. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Verify the pinned django-allauth version and project-level SocialAccount constraints.
fd -HI -t f . | rg '(^|/)(requirements[^/]*\.txt|pyproject\.toml|poetry\.lock|uv\.lock|Pipfile(\.lock)?|setup\.py)$' \
| xargs -r rg -n -i 'django[-_]allauth|allauth'
rg -n -C 4 'SocialAccount\.objects\.(get|filter)|UniqueConstraint|unique_together' \
users configRepository: boostorg/website-v2 Length of output: 2116 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1080,1160p' users/views.py
sed -n '520,590p' users/views.py
rg -n -C 8 'get_social_accounts|disconnect|DisconnectForm|account.*pk|platform' users templatesRepository: boostorg/website-v2 Length of output: 30298 🌐 Web query:
💡 Result: In django-allauth, the SocialAccount model enforces a unique constraint on the combination of provider and uid [1][2]. This means that within a single database, a specific provider (e.g., "google") and a specific user ID (e.g., "123456") can only be associated with one local user account [3][1]. Regarding the possibility of connecting multiple accounts from the same provider to the same user: 1. Relationship: The SocialAccount model has a foreign key to the User model, not the other way around [1][2]. This design naturally supports a single local user account having multiple associated social accounts, including multiple accounts from the same provider (e.g., a user could theoretically link both 'user_a@gmail.com' and 'user_b@gmail.com' to their single local account) [4][5]. 2. Unique Constraint: The unique constraint is strictly on (provider, uid) [1]. It ensures that two different local users cannot claim the same social account ID for the same provider [3]. It does not prevent a single user from owning multiple distinct social accounts from that same provider, provided those accounts have different uids [4][6]. 3. Implementation details: While the model allows this, the actual user flow for connecting multiple accounts from the same provider can sometimes be complicated by the provider's own authentication state (e.g., being automatically logged into the first Google account when attempting to link a second one) [7]. Users often need to sign out of the external provider or use specific authentication parameters (like forcing an "authorize" action) to connect a second account [7]. In summary, the SocialAccount unique constraint (provider, uid) is designed to ensure one-to-one mapping between a social provider's identity and a local user, while allowing a single local user to maintain one-to-many relationships with social accounts across or within providers [3][4]. Citations:
Select one social account explicitly.
🤖 Prompt for AI Agents |
||
| else: | ||
| messages.error( | ||
| self.request, | ||
| " ".join(form.non_field_errors()) | ||
| or "An error has occurred while removing your connection. Please try again shortly.", | ||
| ) | ||
|
Comment on lines
+1139
to
+1156
Collaborator
There was a problem hiding this comment. Currently it seems like we're not handling these exceptions, which will cause the page to break with 500 errors. Should we consider raising error banner
Collaborator
Author
There was a problem hiding this comment. Just curious, why can we delete the messages section?
Collaborator
There was a problem hiding this comment. Hey @jlchilders11 I think this still needs to be addressed. {% block messages %}
<div id="messages" class="w-full text-center transition-opacity" x-data="{show: true}">
{% for message in messages %}{% endfor %}
</div>
{% endblock messages %}which swallows error messages and they don't get rendered in the UI. |
||
|
|
||
| return HttpResponseRedirect(redirect_url) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we build this URL in the backend? Besides being better to build it there to keep the logic centralized, the view in
DisconnectSocialAccountView.post()is also having to perform an awkward parsing removing the single quotes'inredirect_url = self.request.GET.get("redirect_url", "").strip("'")