Make dashboards, leaderboards, and profiles public#831
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 20 minutes and 32 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR makes multiple backend API endpoints and frontend routes publicly accessible without authentication. Backend changes relax permission classes on leaderboard, contribution types, steward submissions, and user profile endpoints, while adding visibility filters and email-field protection for unauthenticated callers. Frontend route guards are removed for listing and leaderboard pages, and navigation components are updated to show auth-gated write actions only to authenticated users. ChangesPublic Access Rollout
Sequence Diagram(s)sequenceDiagram
participant Client as Unauthenticated Client
participant App as App.svelte
participant SectionHeader as SectionHeader
participant UserAPI as UserViewSet
participant StewardAPI as StewardSubmissionViewSet
Client->>App: Navigate to /leaderboard
App->>Client: Render public Leaderboard (no protectedRoute)
Client->>SectionHeader: Click "View all" with requiresAuth=true
SectionHeader->>SectionHeader: handleLinkClick() — not authenticated
SectionHeader->>Client: Set sessionStorage.redirectAfterLogin
SectionHeader->>Client: Trigger [data-auth-button] click
Client->>UserAPI: GET /api/v1/users/{address}/
UserAPI->>UserAPI: get_permissions() → AllowAny for retrieve
UserAPI->>UserAPI: _can_view_user() check visibility
UserAPI->>Client: 200 public profile (email fields absent)
Client->>StewardAPI: GET /api/v1/steward-submissions/stats/
StewardAPI->>StewardAPI: permission_classes=[AllowAny]
StewardAPI->>StewardAPI: User not steward → public aggregates
StewardAPI->>StewardAPI: Query SubmittedContribution(user__visible=True)
StewardAPI->>Client: 200 public stats with acceptance_rate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/contributions/views.py (1)
71-84: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFilter
submission_countto public-visible users before exposing this view.With Line 71 making the endpoint anonymous,
active_submissionsstill counts submissions from hidden users. Adduser__visible=Truebefore the aggregate so public contribution-type counts do not reveal hidden profile activity.Proposed fix
- active_submissions = SubmittedContribution.objects.exclude( + active_submissions = SubmittedContribution.objects.filter( + user__visible=True, + ).exclude( state__in=['rejected', 'canceled'] )As per path instructions, “Flag unauthenticated routes exposing user data.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/contributions/views.py` around lines 71 - 84, The get_queryset method in this view is accessible to unauthenticated users due to the AllowAny permission class, but the active_submissions queryset counts submissions from all users including those with hidden profiles. This leaks information about hidden user activity. Filter the active_submissions queryset by adding user__visible=True before the aggregate annotation to ensure the submission_count only reflects contributions from publicly visible users.Source: Path instructions
backend/leaderboard/views.py (1)
63-68: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the anonymous leaderboard list response.
The viewset is now public, but
limitis optional and unbounded, so/leaderboard/can still return every visible entry. Clampoffset/limitand apply a safe default page size for anonymous access.Proposed bounds
- offset = 0 - limit = None + offset = 0 + limit = 100 try: - offset = int(self.request.query_params.get('offset', 0)) + offset = max(int(self.request.query_params.get('offset', 0)), 0) except (ValueError, TypeError): offset = 0 limit_param = self.request.query_params.get('limit') if limit_param: try: - limit = int(limit_param) + limit = min(max(int(limit_param), 1), 100) except (ValueError, TypeError): - limit = None + limit = 100As per path instructions, “Flag missing input validation, unsafe filtering, dynamic ordering/search abuse.”
Also applies to: 151-174
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/leaderboard/views.py` around lines 63 - 68, The leaderboard viewset permits anonymous access with pagination disabled, allowing unrestricted retrieval of all entries. Replace the `pagination_class = None` setting with a proper pagination class that enforces a maximum page size for anonymous users. Implement explicit bounds on the offset and limit parameters to prevent abuse, and apply a safe default page size (e.g., 100 entries) for anonymous requests. Ensure the same pagination bounds are applied to the related viewset code mentioned at lines 151-174 to maintain consistent access controls across all public leaderboard endpoints.Source: Path instructions
backend/users/views.py (2)
136-159: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFinish public hardening for
user_highlights.The new
_can_view_usercheck only authorizes the profile. The response still serializes everyContributionHighlightfor that user, andlimitcan be non-numeric, negative, or huge. Before exposing this action publicly, apply the same public contribution/highlight visibility predicate used elsewhere and clamp or reject invalid limits.Proposed `limit` guard
- limit = int(request.query_params.get('limit', 5)) + try: + limit = int(request.query_params.get('limit', 5)) + except (TypeError, ValueError): + return Response( + {'error': 'Invalid limit'}, + status=status.HTTP_400_BAD_REQUEST + ) + limit = min(max(limit, 1), 20)As per path instructions, "Flag any handler/viewset/action that fetches a resource by id, slug, address, wallet, username, path param, or query param without proving object ownership, steward/admin role, or an explicit permission check on that exact resource" and "Flag missing input validation, unsafe filtering, dynamic ordering/search abuse."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/users/views.py` around lines 136 - 159, The user_highlights action has two security issues that need to be addressed before public exposure. First, the queryset filters by user but does not apply the public contribution/highlight visibility predicate that is used elsewhere in the codebase to control which highlights are visible based on their public/private status. Second, the limit parameter extracted from request.query_params has no input validation and can accept non-numeric, negative, or excessively large values. Add the appropriate visibility filter to the queryset based on the same predicate used elsewhere, and add validation to clamp the limit parameter to a reasonable range or reject invalid input before using it to slice the queryset.Source: Path instructions
892-909: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCap and stabilize anonymous search queries.
Making
searchpublic exposes an unbounded multi-fieldicontainsquery plus a related-table lookup. Add a max query length and return stable distinct rows to reduce scraping/load and duplicate users from the join.Proposed fix
query = request.query_params.get('q', '').strip() if len(query) < 2: return Response([]) + + max_query_length = 64 + if len(query) > max_query_length: + return Response( + {'error': 'Search query is too long'}, + status=status.HTTP_400_BAD_REQUEST + ) search_query = ( Q(name__icontains=query) | Q(address__icontains=query) | Q(twitter_handle__icontains=query) | @@ - users = User.objects.filter(search_query).filter(visible=True)[:10] + users = ( + User.objects + .filter(search_query, visible=True) + .distinct() + .order_by('name', 'id') + .only('id', 'name', 'address', 'profile_image_url')[:10] + )As per path instructions, "Flag missing input validation, unsafe filtering, dynamic ordering/search abuse."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/users/views.py` around lines 892 - 909, The search method lacks both input validation for maximum query length and deduplication of results. Add a maximum length check for the query parameter after the existing minimum length check (similar to how len(query) < 2 is validated), and add .distinct() to the User.objects.filter() queryset chain before the slice operation to eliminate duplicate rows that can result from the join with githubconnection__platform_username. This will cap unbounded search queries and ensure stable, non-duplicated results.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/api/metrics_views.py`:
- Around line 43-48: Update the docstring of the class containing the metrics
view to accurately reflect the new fallback behavior. The docstring currently
states that public reads never aggregate source data, but the code now calls
build_overview_payload() when latest_overview_payload() returns None, which
performs live aggregation. Modify the class docstring to clarify that when no
snapshot exists, the view performs a live rebuild of the overview payload via
build_overview_payload(), which does aggregate source data in real-time.
In `@backend/contributions/tests/test_steward_permissions.py`:
- Around line 121-125: The test method names are misleading because they suggest
"cannot access steward endpoints" but the actual test assertions validate
successful public access to the /stats/ endpoint with HTTP 200 OK. Rename the
test methods to accurately reflect the intent of testing mixed access behavior,
specifically that the steward statistics endpoint is publicly accessible. Apply
this rename to all affected test methods as indicated by the comment, updating
the names to clearly convey that they verify public access to certain endpoints
rather than blocking access.
In `@backend/contributions/views.py`:
- Around line 2407-2413: The daily_metrics action lacks input validation on its
date range parameters, allowing callers to request extremely large spans
(especially with group_by=day) that could cause memory exhaustion. Add
validation in the daily_metrics method to reject inverted date ranges (where
start_date is after end_date) and implement maximum allowed span limits based on
the group_by parameter value (such as stricter limits for day grouping, looser
limits for month grouping). This validation should occur before processing the
date range logic to prevent the allocation of large responses.
In `@backend/leaderboard/views.py`:
- Line 63: The permission_classes setting of AllowAny on line 63 exposes the
user_stats and user_stats_by_address actions publicly, but these actions do not
validate user visibility before returning stats data. In the user_stats and
user_stats_by_address action methods (around lines 618-650), add a visibility
check before returning the stats using the same visibility validation logic
applied to public profiles. This ensures that hidden users' statistics are not
exposed through the anonymous API endpoints.
In `@backend/users/views.py`:
- Around line 43-46: The get_permissions method allows public access to retrieve
and by_address actions, but the serializers being used for these actions still
include referral details which should only be exposed to authenticated users. To
fix this, modify the get_serializer_class method to return a different
serializer for public actions (retrieve, by_address) that excludes
referral-related fields, while keeping the full serializer with referral details
for authenticated-only endpoints like the owner/staff-only path. This ensures
referral information is only accessible through restricted endpoints and not
through the public profile endpoints.
In `@frontend/src/components/AuthButton.svelte`:
- Around line 197-202: The `.auth-button:not(.connected)` CSS rule for
unauthenticated button sizing violates the Tailwind-only styling guideline for
Svelte components. Remove this custom CSS rule from the style block and instead
apply the equivalent Tailwind utility classes directly to the button element's
class attribute when in the unauthenticated state. Replace the custom min-width,
height, padding, and font-size properties with their Tailwind equivalents such
as min-w-[168px], h-11, px-5, py-2.5, and text-[15px], or adjust to the nearest
Tailwind scale values as preferred.
In `@frontend/src/components/Sidebar.svelte`:
- Around line 220-222: The anchor element for the leaderboard link is
unnecessarily preventing default behavior and manually calling navigate.
Simplify this by removing the onclick handler that contains e.preventDefault()
and navigate('/leaderboard'), and keep the anchor as a plain in-app link with
just href="/leaderboard". This allows the global link interceptor to handle the
routing centrally. Apply the same fix to the other similar anchor instances
mentioned at lines 677-679.
In `@frontend/src/components/ui/SectionHeader.svelte`:
- Around line 15-23: The handleLinkClick() function is calling push(linkPath)
for all clicks, which violates the coding guidelines requiring plain anchor tags
for navigation. Refactor the rendered element to use a standard <a
href={linkPath}> anchor tag instead of calling push() in the handler. Modify the
handleLinkClick() function to only preventDefault() and trigger login when
requiresAuth is true and the user is unauthenticated; otherwise, allow the
anchor tag's default navigation behavior to handle the routing through the
global SPA interceptor.
In `@frontend/src/routes/Metrics.svelte`:
- Around line 301-305: The fetchSubmissionsData() function call at line 305
needs to be updated to align with the backend contract. For anonymous users, the
backend returns 403 when requesting full metrics without the summary_only
parameter. Check if the user is authenticated before calling
fetchSubmissionsData, and if unauthenticated, either pass summary_only: true as
a parameter to the function call or conditionally request only summary metrics
for anonymous users. This ensures the frontend respects the backend's permission
model and prevents page-level errors for unauthenticated users viewing the
metrics page. Apply the same fix to the other fetchSubmissionsData calls
mentioned in the comment (around lines 1768 and 1924).
---
Outside diff comments:
In `@backend/contributions/views.py`:
- Around line 71-84: The get_queryset method in this view is accessible to
unauthenticated users due to the AllowAny permission class, but the
active_submissions queryset counts submissions from all users including those
with hidden profiles. This leaks information about hidden user activity. Filter
the active_submissions queryset by adding user__visible=True before the
aggregate annotation to ensure the submission_count only reflects contributions
from publicly visible users.
In `@backend/leaderboard/views.py`:
- Around line 63-68: The leaderboard viewset permits anonymous access with
pagination disabled, allowing unrestricted retrieval of all entries. Replace the
`pagination_class = None` setting with a proper pagination class that enforces a
maximum page size for anonymous users. Implement explicit bounds on the offset
and limit parameters to prevent abuse, and apply a safe default page size (e.g.,
100 entries) for anonymous requests. Ensure the same pagination bounds are
applied to the related viewset code mentioned at lines 151-174 to maintain
consistent access controls across all public leaderboard endpoints.
In `@backend/users/views.py`:
- Around line 136-159: The user_highlights action has two security issues that
need to be addressed before public exposure. First, the queryset filters by user
but does not apply the public contribution/highlight visibility predicate that
is used elsewhere in the codebase to control which highlights are visible based
on their public/private status. Second, the limit parameter extracted from
request.query_params has no input validation and can accept non-numeric,
negative, or excessively large values. Add the appropriate visibility filter to
the queryset based on the same predicate used elsewhere, and add validation to
clamp the limit parameter to a reasonable range or reject invalid input before
using it to slice the queryset.
- Around line 892-909: The search method lacks both input validation for maximum
query length and deduplication of results. Add a maximum length check for the
query parameter after the existing minimum length check (similar to how
len(query) < 2 is validated), and add .distinct() to the User.objects.filter()
queryset chain before the slice operation to eliminate duplicate rows that can
result from the join with githubconnection__platform_username. This will cap
unbounded search queries and ensure stable, non-duplicated results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94bef21f-2651-4f56-954f-819cbfae0f51
📒 Files selected for processing (21)
backend/api/metrics_views.pybackend/api/tests.pybackend/contributions/tests/test_canceled_submissions.pybackend/contributions/tests/test_is_submittable.pybackend/contributions/tests/test_steward_permissions.pybackend/contributions/views.pybackend/leaderboard/tests/test_stats.pybackend/leaderboard/views.pybackend/stewards/tests.pybackend/stewards/views.pybackend/tally/settings.pybackend/users/tests/test_email_security.pybackend/users/views.pyfrontend/src/App.sveltefrontend/src/components/AuthButton.sveltefrontend/src/components/Navbar.sveltefrontend/src/components/Sidebar.sveltefrontend/src/components/ui/SectionHeader.sveltefrontend/src/routes/Dashboard.sveltefrontend/src/routes/Leaderboard.sveltefrontend/src/routes/Metrics.svelte
Comment on lines
+301
to
+305
| const typesResponse = await api.get('/contribution-types/', { params: { page_size: 1000 } }); | ||
| contributionTypes = normalizeContributionTypes( | ||
| typesResponse.data.results || typesResponse.data || [] | ||
| ); | ||
| await fetchSubmissionsData({ syncDates: true }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Align the public metrics request with the backend contract.
fetchSubmissionsData() still calls /steward-submissions/daily-metrics/ without summary_only, while the supplied backend view returns 403 for unauthenticated non-summary requests. With the section now rendered unconditionally, anonymous users can hit the page-level error path instead of seeing the public metrics page. Either make the backend allow this exact public full-metrics request, or have the frontend request/render only the public summary_only contract for anonymous users.
Also applies to: 1768-1768, 1924-1924
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/routes/Metrics.svelte` around lines 301 - 305, The
fetchSubmissionsData() function call at line 305 needs to be updated to align
with the backend contract. For anonymous users, the backend returns 403 when
requesting full metrics without the summary_only parameter. Check if the user is
authenticated before calling fetchSubmissionsData, and if unauthenticated,
either pass summary_only: true as a parameter to the function call or
conditionally request only summary metrics for anonymous users. This ensures the
frontend respects the backend's permission model and prevents page-level errors
for unauthenticated users viewing the metrics page. Apply the same fix to the
other fetchSubmissionsData calls mentioned in the comment (around lines 1768 and
1924).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/users/views.py (1)
43-54: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate and cap
limitfor the now-public highlights endpoint.Line 140 parses untrusted input with
int(...)and no guard; malformed values can raise a server error, and unbounded values can amplify query/load on an unauthenticated route.Suggested fix
@@ - limit = int(request.query_params.get('limit', 5)) + try: + limit = int(request.query_params.get('limit', 5)) + except (TypeError, ValueError): + return Response( + {'error': 'Invalid limit value'}, + status=status.HTTP_400_BAD_REQUEST, + ) + limit = max(1, min(limit, 50)) @@ - highlights = queryset[:limit] + highlights = queryset[:limit]As per path instructions, "Flag missing input validation, unsafe filtering, dynamic ordering/search abuse, and SQL/ORM/raw-query injection risks from string-built queries."
Also applies to: 140-141, 159-160
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/users/views.py` around lines 43 - 54, Add input validation and enforce a maximum limit cap for the public highlights endpoint and search endpoint. Locate where the limit parameter is parsed using int() around line 140 (in the user_highlights action handler) and lines 159-160 (in the search action handler). For both locations, wrap the int() conversion in a try-except block to handle malformed input gracefully, and enforce a maximum cap on the limit value to prevent unbounded query amplification on unauthenticated routes. Return an appropriate error response for invalid limit values instead of allowing the server to raise an exception.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/users/views.py`:
- Around line 43-54: Add input validation and enforce a maximum limit cap for
the public highlights endpoint and search endpoint. Locate where the limit
parameter is parsed using int() around line 140 (in the user_highlights action
handler) and lines 159-160 (in the search action handler). For both locations,
wrap the int() conversion in a try-except block to handle malformed input
gracefully, and enforce a maximum cap on the limit value to prevent unbounded
query amplification on unauthenticated routes. Return an appropriate error
response for invalid limit values instead of allowing the server to raise an
exception.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 138d062e-8eb4-442f-9b0a-874dc44bf401
📒 Files selected for processing (12)
backend/api/metrics_views.pybackend/contributions/tests/test_canceled_submissions.pybackend/contributions/tests/test_steward_permissions.pybackend/contributions/views.pybackend/leaderboard/tests/test_stats.pybackend/leaderboard/views.pybackend/users/serializers.pybackend/users/tests/test_email_security.pybackend/users/views.pyfrontend/src/components/AuthButton.sveltefrontend/src/components/Sidebar.sveltefrontend/src/components/ui/SectionHeader.svelte
💤 Files with no reviewable changes (1)
- frontend/src/components/Sidebar.svelte
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates the unauthenticated experience so Overview, role dashboards, leaderboards, public profiles, and search can be browsed without connecting a wallet.
Keeps contribution submission, protected subsections, steward workflows, referrals, and contribution detail flows behind authentication.
Moves builder, validator, and community leaderboards into the public leaderboard view and removes validator waitlist dashboard surfaces.
Adds public-safe API responses for metrics, steward aggregates, user profiles/search, and leaderboard data with visibility filtering and throttling.
Summary by CodeRabbit