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: differentiate between missing sessions and internal server errors #3975

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

osbornk
Copy link
Contributor

@osbornk osbornk commented Jun 28, 2024

Our consumer of the whoami call caches both authorized and unauthorized responses from Kratos. If a session is unauthorized, we should not ping Kratos again for the duration of the cache. However, if Kratos itself had a problem, such as a database error, these responses were coming back as 401s as well. And now we have this potentially false unauthorized response in our cache.

Instead, a 500 is the correct response for a Kratos failure. That way our cache can choose to do nothing with this response and not end up with a poisoned cache.

With this change, an invalid session continued to produce a 401. Valid sessions continue to produce a 200. A Kratos failure, such as a database problem, produces a 500.

BREAKING CHANGES: A Kratos failures for `whoami` currently returns a 401. It will now return a 500.

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

@osbornk osbornk changed the title Differentiate between missing sessions and internal server errors fix: Differentiate between missing sessions and internal server errors Jun 28, 2024
@osbornk osbornk changed the title fix: Differentiate between missing sessions and internal server errors fix: differentiate between missing sessions and internal server errors Jun 28, 2024
@osbornk osbornk marked this pull request as ready for review July 22, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant