-
-
Notifications
You must be signed in to change notification settings - Fork 964
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: Do not send session after registration without hook #2094
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This looks good to merge! I made one little change because we can actually just omit the session in the registration hook handler. If a session is issued by the session issuer, the flow is completely aborted, meaning that we never end up at that linked statement :)
Thanks for the feedback!
I am not sure this is true, judging from the failing test case?
|
Perhaps there needs to be another check in |
Sorry my bad! I worked on it but had to leave. I’ll push my changes later :) |
Thank you @meyfa for report & fix :) |
Thank you so much for making this such a quick process & being super helpful! :) |
Hello @meyfa |
See ory#2093 Co-authored-by: aeneasr <[email protected]>
This fixes the registration hooks to not include a
session
property in the response for API and SPA flows if there is no"session"
hook configured. This new behavior matches up with pre-existing documentation.Related issue(s)
Fixes #2093
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments
"session"
hook determines whether a valid session will be created, checking for that works well here. I do not know whether there is a more explicit way of verifying session validity that does not depend on the config.hook.KeySessionIssuer
constant but that resulted in a circular import.ContentNegotiationRedirection
.