-
Notifications
You must be signed in to change notification settings - Fork 467
feat: parsing & validating the new redirect url patterns #2048
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
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 15561832509Details
💛 - Coveralls |
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.
Generally fine from a quick glance
} | ||
|
||
// CategorizePattern determines the type of a redirect uri and returns a RedirectPattern | ||
func CategorizePattern(pattern string) (RedirectPattern, error) { |
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 see a few glob.Compile()
calls in this function. It should be OK from a performance POV and definitely not a blocker, but maybe double-check (if you haven't already) what the library docs say about it
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.
Good catch! We're only calling glob.Compile()
during app startup when parsing the config, not during the requests (aka compile-once, match-many). Since it's a one-time initialization cost, there's no performance concern here. The library warns against compiling repeatedly in hot paths, which we're not doing.
We're good!
Summary
Implements the new redirect URI validation system as specified in the technical specification. This introduces a cleaner, more maintainable approach to redirect URI validation with automatic pattern categorization.
What kind of change does this PR introduce?
Feature - New redirect URI validation system with pattern categorization
What is the current behavior?
Supabase Auth currently uses an overly permissive redirect URL system that supports dangerous wildcard patterns like
**
which create security vulnerabilities The validation logic is scattered and uses complex glob patterns that are difficult to maintain and debug.What is the new behavior?
Implements Phase 1 of the redirect URI security enhancement by introducing a new categorized validation system in the
internal/redirecturi
package. This lays the foundation for migrating away from dangerous wildcard patterns to a more secure, OAuth-compliant approach.New pattern categories:
https://example.com/callback
(literal matching)example.com
(any HTTPS path on domain)*.preview.app
(subdomain matching only)myapp://
(mobile app deep links)Additional context
Merging this PR won't have any affect on the existing behaviour.
The actual parsing & validating (shadow mode) will be enabled in a future PR.