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

Fix some lints and remove panic in auth action #166

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Conversation

adam-cattermole
Copy link
Member

@adam-cattermole adam-cattermole commented Feb 6, 2025

Only a couple minor changes:

  • Move the logic for FailureMode resolution into a resolve_failure_mode function to reduce duplication
  • Remove the 4 panics in the auth_action and, ignoring FailureMode, warn and return an InternalServerError
    • Currently these fields are NOT POPULATED by authorino (so these branches won't be hit under usual conditions) and would indicate the CheckResponse has been tampered with; that being said we don't want to panic and potentially end up denying legitimate requests

@adam-cattermole adam-cattermole requested a review from a team as a code owner February 6, 2025 15:11
@adam-cattermole adam-cattermole self-assigned this Feb 7, 2025
@adam-cattermole adam-cattermole added enhancement New feature or request size/small labels Feb 7, 2025
@adam-cattermole adam-cattermole added this pull request to the merge queue Feb 10, 2025
Merged via the queue into main with commit abe70bb Feb 10, 2025
14 checks passed
@adam-cattermole adam-cattermole deleted the minor-refactor branch February 10, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/small
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants