Skip to content
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

[google_sign_in] Enable FedCM for web. Use token expiration. #5225

Merged
merged 4 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/google_sign_in/google_sign_in_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 0.12.1

* Enables FedCM on browsers that support this authentication mechanism.
* Uses the expiration timestamps of Credential and Token responses to improve
the accuracy of `isSignedIn` and `canAccessScopes` methods.
* Deprecates `signIn()` method.
* Users should migrate to `renderButton` and `silentSignIn`, as described in
the README.

## 0.12.0+5

* Migrates to `dart:ui_web` APIs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ final CredentialResponse minimalCredential =
'credential': minimalJwtToken,
});

final CredentialResponse expiredCredential =
jsifyAs<CredentialResponse>(<String, Object?>{
'credential': expiredJwtToken,
});

/// A JWT token with predefined values.
///
/// 'email': 'adultman@example.com',
Expand Down Expand Up @@ -55,11 +60,30 @@ const String minimalJwtToken =

/// The payload of a JWT token that contains only non-nullable values.
///
/// "email": "adultman@example.com",
/// "sub": "123456"
/// 'email': 'adultman@example.com',
/// 'sub': '123456'
const String minimalPayload =
'eyJlbWFpbCI6ImFkdWx0bWFuQGV4YW1wbGUuY29tIiwic3ViIjoiMTIzNDU2In0';

/// A JWT token with minimal set of predefined values and an expiration timestamp.
///
/// 'email': 'adultman@example.com',
/// 'sub': '123456',
/// 'exp': 1430330400
///
/// Signed with HS256 and the private key: 'symmetric-encryption-is-weak'
const String expiredJwtToken =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.$expiredPayload.--gb5tnVSSsLg4zjjVH0FUUvT4rbehIcnBhB-8Iekm4';

/// The payload of a JWT token that contains only non-nullable values, and an
/// expiration timestamp of 1430330400 (Wednesday, April 29, 2015 6:00:00 PM UTC)
///
/// 'email': 'adultman@example.com',
/// 'sub': '123456',
/// 'exp': 1430330400
const String expiredPayload =
'eyJlbWFpbCI6ImFkdWx0bWFuQGV4YW1wbGUuY29tIiwic3ViIjoiMTIzNDU2IiwiZXhwIjoxNDMwMzMwNDAwfQ';

// More encrypted JWT Tokens may be created on https://jwt.io.
//
// First, decode the `goodJwtToken` above, modify to your heart's
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,30 @@ void main() {
});
});

group('getCredentialResponseExpirationTimestamp', () {
testWidgets('Good payload -> data', (_) async {
final DateTime? expiration =
getCredentialResponseExpirationTimestamp(expiredCredential);

expect(expiration, isNotNull);
expect(expiration!.millisecondsSinceEpoch, 1430330400 * 1000);
});

testWidgets('No expiration -> null', (_) async {
expect(
getCredentialResponseExpirationTimestamp(minimalCredential), isNull);
});

testWidgets('Bad data -> null', (_) async {
final CredentialResponse bogus =
jsifyAs<CredentialResponse>(<String, Object?>{
'credential': 'some-bogus.thing-that-is-not.valid-jwt',
});

expect(getCredentialResponseExpirationTimestamp(bogus), isNull);
});
});

