Bug: Potential permission bypass in IsChallengeCreator for SAFE_METHODS

[iceybubble]

While reviewing the challenge permissions, I came across a point in the IsChallengeCreator permission class that might need clarification or adjustment.

In apps/challenges/permissions.py, the has_permission method currently allows all SAFE_METHODS (GET, HEAD, OPTIONS) without performing an explicit authentication or authorization check:

if request.method in permissions.SAFE_METHODS:
    return True

As IsChallengeCreator is used in challenge-related views, this behavior may result in broader access than intended for certain GET endpoints.

Additionally, the permission logic accesses request.parser_context["kwargs"]["challenge_pk"] directly, which may raise a KeyError if the expected key is not present in the request context.

I wanted to bring this to attention and check whether this aligns with the intended access control.

[Youssef-SH]

I took a closer look at how IsChallengeCreator is used across the challenges views, and this appears to be more than a defensive KeyError concern.

IsChallengeCreator.has_permission() currently allows all SAFE_METHODS unconditionally. Since this permission is combined with IsAuthenticated and HasVerifiedEmail, any authenticated user with a verified email can access GET endpoints guarded by this class without being a challenge creator or host.

In practice, IsChallengeCreator is applied to several GET endpoints that expose challenge details, phase configuration, participant data, analytics, and CSV exports. These do not look like endpoints that should be readable by unrelated users.

If public reads are intended, applying IsChallengeCreator to those views is misleading. Otherwise, GET requests should likely still enforce creator or host checks instead of bypassing authorization entirely.

Also, accessing request.parser_context["kwargs"]["challenge_pk"] directly in a permission class is fragile and may fail depending on routing. Using request.resolver_match.kwargs.get("challenge_pk") and failing closed would be safer.

Happy to help with a fix or clarification if needed.

[iceybubble]

Thanks for the perspective — it helped clarify the broader impact of the current behavior.

When I opened the issue, my focus was on the permission logic itself: SAFE_METHODS currently bypass all creator/host checks, which means IsChallengeCreator doesn’t really participate in authorization for GET requests. The direct access to parser_context["kwargs"]["challenge_pk"] also seemed fragile in a permission class.

To me, the key question is intent. If IsChallengeCreator is meant to represent strict creator/host access, then skipping those checks for GET requests weakens that contract. If broader read access is intentional, then the permission’s name and usage across views may be mixing read visibility with write authorization.

Either way, making that distinction explicit in code — and resolving challenge_pk more defensively — would improve clarity. I’m happy to help implement a fix once the intended access model is clear.

[kanankotwani28]

If the team agrees this needs a fix, I’d be happy to take this up and implement the changes once the intended access model is confirmed.

Please let me know if I can work on this and if there’s a preferred direction for handling SAFE_METHODS and challenge_pk resolution.

[AmanChoudhary17]

Hi, I’m interested in contributing to this issue and learning more about the permission logic here.

From the discussion, it seems the main question is whether SAFE_METHODS should bypass creator checks or still enforce IsChallengeCreator for GET requests.

I’d be happy to work on this once the intended access control behavior is confirmed. Please let me know if I can take this up.

[iceybubble]

Thanks @kanankotwani28 and @AmanChoudhary17 for your interest in this issue!

Since I originally reported this and have been engaged in the discussion with @Youssef-SH, I'd like to take ownership of implementing the fix.

To move forward, I need clarification from the maintainers:

1. Should IsChallengeCreator enforce creator/host checks for GET requests, or is the current SAFE_METHODS bypass intentional?
2. If GET access should be restricted, should we check creator/host permissions, or is there a different access model (e.g., public challenges vs. private)?

Once confirmed, I'll submit a PR that:

• Enforces appropriate permission checks for SAFE_METHODS (if needed)
• Uses safer challenge_pk resolution with request.resolver_match.kwargs.get("challenge_pk")
• Includes tests to verify the behavior

Please let me know if I should proceed, and if there's any specific direction for the fix. Thanks!