Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove trailing slashes from endpoints, fix check_access for backend roles #1893

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

spwoodcock
Copy link
Member

@spwoodcock spwoodcock commented Nov 18, 2024

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Describe this PR

Trailing slashes, Origin header, and CORS:

  • I removed all trailing slashes from routes in FastAPI.
  • Proper REST APIs should distinguish between /route/ and /route - they are different.
  • We were getting an automatic 307 redirect from FastAPI from /route --> /route/, which was causing the Origin request header to be stripped in Firefox. As a result, with Origin: null the CORS check could not complete, so we get a CORS error.
  • I removed this default functionality from FastAPI.
  • In future all APIs should not end in a trailing slash. Frontend calls should also account for this.

Fixes check_access logic:

  • The logic for check_access on the roles was broken a bit.
  • If no org_id was passed, then we need to extract the org_id from the project - fixed this.
  • I also updated the project_roles dict to correctly return when user details are returned from the APIs.
  • Finally, I converted the project role enum values into numerical values for assessing the hierarchy in check_access.

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

mKwrd0qX7vRuYxQkRf

@spwoodcock spwoodcock added the backend Related to backend code label Nov 18, 2024
@spwoodcock spwoodcock self-assigned this Nov 18, 2024
@github-actions github-actions bot added bug Something isn't working docs Improvements or additions to documentation frontend Related to frontend code devops Related to deployment or configuration tests Related to automated code tests labels Nov 18, 2024
@spwoodcock spwoodcock merged commit 743495f into development Nov 18, 2024
9 checks passed
@spwoodcock spwoodcock deleted the fix/trailing-slashes branch November 18, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code bug Something isn't working devops Related to deployment or configuration docs Improvements or additions to documentation frontend Related to frontend code tests Related to automated code tests
Projects
Development

Successfully merging this pull request may close these issues.

3 participants