Conversation
8e7b99a to
c7f7c31
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 68 changed files in this pull request and generated 12 comments.
Files not reviewed (4)
- .idea/.gitignore: Language not supported
- .idea/auth0-flutter.iml: Language not supported
- .idea/modules.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Credentials creds = DecodeTokenResponse(json); | ||
|
|
||
| EXPECT_EQ(creds.accessToken, "test_access_token"); | ||
| EXPECT_EQ(creds.tokenType, "Bearer"); | ||
| EXPECT_FALSE(creds.idToken.empty()); // Empty, not uninitialized | ||
| EXPECT_FALSE(creds.refreshToken.has_value()); | ||
| EXPECT_FALSE(creds.expiresIn.has_value()); |
There was a problem hiding this comment.
In the minimal token response case, DecodeTokenResponse leaves creds.idToken as an empty string when id_token is absent, but this test asserts it is non-empty (EXPECT_FALSE(creds.idToken.empty())). This assertion will fail; either require id_token in DecodeTokenResponse (throw if missing) or update the test expectation to allow an empty idToken for non-OIDC responses.
| static std::optional<bool> GetBoolOrFalse( | ||
| const EncodableMap& map, | ||
| const std::string& key) { | ||
| auto it = map.find(EncodableValue(key)); | ||
| if (it == map.end()) return std::nullopt; | ||
| if (!std::holds_alternative<bool>(it->second)) return std::nullopt; | ||
| return std::get<bool>(it->second); | ||
| } |
There was a problem hiding this comment.
GetBoolOrFalse returns std::nullopt when the key is missing or not a bool, despite the name suggesting a default false. This also conflicts with the added unit test that calls .value() expecting false for non-bool values. Either return false in those cases, rename the helper to reflect optional<bool> semantics, or adjust the tests/callers to handle nullopt safely.
| import 'dart:io' show Platform; | ||
|
|
||
| import 'package:auth0_flutter_platform_interface/auth0_flutter_platform_interface.dart'; | ||
|
|
||
| import '../../auth0_flutter.dart'; |
There was a problem hiding this comment.
Platform is imported but not used anywhere in this file, which will trigger an unused import warning in analyzer/lints. Remove the import or use it (e.g., by asserting Windows-only usage) to keep the package analysis clean.
| /// Basic login: | ||
| /// ```dart | ||
| /// final auth0 = Auth0('DOMAIN', 'CLIENT_ID'); | ||
| /// final result = await auth0.webAuthentication().login( | ||
| /// redirectUrl: 'http://localhost:8080/callback', | ||
| /// ); | ||
| /// final accessToken = result.accessToken; | ||
| /// ``` |
There was a problem hiding this comment.
The “Basic login” doc snippet calls auth0.webAuthentication().login(...), but this class is Windows-specific and the rest of the docs reference windowsWebAuthentication(). This example should use auth0.windowsWebAuthentication().login(...) to avoid misleading users on Windows.
| - name: Set up vcpkg | ||
| uses: lukka/run-vcpkg@v11 # pin@v11 | ||
| with: |
There was a problem hiding this comment.
This workflow introduces unpinned third-party actions (lukka/run-vcpkg@v11 and multiple actions/upload-artifact@v6). Elsewhere in this workflow actions are pinned to commit SHAs; these should be pinned as well to reduce supply-chain risk and keep consistency with the repo’s existing CI security posture.
| # === Tests === | ||
| option(AUTH0_FLUTTER_ENABLE_TESTS "Build auth0_flutter unit tests" ON) | ||
|
|
||
| if (AUTH0_FLUTTER_ENABLE_TESTS) | ||
| enable_testing() | ||
|
|
There was a problem hiding this comment.
Unit tests are enabled by default (AUTH0_FLUTTER_ENABLE_TESTS is ON), which means normal plugin consumers will download/build GoogleTest via FetchContent during flutter build windows. Default this option to OFF (and only enable in CI/dev) to avoid slow, network-dependent, and potentially policy-violating builds for end users.
| /// ```dart | ||
| /// final auth0 = Auth0('DOMAIN', 'CLIENT_ID'); | ||
| /// final result = await auth0.windowsWebAuthentication().login( | ||
| /// redirectUrl: 'auth0-flutter://callback', |
There was a problem hiding this comment.
The Windows WebAuth usage example uses redirectUrl: 'auth0-flutter://callback', but the Windows implementation and docs elsewhere use auth0flutter://callback (no hyphen). This example should be corrected to avoid users registering/using the wrong custom scheme.
| /// redirectUrl: 'auth0-flutter://callback', | |
| /// redirectUrl: 'auth0flutter://callback', |
| @@ -86,11 +87,37 @@ If your Auth0 domain was `company.us.auth0.com` and your package name (Android) | |||
| - Android: `https://company.us.auth0.com/android/com.company.myapp/callback` | |||
| - iOS: `https://company.us.auth0.com/ios/com.company.myapp/callback,com.company.myapp://company.us.auth0.com/ios/com.company.myapp/callback` | |||
| - macOS: `https://company.us.auth0.com/macos/com.company.myapp/callback,com.company.myapp://company.us.auth0.com/macos/com.company.myapp/callback` | |||
| - Windows: `https://your-app.example.com/callback` (your intermediary server endpoint) | |||
|
|
|||
| </details> | |||
|
|
|||
| > 💡 **Windows**: The Windows implementation uses a custom scheme callback architecture (`auth0flutter://callback`). This requires an intermediary server to receive the Auth0 callback and forward it to your Windows app via the custom protocol. The intermediary server URL (e.g., `https://your-app.example.com/callback`) should be configured as the callback URL in your Auth0 dashboard. The server should handle the Auth0 redirect and trigger the `auth0flutter://` protocol to activate your app with the authorization code and state parameters. | |||
|
|
|||
There was a problem hiding this comment.
The README states that Windows “requires an intermediary server”, but the new WindowsWebAuthentication docs and PR description both describe a direct callback option using auth0flutter://callback without a server. The README should document both patterns (direct custom-scheme redirect and intermediary HTTPS redirect) to avoid contradictory setup guidance.
| std::string urlEncode(const std::string &str) | ||
| { | ||
| std::ostringstream encoded; | ||
| encoded.fill('0'); | ||
| encoded << std::hex << std::uppercase; | ||
|
|
||
| for (unsigned char c : str) | ||
| { | ||
| if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') | ||
| { | ||
| encoded << c; |
There was a problem hiding this comment.
urlEncode uses isalnum without including <cctype> (and without qualifying it as std::isalnum), which can lead to build failures or locale-dependent behavior. Include <cctype> and call std::isalnum(static_cast<unsigned char>(c)) (or equivalent) for a well-defined implementation.
| #include "url_utils.h" | ||
| #include <sstream> | ||
|
|
||
| namespace auth0_flutter | ||
| { | ||
|
|
||
| std::string UrlDecode(const std::string &str) | ||
| { | ||
| std::string out; | ||
| out.reserve(str.size()); | ||
| for (size_t i = 0; i < str.size(); ++i) | ||
| { | ||
| char c = str[i]; | ||
| if (c == '%') | ||
| { | ||
| if (i + 2 < str.size()) | ||
| { | ||
| std::string hex = str.substr(i + 1, 2); | ||
| char decoded = (char)strtol(hex.c_str(), nullptr, 16); | ||
| out.push_back(decoded); |
There was a problem hiding this comment.
UrlDecode calls strtol but this file doesn’t include <cstdlib>/<stdlib.h>, which can cause missing-declaration build errors depending on the toolchain. Add the appropriate standard header include for strtol.
| final Account _account; | ||
| final UserAgent _userAgent; | ||
| final String? _scheme; | ||
| final CredentialsManager? _credentialsManager; |
There was a problem hiding this comment.
Do we need the _scheme and _credentialsManager in this class.
scheme is used for generating the redirection url. But since we are asking the user to explicitly add the redirect url for Windows, this serves no purpose. Also we currently don't have the any credentials storing support. We can add this when we add that support
| /// Auth0 redirects directly to the app; no server is required. | ||
| /// - `https://your-server.com/callback` — an HTTPS endpoint on an | ||
| /// intermediary server that receives the Auth0 redirect and forwards it | ||
| /// to the app via the `auth0flutter://callback` scheme. |
There was a problem hiding this comment.
The intermediary server redirect is a workaround . So we shouldn't add that in the official docs. We can add the workaround flow and related references in Examples.md file or a new file
| /// **Example:** | ||
| /// ```dart | ||
| /// await auth0.webAuthentication().login( | ||
| /// redirectUrl: 'http://localhost:8080/callback', |
There was a problem hiding this comment.
lets give a windows specific redirect url here
| /// [federated] controls whether to perform federated logout, which also logs | ||
| /// the user out from their identity provider. | ||
| Future<void> logout({ | ||
| final String? returnTo, |
There was a problem hiding this comment.
Shouldn't his also be a required field to return back post login ?
| /// | ||
| static void cancel() { | ||
| Auth0FlutterWebAuthPlatform.instance.cancel(); | ||
| } |
There was a problem hiding this comment.
Remove this cancel method if it doesn't apply to Windows
| await Auth0('test-domain', 'test-clientId') | ||
| .webAuthentication() | ||
| .login(parameters: { | ||
| 'appCallbackUrl': 'myapp://callback', |
There was a problem hiding this comment.
appCallbackUrl property is not being used anymore. Update the tests to avoid any confusion
| .single as WebAuthRequest<WebAuthLoginOptions>; | ||
| // ignore: inference_failure_on_collection_literal | ||
| expect(verificationResult.options.parameters, { | ||
| 'appCallbackUrl': 'myapp://callback', |
There was a problem hiding this comment.
appCallbackUrl property is not being used anymore. Update the tests to avoid any confusion
| expect(verificationResult.options.useEphemeralSession, false); | ||
| }); | ||
|
|
||
| test('passes custom parameters to platform', () async { |
There was a problem hiding this comment.
You should have a new testClass for WindowsWebAuthentication. The new test class should test specific scenarios like mandatory redirect uri to login and logout method etc
| // If no scopes provided, use just "openid" | ||
| scopeStr = "openid"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Do you need such a long logic for scopes? Can't it be simple as append scopes from the list to the existing default scope string ?
There was a problem hiding this comment.
this is required to append open id if not already present in the scopes
There was a problem hiding this comment.
Yes I know we need the openId scope always. What I meant is to simplify this logic a bit if we can . For example we can define a set with default scopes like
std::set<std::string> scopes = {"openid", "profile", "email"};
And then add to this set from the scope list passed. This will ensure no duplicates are added.
In the end just append all the items to a single string.
This will be more readable and straightforward. No need for all the flag check and everything for openid
📋 Changes
This PR adds native Windows desktop support to the
auth0_flutterSDK, enabling Auth0 Universal Login on Flutter Windows apps using the OAuth 2.0 Authorization Code Flow with PKCE. The implementation is a C++ Flutter plugin that integrates with the existing platform-interface layer without modifying the mobile (iOS/Android) code paths.New:
WindowsWebAuthenticationclassA dedicated Windows authentication class exposed via
Auth0.windowsWebAuthentication(). Unlike the mobileWebAuthenticationclass, this:redirectUrlexplicitly (no platform default exists on Windows)CredentialsManager(no Keychain/Keystore on Windows)parametersmap for Windows-specific configurationauthTimeoutSeconds(default'180'): How long the plugin polls for the OAuth callback before returningUSER_CANCELLED. Increase for slow MFA flows; decrease for fast failure in tests.New: Windows C++ plugin (
auth0_flutter/windows/)login_web_auth_request_handler.cpplogout_web_auth_request_handler.cppoauth_helpers.cppauth0flutter://callback pollingauth0_client.cppid_token_validator.cppid_token_signature_validator.cppjwt_util.cpptoken_decoder.cppCredentialsstructuser_profile.cpp/user_identity.cppUserProfilestructtime_util.cppurl_utils.cppwindows_utils.cppWideToUtf8,BringFlutterWindowToFrontAuthentication flow:
code_verifier(32 cryptographically random bytes viaRAND_bytes) andcode_challenge(SHA-256 via OpenSSL, base64-URL encoded)statevalue for CSRF protection/authorizeURL with all parameters RFC 3986-encodedShellExecuteAPLUGIN_STARTUP_URLenvironment variable (set by Windows when the app is launched via theauth0flutter://custom scheme) every 200 ms until the callback arrives or the timeout expiresstateto prevent CSRF; extractcodecode+code_verifierfor tokens via POST to/oauth/tokenauth_time, nonce, RS256 signature)Key design decisions:
auth0flutter://callback(kDefaultRedirectUri). TheredirectUrlsent to Auth0 may differ (e.g. an HTTPS intermediary server URL); that server is responsible for forwarding toauth0flutter://callback?code=…&state=….authTimeoutSeconds) are consumed before building the authorize URL and are not appended to it.std::threadto avoid blocking the Flutter UI thread.openidscope is always enforced even when not explicitly passed, as required by OpenID Connect.New:
vcpkg.jsondependency manifestManages C++ dependencies via vcpkg, integrating automatically with CMake through the vcpkg toolchain file set by Flutter during
flutter build windows:cpprestsdkopensslRAND_bytes(PKCE entropy), SHA-256 (code challenge), RS256 signature verification, TLSboost-system/boost-date-time/boost-regexNew: Unit tests (Google Test,
auth0_flutter/windows/test/)oauth_helpers_test.cppid_token_validator_test.cppauth_time, nonce, leeway validationjwt_util_test.cpptime_util_test.cpptoken_decoder_test.cppurl_utils_test.cppuser_identity_test.cppuser_profile_test.cppwindows_utils_test.cppWideToUtf8wide-to-UTF-8 conversionTests are compiled as a separate
auth0_flutter_testsexecutable and registered with CTest, enabled via-DAUTH0_FLUTTER_ENABLE_TESTS=ON.New: CI pipeline (
.github/workflows/main.yml)Added a
windows-testsjob that installs vcpkg dependencies, builds the test executable with CMake, and runs all C++ unit tests via CTest onwindows-latest.📎 References
🎯 Testing
Automated — C++ unit tests (Windows)
All 9 test suites pass.
Automated — Flutter unit tests (any platform)
Manual — end-to-end on Windows
Prerequisites:
auth0flutteras a custom URL scheme pointing to your app executable (via installer or registry)auth0flutter://callbackto Allowed Callback URLs in the Auth0 dashboardcd auth0_flutter/example flutter run -d windowsauth0flutter://callback?code=…&state=…To test the intermediary server pattern, point
redirectUrlat an HTTPS endpoint that reads thecodeandstatequery parameters and responds with a redirect toauth0flutter://callback?code=…&state=….