Skip to content

Commit 4fca412

Browse files
AndroidPoetspydon
andauthored
fix(supabase_flutter): preserve hash routes and repeated query keys when clearing auth params (#1511)
## What `removeAuthParametersFromUrl` cleans the auth parameters out of a URL after a code exchange. It had two bugs that broke URLs it was supposed to leave intact: 1. **Hash-based routes were corrupted.** The fragment was parsed with `Uri.splitQueryString` and re-emitted as query parameters, so a route like `#/dashboard` became `#%2Fdashboard`. Hash-based routing is Flutter web's **default** URL strategy, so after a PKCE callback the app route was mangled. 2. **Repeated query keys were dropped.** It read `queryParameters` (last value wins), so `?tag=a&tag=b` collapsed to `?tag=b`. Both contradict the function's own docstring ("preserving any unrelated parameters"). ## Why ```dart // (1) fragment treated as a query string, then re-encoded Uri.splitQueryString(currentUri.fragment) // '/dashboard' -> {'/dashboard': ''} // (2) last-wins map loses duplicates Map<String, String>.of(currentUri.queryParameters) // ?tag=a&tag=b -> {tag: b} ``` Fix: - Use `queryParametersAll` so repeated keys are preserved. - Only rewrite the fragment when it actually contains an auth parameter (implicit flow: `#access_token=...`); otherwise preserve it verbatim so routes aren't touched. ## Not a breaking change Auth clearing is unchanged: the PKCE `code` in the query and implicit-flow tokens in the fragment are still removed. Only previously-corrupted output changes — hash routes and duplicate query keys are now preserved. ## Tests Extends `clear_auth_url_parameters_test.dart` (all 6 existing tests still pass): - `#/dashboard` is preserved (was `#%2Fdashboard`). - `?tag=a&tag=b&code=abc123` → `?tag=a&tag=b` (was `?tag=b`). - Implicit-flow tokens in the fragment are still cleared. The two new bug tests fail on the current code and pass with the fix. `dart format` and `dart analyze --fatal-warnings` are clean. --------- Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
1 parent 682dd96 commit 4fca412

2 files changed

Lines changed: 59 additions & 10 deletions

File tree

packages/supabase_flutter/lib/src/clear_auth_url_parameters.dart

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,34 @@ const _authParameters = {
2525
String removeAuthParametersFromUrl(String url) {
2626
final currentUri = Uri.parse(url);
2727

28-
final query = Map<String, String>.of(currentUri.queryParameters)
28+
final query = Map<String, List<String>>.of(currentUri.queryParametersAll)
2929
..removeWhere((key, value) => _authParameters.contains(key));
3030

31-
final fragmentParameters =
32-
Map<String, String>.of(Uri.splitQueryString(currentUri.fragment))
33-
..removeWhere((key, value) => _authParameters.contains(key));
34-
35-
final fragment = fragmentParameters.isEmpty
36-
? null
37-
: Uri(queryParameters: fragmentParameters).query;
38-
3931
final cleanedUri = Uri(
4032
scheme: currentUri.scheme,
4133
userInfo: currentUri.userInfo,
4234
host: currentUri.host,
4335
port: currentUri.hasPort ? currentUri.port : null,
4436
path: currentUri.path,
4537
queryParameters: query.isEmpty ? null : query,
46-
fragment: fragment,
38+
fragment: _removeAuthParametersFromFragment(currentUri.fragment),
4739
);
4840

4941
return cleanedUri.toString();
5042
}
43+
44+
/// Strips auth parameters from a URL [fragment].
45+
String? _removeAuthParametersFromFragment(String fragment) {
46+
if (fragment.isEmpty) return null;
47+
48+
final fragmentParameters = Uri(query: fragment).queryParametersAll;
49+
final cleaned = {
50+
for (final entry in fragmentParameters.entries)
51+
if (!_authParameters.contains(entry.key)) entry.key: entry.value,
52+
};
53+
if (cleaned.length == fragmentParameters.length) {
54+
return fragment;
55+
}
56+
57+
return cleaned.isEmpty ? null : Uri(queryParameters: cleaned).query;
58+
}

packages/supabase_flutter/test/clear_auth_url_parameters_test.dart

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,46 @@ void main() {
5252
'https://example.com/login?foo=bar',
5353
);
5454
});
55+
56+
test('preserves a hash-based route in the fragment', () {
57+
expect(
58+
removeAuthParametersFromUrl('https://example.com/#/dashboard'),
59+
'https://example.com/#/dashboard',
60+
);
61+
});
62+
63+
test('preserves a hash-based route with its own query', () {
64+
expect(
65+
removeAuthParametersFromUrl('https://example.com/#/dashboard?foo=bar'),
66+
'https://example.com/#/dashboard?foo=bar',
67+
);
68+
});
69+
70+
test('still clears auth tokens from the fragment (implicit flow)', () {
71+
expect(
72+
removeAuthParametersFromUrl(
73+
'https://example.com/#access_token=abc&expires_in=3600',
74+
),
75+
'https://example.com/',
76+
);
77+
});
78+
79+
test('preserves repeated query keys', () {
80+
expect(
81+
removeAuthParametersFromUrl(
82+
'https://example.com/?tag=a&tag=b&code=abc123',
83+
),
84+
'https://example.com/?tag=a&tag=b',
85+
);
86+
});
87+
88+
test('preserves repeated fragment keys alongside auth tokens', () {
89+
expect(
90+
removeAuthParametersFromUrl(
91+
'https://example.com/#access_token=abc&ref=a&ref=b',
92+
),
93+
'https://example.com/#ref=a&ref=b',
94+
);
95+
});
5596
});
5697
}

0 commit comments

Comments
 (0)