Skip to content

fix(auth,windows): populate ID token claims and metadata - #18901

Open
JulianPscheid wants to merge 1 commit into
firebase:mainfrom
JulianPscheid:fix/auth-windows-id-token-claims
Open

JulianPscheid wants to merge 1 commit into
firebase:mainfrom
JulianPscheid:fix/auth-windows-id-token-claims

Conversation

@JulianPscheid

Copy link
Copy Markdown
Contributor

Description

On Windows, User.getIdTokenResult() returns only the raw token from the Firebase C++ SDK. Claims, timestamps and sign-in provider are all unset, so apps can't read custom claims through the public API.

This decodes the JWT payload in the Windows plugin and fills the same fields Android and Apple do: the full claims map, authTime, issuedAtTime, expirationTime, signInProvider, and signInSecondFactor when it's present. JWT timestamps are converted from seconds to milliseconds. Missing or invalid metadata stays unset, and if the payload can't be decoded the original token is still returned.

The public C++ SDK headers don't expose their internal JSON parser or base64 helpers, so this uses the Windows base64 decoder that's already linked through firebase_core, plus a small JSON-to-EncodableValue parser for the payload. No new dependency. The existing native test target now links the static plugin and tests the token-result parser, replacing its obsolete getPlatformVersion stub. I also enabled the existing valid-token-result e2e test on Windows and added an Auth emulator test that checks custom claims, nested values, Unicode and all the token metadata it can.

Testing (Flutter 3.47.9, Dart 3.13.7, Firebase C++ SDK 14.1.0):

  • Windows native tests: before the fix, two tests failed on null claims and the raw-token fallback test passed. After the fix all three pass. They cover second-factor metadata, malformed JSON and base64, invalid Unicode, and missing or invalid metadata.
  • Windows example: flutter build windows --debug passes on this branch.
  • Windows Auth emulator: firebase emulators:exec --only auth --project flutterfire-e2e-tests "cd ../../../packages/firebase_auth/firebase_auth/example && flutter test integration_test/e2e_test.dart -d windows --plain-name getIdTokenResult" passes both enabled token-result tests. One existing Windows-skipped test also matches the filter.
  • macOS: melos exec --scope=firebase_auth --scope=firebase_auth_platform_interface -c 1 -- flutter test passes 64 and 211 tests. melos run analyze-ci --scope='firebase_auth*' --no-select passes for all four selected packages.
  • Scoped flutter_plugin_tools format (with the repo's clang-format config), Dart formatting and git diff --check pass.

To run the native tests, build the example first, configure its build directory with -Dinclude_firebase_auth_tests=ON, build the firebase_auth_test target, and run build/windows/x64/plugins/firebase_auth/Debug/firebase_auth_test.exe.

Not covered: the full Auth e2e suite, runtime tests on other platforms, a live Auth backend, a real Windows MFA sign-in, and parser fuzzing. The tests use only the Auth emulator and the generated dummy example config.

Related Issues

Fixes #11768 (closed earlier without the Windows implementation changing).

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Decode the JWT payload returned by the C++ SDK and populate claims, authentication/issue/expiry times, sign-in provider and second factor when present. Keep returning the raw token if optional payload decoding fails.

Use the already-linked Windows base64 decoder, exercise the parser in native tests, and verify custom claims and metadata against the Auth emulator. Enable the existing token-result integration check on Windows.

Fixes firebase#11768

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements local parsing of ID token claims on Windows for firebase_auth. It introduces a custom JSON parser (IdTokenClaimsParser) and base64 decoding logic using Windows Cryptography APIs to extract claims, timestamps, and provider metadata from the ID token. It also updates integration tests in Dart and unit tests in C++ to verify the parsing behavior. Feedback on the changes includes: replacing the preprocessor macro MAXDWORD with std::numeric_limits::max() for better C++ type safety, explicitly linking the crypt32 library in CMake to ensure Windows Cryptography APIs are available, and handling potential trailing null terminators in the test helper when using CryptBinaryToStringA.

if (c == '_') c = '/';
}
payload.append((4 - payload.size() % 4) % 4, '=');
if (payload.size() > MAXDWORD) return std::nullopt;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Using std::numeric_limits<DWORD>::max() is more idiomatic in C++ and provides better type safety compared to the preprocessor macro MAXDWORD.

Suggested change
if (payload.size() > MAXDWORD) return std::nullopt;
if (payload.size() > (std::numeric_limits<DWORD>::max)()) return std::nullopt;

Comment on lines +110 to +111
target_link_libraries(${TEST_RUNNER} PRIVATE
${PLUGIN_NAME} firebase_core_plugin flutter_wrapper_plugin gtest_main)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Explicitly link the crypt32 library in CMake to ensure that Windows Cryptography APIs (CryptStringToBinaryA and CryptBinaryToStringA) are always available, making the target self-contained and robust against changes in dependencies.

target_link_libraries(${TEST_RUNNER} PRIVATE
  ${PLUGIN_NAME} firebase_core_plugin flutter_wrapper_plugin gtest_main crypt32)

Comment on lines +28 to +29
payload.resize(length);
while (!payload.empty() && payload.back() == '=') payload.pop_back();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

To ensure robustness across different Windows versions and environments, handle potential trailing null terminators (\0) that might be written by CryptBinaryToStringA and included in the returned length.

  payload.resize(length);
  while (!payload.empty() && (payload.back() == '=' || payload.back() == '\0')) {
    payload.pop_back();
  }

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 [firebase_auth] getIdTokenResult returns null values in IdTokenResult on Windows

1 participant