fix(alerts): route legacy members to help page instead of Neon upgrade - #7509
Conversation
This comment has been minimized.
This comment has been minimized.
| <a href="https://donate.free.law/constituent/memberships/upgrade/{{ alerts_context.neon_id }}" | ||
| id="member-button" class="hidden btn btn-danger" | ||
| target="_blank">Upgrade Membership</a> | ||
| <a href="{{ alerts_context.legacyHelpUrl }}" |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Detected a template variable used in an anchor tag with the 'href' attribute. This allows a malicious actor to input the 'javascript:' URI and is subject to cross- site scripting (XSS) attacks. If using Flask, use 'url_for()' to safely generate a URL. If using Django, use the 'url' filter to safely generate a URL. If using Mustache, use a URL encoding library, or prepend a slash '/' to the variable for relative links (href="/{{link}}"). You may also consider setting the Content Security Policy (CSP) header.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by var-in-href.
You can view more details about this finding in the Semgrep AppSec Platform.
| </p> | ||
| <p id="msg-quota-member-legacy" class="hidden"> | ||
| You've used all of the alerts included with your legacy membership. | ||
| Legacy memberships can't be upgraded online, but <a href="{{ alerts_context.legacyHelpUrl }}" target="_blank">here's how to get more features</a>, or <a href="{% url 'profile_alerts' %}">disable a RECAP alert</a>. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| format_html( | ||
| "You've used all of the alerts included with your legacy membership. " | ||
| "Legacy memberships can't be upgraded online, but " | ||
| "<a href='{}' target='_blank'>here's how to get more features</a>, or " |
There was a problem hiding this comment.
🟡 The new legacy-quota ValidationError at cl/alerts/forms.py:90 adds an <a href='{}' target='_blank'> without rel='noopener noreferrer', while the rest of this PR consistently adds that attribute to every new/touched anchor in alert_modal.html (in response to the frontend-checks bot flagging 6 tabnabbing risks). For consistency with the direction of this PR, consider adding rel='noopener noreferrer' here as well. Nit: the URL is a hardcoded constant and modern browsers already imply noopener for target=_blank.
Extended reasoning...
What the bug is
The newly-introduced ValidationError for the legacy-membership quota path renders an anchor via format_html:
raise ValidationError(
format_html(
"You've used all of the alerts included with your legacy membership. "
"Legacy memberships can't be upgraded online, but "
"<a href='{}' target='_blank'>here's how to get more features</a>, or "
"<a href='{}'>disable a RECAP Alert</a>.",
LEGACY_MEMBERSHIP_HELP_URL,
reverse("profile_search_alerts"),
)
)The first anchor uses target='_blank' but omits rel='noopener noreferrer'.
Why this is worth flagging in this PR specifically
The same PR fixes exactly this issue in cl/search/templates/includes/alert_modal.html — the frontend-checks bot flagged 6 tabnabbing errors and the author added rel='noopener noreferrer' to every one, and to every new anchor introduced in that template. The Python-rendered anchor here is a new instance added by the same PR that missed the same convention, so it's inconsistent with the PR's own direction.
Step-by-step proof
- A user with
NeonMembershipLevel.LEGACYwho is already at their RECAP quota submits the alert form. CreateAlertForm.clean_ratereaches the newMEMBER_QUOTA_EXCEEDED→LEGACYbranch (cl/alerts/forms.py:82-95).format_htmlrenders<a href='https://wiki.free.law/...' target='_blank'>here's how to get more features</a>with norelattribute.- The error is displayed inline in the alert modal on the search results page.
- Because
target='_blank'is present withoutrel='noopener', an old-browser user who clicks the link opens the wiki page withwindow.openeraccessible from the new tab — the classic tabnabbing surface.
Why the practical impact is low (nit)
LEGACY_MEMBERSHIP_HELP_URLis a hardcoded first-partywiki.free.lawconstant, not user-controlled, so tabnabbing exploitation requires either that page being compromised or a redirect chain — a very small attack surface.- Chrome 88+, Firefox 79+, and Safari 12.1+ default
target='_blank'to implyrel='noopener', so nearly all users are protected regardless of the attribute. - The pre-existing
upgrade your membershipanchor a few lines below (cl/alerts/forms.py:102) has the same omission and is out of scope of this PR — this comment does not ask for that pre-existing case to be fixed.
How to fix
Add rel='noopener noreferrer' to the new anchor:
"<a href='{}' target='_blank' rel='noopener noreferrer'>here's how to get more features</a>, or "This matches the convention already applied to every new/touched anchor in alert_modal.html in this PR.
albertisfu
left a comment
There was a problem hiding this comment.
This is working properly, @ERosendo.
It looks like the PR needs to be updated and the merge conflict needs to be resolved.
Is LEGACY_MEMBERSHIP_HELP_URL the final URL?
@albertisfu It's the final URL, but I believe Jenifer is going to make a few tweaks to the wiki page. |
Legacy members hit a 404 when they exceed their alert quota because the "Upgrade your membership" link points to Neon's upgrade flow, which doesn't support legacy accounts. This commit adds logic to identify LEGACY members and send these users to a help page explaining how to access additional features.
062bfe5 to
7b57f35
Compare
for more information, see https://pre-commit.ci
Fixes
This PR fixes #7136
Summary
Legacy members currently hit a 404 when they exceed their alert quota and try to get more features. The existing flows direct all active members to Neon's membership upgrade page, but Neon does not support upgrades for legacy memberships.
This PR adds logic to indentify users with the
NeonMembershipLevel.LEGACYmembership level and directs them to a help page explaining how to access additional features instead of sending them to the Neon upgrade flow. All other membership levels continue to use the standard upgrade URL.The fix updates all surfaces that generate membership-upgrade links:
cl/alerts/forms.py): displays a legacy-specific validation message and help link.cl/search/views.py,alert_modal.html,search-alerts.js): addsisLegacyandlegacyHelpUrlto alerts_context, allowing the UI to display a "Get More Features" action for legacy members instead of "Upgrade Membership".cl/alerts/api_serializers.py): returns a legacy-specific PermissionDenied message.A new LEGACY_MEMBERSHIP_HELP_URL constant centralizes the destination URL.
Important
LEGACY_MEMBERSHIP_HELP_URLcurrently points to a placeholder URL (https://free.law/TODO-legacy-membership-help/). The final help-page URL must be added before this PR is merged.Deployment
This PR should:
skip-deploy(skips everything below)skip-web-deployskip-celery-deployskip-cronjob-deployskip-daemon-deploy