group('getJwtTokenPayload', () {
testWidgets('happy case -> data', (_) async {
final Map<String, Object?>? data = getJwtTokenPayload(goodJwtToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class GisSdkClient {
_initializeIdClient(
clientId,
onResponse: _onCredentialResponse,
hostedDomain: hostedDomain,
useFedCM: true,
);

_tokenClient = _initializeTokenClient(
Expand All @@ -64,6 +66,8 @@ class GisSdkClient {

_tokenResponses.stream.listen((TokenResponse response) {
_lastTokenResponse = response;
_lastTokenResponseExpiration =
DateTime.now().add(Duration(seconds: response.expires_in));
}, onError: (Object error) {
_logIfEnabled('Error on TokenResponse:', <Object>[error.toString()]);
_lastTokenResponse = null;
Expand Down Expand Up @@ -102,13 +106,18 @@ class GisSdkClient {
void _initializeIdClient(
String clientId, {
required CallbackFn onResponse,
String? hostedDomain,
bool? useFedCM,
}) {
// Initialize `id` for the silent-sign in code.
final IdConfiguration idConfig = IdConfiguration(
client_id: clientId,
callback: allowInterop(onResponse),
cancel_on_tap_outside: false,
auto_select: true, // Attempt to sign-in silently.
hd: hostedDomain,
use_fedcm_for_prompt:
useFedCM, // Use the native browser prompt, when available.
);
id.initialize(idConfig);
}
Expand Down Expand Up @@ -230,6 +239,8 @@ class GisSdkClient {
return id.renderButton(parent, convertButtonConfiguration(options)!);
}

// TODO(dit): Clean this up. https://github.com/flutter/flutter/issues/137727
//
/// Starts an oauth2 "implicit" flow to authorize requests.
///
/// The new GIS SDK does not return user authentication from this flow, so:
Expand All @@ -238,7 +249,15 @@ class GisSdkClient {
/// * If [_lastCredentialResponse] is null, we add [people.scopes] to the
/// [_initialScopes], so we can retrieve User Profile information back
/// from the People API (without idToken). See [people.requestUserData].
@Deprecated(
'Use `renderButton` instead. See: https://pub.dev/packages/google_sign_in_web#migrating-to-v011-and-v012-google-identity-services')
Future<GoogleSignInUserData?> signIn() async {
// Warn users that this method will be removed.
domConsole.warn(
'The google_sign_in plugin `signIn` method is deprecated on the web, and will be removed in Q2 2024. Please use `renderButton` instead. See: ',
<String>[
'https://pub.dev/packages/google_sign_in_web#migrating-to-v011-and-v012-google-identity-services'
]);
// If we already know the user, use their `email` as a `hint`, so they don't
// have to pick their user again in the Authorization popup.
final GoogleSignInUserData? knownUser =
Expand All @@ -265,6 +284,8 @@ class GisSdkClient {
// This function returns the currently signed-in [GoogleSignInUserData].
//
// It'll do a request to the People API (if needed).
//
// TODO(dit): Clean this up. https://github.com/flutter/flutter/issues/137727
Future<GoogleSignInUserData?> _computeUserDataForLastToken() async {
// If the user hasn't authenticated, request their basic profile info
// from the People API.
Expand Down Expand Up @@ -302,9 +323,27 @@ class GisSdkClient {
await signOut();
}

/// Returns true if the client has recognized this user before.
/// Returns true if the client has recognized this user before, and the last-seen
/// credential is not expired.
Future<bool> isSignedIn() async {
return _lastCredentialResponse != null || _requestedUserData != null;
bool isSignedIn = false;
if (_lastCredentialResponse != null) {
final DateTime? expiration = utils
.getCredentialResponseExpirationTimestamp(_lastCredentialResponse);
// All Google ID Tokens provide an "exp" date. If the method above cannot
// extract `expiration`, it's because `_lastCredentialResponse`'s contents
// are unexpected (or wrong) in any way.
//
// Users are considered to be signedIn when the last CredentialResponse
// exists and has an expiration date in the future.
//
// Users are not signed in in any other case.
//
// See: https://developers.google.com/identity/openid-connect/openid-connect#an-id-tokens-payload
isSignedIn = expiration?.isAfter(DateTime.now()) ?? false;
Copy link
Contributor

Choose a reason for hiding this comment

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

So a missing expiration field would indicate that it's already expired? Naively (not knowing the API surface) I would expect that if expiration is optional, then not having it should yield a true result here, so maybe a comment would be helpful?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to document this a little bit further, but in this case, if the CredentialResponse payload cannot be decoded, is null, or doesn't contain an "exp" claim, we assume that the token is bad, and thus the user is not authenticated.

In a valid Google ID Token, the "exp" claim should always be provided:

https://developers.google.com/identity/openid-connect/openid-connect#an-id-tokens-payload

}

return isSignedIn || _requestedUserData != null;
}

/// Clears all the cached results from authentication and authorization.
Expand Down Expand Up @@ -338,12 +377,15 @@ class GisSdkClient {
/// Checks if the passed-in `accessToken` can access all `scopes`.
///
/// This validates that the `accessToken` is the same as the last seen
/// token response, and uses that response to check if permissions are
/// still granted.
/// token response, that the token is not expired, then uses that response to
/// check if permissions are still granted.
Future<bool> canAccessScopes(List<String> scopes, String? accessToken) async {
if (accessToken != null && _lastTokenResponse != null) {
if (accessToken == _lastTokenResponse!.access_token) {
return oauth2.hasGrantedAllScopes(_lastTokenResponse!, scopes);
final bool isTokenValid =
_lastTokenResponseExpiration?.isAfter(DateTime.now()) ?? false;
return isTokenValid &&
oauth2.hasGrantedAllScopes(_lastTokenResponse!, scopes);
}
}
return false;
Expand All @@ -368,6 +410,8 @@ class GisSdkClient {
// The last-seen credential and token responses
CredentialResponse? _lastCredentialResponse;
TokenResponse? _lastTokenResponse;
// Expiration timestamp for the lastTokenResponse, which only has an `expires_in` field.
DateTime? _lastTokenResponseExpiration;

/// The StreamController onto which the GIS Client propagates user authentication events.
///
Expand All @@ -379,5 +423,7 @@ class GisSdkClient {
// (if needed)
//
// (This is a synthetic _lastCredentialResponse)
//
// TODO(dit): Clean this up. https://github.com/flutter/flutter/issues/137727
GoogleSignInUserData? _requestedUserData;
}
35 changes: 27 additions & 8 deletions packages/google_sign_in/google_sign_in_web/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,32 +52,51 @@ Map<String, Object?>? decodeJwtPayload(String? payload) {
return null;
}

/// Returns the payload of a [CredentialResponse].
Map<String, Object?>? getResponsePayload(CredentialResponse? response) {
if (response?.credential == null) {
return null;
}

return getJwtTokenPayload(response!.credential);
}

/// Converts a [CredentialResponse] into a [GoogleSignInUserData].
///
/// May return `null`, if the `credentialResponse` is null, or its `credential`
/// cannot be decoded.
GoogleSignInUserData? gisResponsesToUserData(
CredentialResponse? credentialResponse) {
if (credentialResponse == null || credentialResponse.credential == null) {
return null;
}

final Map<String, Object?>? payload =
getJwtTokenPayload(credentialResponse.credential);

final Map<String, Object?>? payload = getResponsePayload(credentialResponse);
if (payload == null) {
return null;
}

assert(credentialResponse?.credential != null,
'The CredentialResponse cannot be null and have a payload.');

return GoogleSignInUserData(
email: payload['email']! as String,
id: payload['sub']! as String,
displayName: payload['name'] as String?,
photoUrl: payload['picture'] as String?,
idToken: credentialResponse.credential,
idToken: credentialResponse!.credential,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is conceptually removed enough from the code that ensures that the ! is safe (it's in another method) that I worry about someone changing that code later and this becoming a crash. Couldn't it just be credentialResponse?.credential so it's safe, since it's a nullable field?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in this case payload != null implies credentialResponse != null but as you said, the connection is pretty vague.

I think what I really want here is an assertion that (credentialResponse?.credential != null) before returning the GoogleSignInUserData, that way I'm expressing in code that the credential must NOT be null before returning a GoogleSignInUserData.

);
}

/// Returns the expiration timestamp ('exp') of a [CredentialResponse].
///
/// May return `null` if the `credentialResponse` is null, its `credential`
/// cannot be decoded, or the `exp` field is not set on the JWT payload.
DateTime? getCredentialResponseExpirationTimestamp(
CredentialResponse? credentialResponse) {
final Map<String, Object?>? payload = getResponsePayload(credentialResponse);
// Get the 'exp' field from the payload, if present.
final int? exp = (payload != null) ? payload['exp'] as int? : null;
// Return 'exp' (a timestamp in seconds since Epoch) as a DateTime.
return (exp != null) ? DateTime.fromMillisecondsSinceEpoch(exp * 1000) : null;
}

/// Converts responses from the GIS library into TokenData for the plugin.
GoogleSignInTokenData gisResponsesToTokenData(
CredentialResponse? credentialResponse, TokenResponse? tokenResponse) {
Expand Down
4 changes: 2 additions & 2 deletions packages/google_sign_in/google_sign_in_web/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
for signing in with a Google account on Android, iOS and Web.
repository: https://github.com/flutter/packages/tree/main/packages/google_sign_in/google_sign_in_web
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22
version: 0.12.0+5
version: 0.12.1

environment:
sdk: ">=3.1.0 <4.0.0"
Expand All @@ -22,7 +22,7 @@ dependencies:
sdk: flutter
flutter_web_plugins:
sdk: flutter
google_identity_services_web: ^0.2.1
google_identity_services_web: ^0.2.2
google_sign_in_platform_interface: ^2.4.0
http: ">=0.13.0 <2.0.0"
js: ^0.6.3
Expand Down