Skip to content

chore: change error type to 400 if id token processing fails#4389

Closed
jonas-jonas wants to merge 1 commit intomasterfrom
jonas-jonas/idTokenErrorTypes
Closed

chore: change error type to 400 if id token processing fails#4389
jonas-jonas wants to merge 1 commit intomasterfrom
jonas-jonas/idTokenErrorTypes

Conversation

@jonas-jonas
Copy link
Copy Markdown
Member

Related issue(s)

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
    security@ory.sh) 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

@jonas-jonas jonas-jonas requested review from a team and aeneasr as code owners April 22, 2025 15:57
@jonas-jonas jonas-jonas self-assigned this Apr 22, 2025
@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Apr 23, 2025

tests failing :(

@jonas-jonas
Copy link
Copy Markdown
Member Author

Hmmm, returning 400 instead of 500 causes the error handler to add the error to the flow UI, which changes the whole flow of the flow. Technically, this could be considered breaking as well, though I'd argue that the 400 case must be handled by clients anyway, so the impact is low.

However, it does make the tests awkward because of #4381 (there is no way to get the flow ID that was used and to fetch the flow to assert the error message is correct).

Sidenote: I also think, not all the possible errors are 400, but rather an expected 500 (e.g. mis config/mis use of the API).

All in all, a bit more work to get the tests over the line, I think.

@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Apr 23, 2025

Tests are failing otherwise LGTM

@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Apr 28, 2025

I think there is another PR to add misconfiguration errors in Ory Hydra by Patrik, we can probably just use that status code here as well.

@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Apr 28, 2025

This one: #3248

@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Apr 28, 2025

ory/herodot#135

@aeneasr aeneasr closed this Nov 14, 2025
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.

3 participants