Skip to content

Commit 5820ce1

Browse files
committed
bug #243 fix redirect uri validation to allow apps like: com.my.app:/ (MarvinG92)
This PR was squashed before being merged into the 1.x-dev branch. Discussion ---------- fix redirect uri validation to allow apps like: com.my.app:/ This PR fixes a bug where valid mobile redirect URLs (e.g., custom schemes like com.my.app:/) were incorrectly rejected by the URL validation logic. Such redirect URIs are commonly used by mobile apps for OAuth flows and deep linking, and should be considered valid. To clarify the issue and ensure it doesn’t regress in the future, I’ve also added a test case demonstrating the expected behavior with these kinds of URLs. This is one way to solve the problem. Feel free to use a different solution if you prefer. Here are some information about it: https://curity.io/resources/learn/oauth-for-mobile-apps-best-practices/ **Changes:** Fixed the redirect URL validation logic to correctly handle custom mobile schemes Added a unit test to verify acceptance of valid mobile redirect URIs **Test:** ✅ Added test code to assert acceptance of com.my.app:/-style URLs **Test Code** ``` <?php function validUrl1(string $url) { $parsedUri = parse_url($url); return $parsedUri && isset($parsedUri['scheme']); } function validUrl2(string $url) { return preg_match('/^[a-zA-Z][a-zA-Z0-9+.-]*:(?:\/\/[^\/\s?#]+(?:\/[^\s?#]*)?|\/[^\s?#]*)?(?:\?[^\s#]*)?(?:#[^\s]*)?$/', $url) === 1; } echo 'Results with filter_var()'.PHP_EOL; var_dump(filter_var('http://google.com', \FILTER_VALIDATE_URL)); var_dump(filter_var('http://google.com/test', \FILTER_VALIDATE_URL)); var_dump(filter_var('http://google.com/test?query=test', \FILTER_VALIDATE_URL)); var_dump(filter_var('invalid', \FILTER_VALIDATE_URL)); var_dump(filter_var('http://invalid url', \FILTER_VALIDATE_URL)); var_dump(filter_var('io.curity.client:/callback', \FILTER_VALIDATE_URL)); // false -> should return url var_dump(filter_var('com.my.app:/', \FILTER_VALIDATE_URL)); // false -> should return url var_dump(filter_var('myapp://callback#token=123', \FILTER_VALIDATE_URL)); echo PHP_EOL; echo 'Results with parse_url()'.PHP_EOL; var_dump(validUrl1('http://google.com')); var_dump(validUrl1('http://google.com/test')); var_dump(validUrl1('http://google.com/test?query=test')); var_dump(validUrl1('invalid')); var_dump(validUrl1('http://invalid url')); // true -> should be false because of whitespace var_dump(validUrl1('io.curity.client:/callback')); var_dump(validUrl1('com.my.app:/')); var_dump(validUrl1('myapp://callback#token=123')); echo PHP_EOL; echo 'Results with preg_match()'.PHP_EOL; var_dump(validUrl2('http://google.com')); var_dump(validUrl2('http://google.com/test')); var_dump(validUrl2('http://google.com/test?query=test')); var_dump(validUrl2('invalid')); var_dump(validUrl2('http://invalid url')); var_dump(validUrl2('io.curity.client:/callback')); var_dump(validUrl2('com.my.app:/')); var_dump(validUrl2('myapp://callback#token=123')); ``` Commits ------- a79b5e5 fix redirect uri validation to allow apps like: com.my.app:/
2 parents 42bc990 + a79b5e5 commit 5820ce1

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

src/ValueObject/RedirectUri.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class RedirectUri
1616
*/
1717
public function __construct(string $redirectUri)
1818
{
19-
if (!filter_var($redirectUri, \FILTER_VALIDATE_URL)) {
19+
if (1 !== preg_match('/^[a-zA-Z][a-zA-Z0-9+.-]*:(?:\/\/[^\/\s?#]+(?:\/[^\s?#]*)?|\/[^\s?#]*)?(?:\?[^\s#]*)?(?:#[^\s]*)?$/', $redirectUri)) {
2020
throw new \RuntimeException(\sprintf('The \'%s\' string is not a valid URI.', $redirectUri));
2121
}
2222

tests/Unit/RedirectUriTest.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace League\Bundle\OAuth2ServerBundle\Tests\Unit;
6+
7+
use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri;
8+
use PHPUnit\Framework\TestCase;
9+
10+
final class RedirectUriTest extends TestCase
11+
{
12+
public function exceptionRedirectUriProvider(): array
13+
{
14+
return [
15+
['invalid'],
16+
['http://invalid url'],
17+
['http:/invalid'],
18+
['http:/invalid.com'],
19+
['http:/invalid.com/test'],
20+
];
21+
}
22+
23+
/**
24+
* @dataProvider exceptionRedirectUriProvider
25+
*/
26+
public function testInvalidRedirectUris($data): void
27+
{
28+
$this->expectException(\RuntimeException::class);
29+
30+
new RedirectUri($data[0]);
31+
}
32+
33+
public function testValidRedirectUris(): void
34+
{
35+
// Test standard URIs
36+
$this->assertIsObject(new RedirectUri('http://github.com'));
37+
$this->assertIsObject(new RedirectUri('http://github.com/test'));
38+
$this->assertIsObject(new RedirectUri('http://github.com/test?query=test'));
39+
40+
// Test mobile URIs
41+
$this->assertIsObject(new RedirectUri('com.my.app:/'));
42+
$this->assertIsObject(new RedirectUri('com.my.app:/callback'));
43+
$this->assertIsObject(new RedirectUri('myapp://callback#token=123'));
44+
}
45+
}

0 commit comments

Comments
 (0)