Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Catch errors when failing to verify JWTs #5153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rolodato
Copy link
Member

Fixes #4982.

The issue as described is incorrect, as the SAML routes already use AllowAny. The problem is actually caused by get_validated_token returning an exception when called with an invalid JWT. We now catch any of these expected errors and gracefully fail the authentication instead.

This is completely untested.

@rolodato rolodato requested a review from a team as a code owner February 21, 2025 23:25
@rolodato rolodato requested review from gagantrivedi and removed request for a team February 21, 2025 23:25
Copy link

vercel bot commented Feb 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Feb 21, 2025 11:25pm
flagsmith-frontend-preview ⬜️ Ignored (Inspect) Feb 21, 2025 11:25pm
flagsmith-frontend-staging ⬜️ Ignored (Inspect) Feb 21, 2025 11:25pm

@github-actions github-actions bot added api Issue related to the REST API fix labels Feb 21, 2025
Copy link
Contributor

github-actions bot commented Feb 21, 2025

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-5153 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-5153 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-5153 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-5153 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-5153 Finished ✅ Results

Copy link
Contributor

github-actions bot commented Feb 21, 2025

Uffizzi Ephemeral Environment deployment-61216

☁️ https://app.uffizzi.com/github.com/Flagsmith/flagsmith/pull/5153

📄 View Application Logs etc.

What is Uffizzi? Learn more!

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.47%. Comparing base (210519e) to head (6f8448e).
Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5153   +/-   ##
=======================================
  Coverage   97.46%   97.47%           
=======================================
  Files        1224     1224           
  Lines       42545    42571   +26     
=======================================
+ Hits        41466    41495   +29     
+ Misses       1079     1076    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api Issue related to the REST API fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not check cookie authentication for /auth/saml/{configuration}/request/
1 participant