-
Notifications
You must be signed in to change notification settings - Fork 972
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
chore: don't return allowed return URLs #4044
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4044 +/- ##
===========================================
+ Coverage 0 78.34% +78.34%
===========================================
Files 0 370 +370
Lines 0 26118 +26118
===========================================
+ Hits 0 20461 +20461
- Misses 0 4096 +4096
- Partials 0 1561 +1561 ☔ View full report in Codecov by Sentry. |
WithDebugf("Allowed domains are: %v", strings.Join(lo.Map(o.allowlist, func(u url.URL, _ int) string { | ||
return u.String() | ||
}), ", "))) | ||
WithReasonf("Requested return_to URL %q is not allowed.", returnTo), |
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.
Log this instead then? Or trace?
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.
I don't think that adds any value, because you can always check the project configuration.
I added this a while back mainly to make live for a developer implementing Ory a little easier, but in this configuration we're leaking this information to end users if they deliberately trigger this error (e.g. by providing an illegal return_to
).
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.
ok
Related issue(s)
Checklist
introduces a new feature.
contributing code guidelines.
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.
works.
Further Comments