-
Notifications
You must be signed in to change notification settings - Fork 122
feat(dart): Add Google Sign-In template #343
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new Dart package sign_in_with_google with project metadata and tooling files (.gitignore, analysis_options.yaml, pubspec.yaml, README.md) and a runtime at lib/main.dart. The runtime accepts a Google ID token, fetches Google JWKS, verifies JWT signature and claims (kid, audience against GOOGLE_CLIENT_ID, issuer, expiry), extracts user info (sub, email, email_verified, name), and uses Appwrite APIs to find or create a user, update email verification, generate an Appwrite access token, and return userId and token. Error handling covers missing/invalid tokens, key fetch/selection, verification failures, and Appwrite interactions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Files/areas to focus review on:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
dart/sign_in_with_google/lib/main.dart (1)
17-25: Consider error handling for JSON parsing.The code assumes
context.req.bodyJsonis a validMap<String, dynamic>. If the request body is malformed or not JSON (despite Content-Type header), the cast will fail. Consider wrapping in a try-catch or validating the type.- final reqBody = context.req.bodyJson as Map<String, dynamic>; + final reqBody = context.req.bodyJson; + if (reqBody is! Map<String, dynamic>) { + throw Exception('Request body must be a valid JSON object.'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dart/sign_in_with_google/.gitignore(1 hunks)dart/sign_in_with_google/README.md(1 hunks)dart/sign_in_with_google/analysis_options.yaml(1 hunks)dart/sign_in_with_google/lib/main.dart(1 hunks)dart/sign_in_with_google/pubspec.yaml(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.29.0)
dart/sign_in_with_google/README.md
[high] 33-33: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 49-49: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (19)
dart/sign_in_with_google/.gitignore (1)
1-3: LGTM!Standard Dart ignore patterns are appropriate for this project.
dart/sign_in_with_google/analysis_options.yaml (1)
1-1: LGTM!Standard linting configuration using the recommended lint rules.
dart/sign_in_with_google/pubspec.yaml (2)
11-12: LGTM!The lints package is appropriately included as a dev dependency.
4-5: The README and pubspec.yaml SDK constraint are misaligned.The claim is confirmed: the README specifies "Dart (3.5)" but
pubspec.yamlconstrains SDK to^2.17.0. However, this pattern is inconsistent across templates—some match (e.g., censor_with_redact claims 2.17 and uses^2.17.0), while others don't (whatsapp_with_vonage claims 3.1 but uses^2.17.0). All Dart templates consistently use^2.17.0in pubspec, suggesting a repository-wide policy. Verify with maintainers whether README versions should be updated to reflect the actual SDK constraint, or if SDK constraints should be tightened per template runtime.dart/sign_in_with_google/README.md (3)
1-12: LGTM!The introduction clearly describes the function's purpose and provides a helpful reference to Google's official documentation.
76-104: LGTM!The client-side integration example is clear and demonstrates the correct flow for obtaining and sending the Google ID token.
106-113: LGTM!The security notes accurately describe the verification steps implemented in the code.
dart/sign_in_with_google/lib/main.dart (12)
1-8: LGTM!The imports are appropriate for the Google Sign-In functionality.
10-15: LGTM!Environment variable validation is properly implemented with clear error messages.
27-36: LGTM!Token verification follows Google's official tokeninfo endpoint pattern correctly. The implementation properly checks the response status and handles errors.
38-44: LGTM!Audience validation correctly prevents token reuse across different applications.
46-50: LGTM!Issuer validation correctly handles both valid Google issuer formats.
52-57: LGTM!Expiration validation correctly converts timestamps and checks token validity.
59-68: LGTM!User data extraction correctly handles Google's tokeninfo response format, including the string-based
email_verifiedfield.
70-76: Verify API key handling for production use.The code defaults to an empty string if
x-appwrite-keyis missing from request headers (line 75). Ensure this aligns with your security model. If the function requires an API key for user management, consider validating its presence explicitly.
78-86: LGTM!User lookup by ID correctly handles the not-found case while propagating other errors.
88-94: LGTM!The fallback to email-based lookup is well-implemented and handles the case where a user might exist with the same email but different ID.
96-101: LGTM!User creation logic correctly uses the Google subject as a custom ID and handles optional fields appropriately.
108-119: LGTM!Token creation and response formatting are correct. The 60-second expiration is appropriate for a session exchange token.
…ion note is clear
Co-authored-by: Steven Nguyen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
dart/sign_in_with_google/README.md (2)
16-16: Use proper markdown headings for better document structure.The bolded text on lines 16, 22, and 36 should use proper heading syntax to improve navigation and semantic structure.
-**Headers** +### Headers-**Request** +### Request-**Response** +### ResponseAlso applies to: 22-22, 36-36
42-42: Use hyphenated compound adjective."ISO formatted" should be "ISO-formatted" when used as a compound adjective.
-* `expire` - ISO formatted timestamp for when the secret expires +* `expire` - ISO-formatted timestamp for when the secret expiresdart/sign_in_with_google/lib/main.dart (2)
20-21: Add error handling for request body parsing.The cast to
Map<String, dynamic>on line 20 will throw if the request body is not a valid JSON object. Consider wrapping this in a try-catch to provide a clearer error message to the caller.- final reqBody = context.req.bodyJson as Map<String, dynamic>; - final idToken = reqBody['idToken'] ?? ''; + Map<String, dynamic> reqBody; + try { + reqBody = context.req.bodyJson as Map<String, dynamic>; + } catch (e) { + throw Exception('Invalid request body: expected JSON object.'); + } + final idToken = reqBody['idToken'] ?? '';
109-109: Consider validating the API key is present.The API key is retrieved from the
x-appwrite-keyheader with an empty string fallback. If the header is missing, subsequent Appwrite API calls will fail with authentication errors. Consider adding validation to provide a clearer error message.- .setKey(context.req.headers['x-appwrite-key'] ?? ''); + .setKey(context.req.headers['x-appwrite-key'] ?? + throw Exception('Missing x-appwrite-key header'));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dart/sign_in_with_google/README.md(1 hunks)dart/sign_in_with_google/lib/main.dart(1 hunks)dart/sign_in_with_google/pubspec.yaml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
dart/sign_in_with_google/README.md
[grammar] ~42-~42: Use a hyphen to join words.
Context: ...()to create a session *expire` - ISO formatted timestamp for when the secret ...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
dart/sign_in_with_google/README.md
16-16: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
22-22: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
36-36: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (11)
dart/sign_in_with_google/lib/main.dart (11)
11-16: Environment validation logic is correct.The validation properly checks that
GOOGLE_CLIENT_IDis set and non-empty using null-aware operators.
40-53: JWT parsing and key ID extraction looks good.The implementation correctly splits the JWT, validates the format, decodes the header, and extracts the key ID with proper error handling.
55-59: Key matching logic is correct.Properly uses
firstWherewithorElseto find the matching key bykidand provides a clear error when no match is found.
61-70: JWT signature verification is properly implemented.The signature verification using the
josepackage correctly verifies the JWT against Google's public key, addressing the security requirement from the documentation.
72-91: Token claims validation is thorough and correct.The implementation properly validates the three critical JWT claims:
- Audience matches the configured client ID
- Issuer is Google
- Token has not expired
93-102: User information extraction is safe and correct.Properly extracts user data from the verified JWT claims with appropriate null-safe fallbacks.
112-120: User lookup by ID with proper error handling.Correctly handles the
user_not_foundcase while re-throwing other exceptions. This is the right pattern for distinguishing between expected and unexpected errors.
122-128: Email-based user lookup is implemented correctly.The fallback to email lookup when user ID is not found follows a logical flow and uses the appropriate Appwrite query.
130-135: User creation logic is well-implemented.Uses the null-coalescing operator idiomatically and correctly handles optional fields (email, name) by passing null when empty.
152-156: Response format matches documentation.The returned JSON structure correctly matches the response schema documented in the README with the three required fields: secret, userId, and expire.
146-150: Token expiry duration is appropriate.The
expireparameter accepts duration in seconds. The value60creates a token that expires in 60 seconds, which is a standard and appropriate duration for a temporary session exchange token used in authentication flows. The SDK returns this as an ISO-8601 timestamp in the response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
dart/sign_in_with_google/lib/main.dart (2)
164-176: Consider making token parameters configurable.The token expiry (60 minutes) and length (128) are hardcoded. While these are reasonable defaults, consider making them configurable via environment variables to allow flexibility for different deployment scenarios.
Example implementation:
final tokenExpiry = int.tryParse( Platform.environment['TOKEN_EXPIRE_MINUTES'] ?? '60' ) ?? 60; final tokenLength = int.tryParse( Platform.environment['TOKEN_LENGTH'] ?? '128' ) ?? 128; final token = await users.createToken( userId: user.$id, expire: tokenExpiry, length: tokenLength, );
20-26: Consider adding input length validation.While invalid JWTs will fail verification, there's no upper bound on
idTokenlength. A reasonable limit (e.g., 2048 characters for a JWT) could prevent potential memory issues from malicious inputs.Example:
final idToken = reqBody['idToken'] ?? ''; if (idToken.isEmpty) { throw Exception('idToken must be provided in the request body.'); } if (idToken.length > 2048) { throw Exception('idToken exceeds maximum allowed length.'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dart/sign_in_with_google/lib/main.dart(1 hunks)dart/sign_in_with_google/pubspec.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dart/sign_in_with_google/pubspec.yaml
🔇 Additional comments (8)
dart/sign_in_with_google/lib/main.dart (8)
28-55: LGTM!The HTTP request now includes a proper timeout and comprehensive error handling. The 5-second timeout is reasonable, and the code correctly handles both timeout exceptions and non-200 status codes.
57-78: LGTM!The JWT header parsing and key selection logic is well-implemented with proper validation at each step, including format validation, kid extraction, and key matching with appropriate error messages.
80-89: LGTM! Signature verification properly implemented.The JWT signature verification using Google's public keys is now correctly implemented with the
joselibrary, addressing the previous review concern. The error handling appropriately catches verification failures.
91-110: LGTM!The claims validation correctly implements all required checks per Google's ID token verification guidelines: audience matching, issuer validation (both Google URL formats), and expiry validation.
112-122: LGTM!User data extraction is implemented safely with proper validation for the required
subjectclaim and appropriate handling of optional fields like email and name.
131-147: LGTM!The user lookup logic correctly implements a two-stage fallback (by ID, then by email) with proper exception handling that only ignores
user_not_founderrors while re-throwing others.
149-154: LGTM!User creation correctly uses
ID.custom(userId)to preserve Google's subject claim as the Appwrite user ID, and properly handles optional email and name fields by passingnullfor empty values.
156-162: LGTM! Correctly usesuser.$id.The email verification update correctly uses
user.$idinstead of the GoogleuserId, addressing the previous review concern. The conditional logic appropriately only updates when Google reports the email as verified and Appwrite doesn't already have it marked as verified.
…gration Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
dart/sign_in_with_google/lib/main.dart (3)
161-167: Consider making email verification update non-fatal.If the email verification update fails (e.g., due to permissions or network issues), the entire authentication flow fails and the user doesn't receive their session token, even though they successfully authenticated with Google.
For better resilience, you might want to wrap this in a try-catch block to log the error and continue with token generation, as the core authentication has already succeeded.
Apply this diff if you want to make it non-fatal:
// Mark the user as verified if the email is verified by Google and not already verified - if (emailVerified && !user.emailVerification) { - await users.updateEmailVerification( - userId: user.$id, - emailVerification: true, - ); + if (emailVerified && !user.emailVerification) { + try { + await users.updateEmailVerification( + userId: user.$id, + emailVerification: true, + ); + } catch (e) { + context.log('Failed to update email verification: $e'); + // Continue with token generation despite verification update failure + } }
169-180: Consider making token parameters configurable.The token expiration (60) and length (128) are hardcoded. For production deployments, these could be made configurable via environment variables to allow flexibility without code changes.
Example configuration:
final tokenExpire = int.tryParse( Platform.environment['TOKEN_EXPIRE'] ?? '60' ) ?? 60; final tokenLength = int.tryParse( Platform.environment['TOKEN_LENGTH'] ?? '128' ) ?? 128; final token = await users.createToken( userId: user.$id, expire: tokenExpire, length: tokenLength, );
32-61: Consider caching Google's JWKS to reduce per-request latency.The function fetches Google's public keys on every request. Google recommends examining the Cache-Control header in the response to determine when you should retrieve keys again, rather than fetching on each invocation.
Since this runs as a Cloud Function called per-request, caching would:
- Reduce latency by eliminating unnecessary HTTP calls
- Lower the risk of rate limiting from repeated requests
- Improve reliability by reducing external service dependency
Consider implementing a cache that respects the
max-agedirective from the response headers. Note: In serverless environments, you may leverage global/static variables or an external cache service depending on your runtime's invocation lifecycle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dart/sign_in_with_google/lib/main.dart(1 hunks)
🔇 Additional comments (9)
dart/sign_in_with_google/lib/main.dart (9)
10-20: LGTM! Comprehensive environment variable validation.All required environment variables are validated upfront with clear error messages, preventing obscure failures later in the function execution.
24-30: LGTM! Input validation is correct.The idToken parameter is properly extracted and validated before use.
63-76: LGTM! JWT header parsing and kid extraction are correct.The JWT format is validated, the header is properly decoded, and the key ID is extracted with appropriate error handling.
78-93: LGTM! JWT signature verification is implemented correctly.The code properly selects the matching key by kid and verifies the JWT signature using the jose package, with appropriate error handling for verification failures.
95-115: LGTM! JWT claims validation is thorough and secure.All critical claims are properly validated:
- Audience matches the expected Google Client ID
- Issuer is verified to be Google (handles both issuer formats)
- Expiration is checked to ensure token hasn't expired
This provides strong security guarantees for the authentication flow.
117-126: LGTM! User information extraction is correct.The subject (user ID) is properly validated, and optional fields like email and name are extracted with safe defaults.
128-134: LGTM! Appwrite client setup follows established patterns.The client configuration correctly uses validated environment variables and follows the same x-appwrite-key header handling pattern as other templates in this repository.
136-152: LGTM! User lookup strategy is well-designed.The two-tier lookup (first by Google user ID, then by email) provides good flexibility for finding existing users while properly handling the user_not_found case.
154-159: LGTM! User creation logic is appropriate.The user is created with the Google user ID as a custom Appwrite ID, with email and name included when available. This pattern is appropriate for OAuth-based user provisioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
dart/sign_in_with_google/lib/main.dart (3)
82-123: Add explicit error handling for certificate response parsing.Line 96 assumes the response is a valid map of strings. If Google returns an unexpected format or the JSON is malformed, the cast could fail with an unclear error.
Apply this diff to add explicit error handling:
- final certificates = Map<String, String>.from(json.decode(response.body)); + final dynamic decoded = json.decode(response.body); + if (decoded is! Map) { + throw GoogleOAuthException( + 'Invalid certificate response format', + details: 'Expected JSON object', + statusCode: 502, + ); + } + final certificates = Map<String, String>.from(decoded);
125-229: Consider validating that email is present.The implementation allows empty email values (lines 206, 245, 262), which could lead to users without email addresses. Google ID tokens should always include an email claim when the email scope is granted.
Consider adding validation to ensure email is present:
final userId = payload['sub']?.toString() ?? ''; if (userId.isEmpty) { throw GoogleOAuthException('Token missing user ID (sub)'); } + + final email = payload['email']?.toString() ?? ''; + if (email.isEmpty) { + throw GoogleOAuthException('Token missing email claim'); + } return GoogleUserInfo( userId: userId, - email: payload['email']?.toString() ?? '', + email: email, emailVerified: payload['email_verified'] == true, name: payload['name']?.toString() ?? '', );Alternatively, if supporting email-less sign-in is intentional, consider documenting this behavior in the README.
245-254: Broad exception handling may hide real errors.Line 251 catches all
AppwriteExceptioninstances when searching by email, which could mask legitimate errors (e.g., network failures, permission issues) rather than just "user not found" cases.Apply this diff to handle only expected exceptions:
if (email.isNotEmpty) { try { final userList = await _users.list( queries: [Query.equal('email', email)], ); return userList.users.isNotEmpty ? userList.users.first : null; - } on AppwriteException catch (_) { + } on AppwriteException catch (e) { + if (e.type == 'general_query_invalid' || e.type == 'general_query_limit_exceeded') { + return null; + } + rethrow; - return null; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dart/sign_in_with_google/lib/main.dart(1 hunks)dart/sign_in_with_google/pubspec.yaml(1 hunks)
🔇 Additional comments (7)
dart/sign_in_with_google/lib/main.dart (6)
10-20: LGTM!The configuration constants are well-organized and use appropriate defaults. The PEM certificate endpoint (
v1/certs) is correctly chosen for use withdart_jsonwebtoken.
22-40: LGTM!The certificate cache implementation correctly manages expiry and invalidation. The design is appropriate for the use case.
42-51: LGTM!The custom exception class provides appropriate context with message, details, and status code for proper error handling and debugging.
58-80: LGTM!The cache-control parsing correctly extracts
max-ageand applies sensible bounds (1 hour to 7 days) with proper fallback handling for malformed headers.
285-369: LGTM!The data classes are well-structured, and
EnvironmentConfigproperly validates all required environment variables with clear error messages.
371-453: LGTM!The authentication handler and main function are well-structured with comprehensive error handling, appropriate logging, and secure error responses that don't leak sensitive information.
dart/sign_in_with_google/pubspec.yaml (1)
8-10: All dependency versions are current and valid.Verification confirms that
dart_appwrite ^19.4.0,http ^1.6.0, anddart_jsonwebtoken ^3.3.1are already at their latest stable versions on pub.dev. No updates needed.
Description
This PR adds a new Dart template for Google Sign-In authentication with Appwrite Functions. The template verifies Google ID tokens and creates or updates Appwrite users accordingly.
What's Added
dart/sign_in_with_googleFeatures
✅ Security Validations:
✅ User Management:
Files Added
dart/sign_in_with_google/lib/main.dart- Main function implementationdart/sign_in_with_google/pubspec.yaml- Dart dependenciesdart/sign_in_with_google/README.md- Comprehensive documentation with usage examplesdart/sign_in_with_google/analysis_options.yaml- Linting configurationdart/sign_in_with_google/.gitignore- Git ignore rulesDocumentation
The README includes:
google_sign_inpackageTesting
The template follows the same pattern as the existing
sign_in_with_appletemplate and uses:dart_appwriteSDK for Appwrite integrationhttppackage for Google API communicationReferences
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.