Repository navigation
change(identity): add auth_type and resource to AuthBlockingEvent #317
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
Changes from all commits
dbaa9cf
607af7c
1609bf8
807c9fe
dd56968
f8293a4
abecffe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| # pylint: disable=protected-access | ||
| import datetime as _dt | ||
| import json as _json | ||
| import os as _os | ||
| import re as _re | ||
| import time as _time | ||
| import typing as _typing | ||
|
|
@@ -229,8 +230,17 @@ def _credential_from_token_data(token_data: dict[str, _typing.Any], time: float) | |
| ) | ||
|
|
||
|
|
||
| def _auth_blocking_event_from_token_data(event_type: str, token_data: dict[str, _typing.Any]): | ||
| from firebase_functions.identity_fn import AuthBlockingEvent | ||
| def _auth_blocking_event_from_token_data( | ||
| event_type: str, | ||
| token_data: dict[str, _typing.Any], | ||
| project_id: str | None, | ||
| ): | ||
| from firebase_functions.identity_fn import AuthBlockingEvent, AuthBlockingResource | ||
|
|
||
| tenant_id = token_data.get("tenant_id") | ||
| resource_name = ( | ||
| f"projects/{project_id}/tenants/{tenant_id}" if tenant_id else f"projects/{project_id}" | ||
| ) | ||
|
|
||
| return AuthBlockingEvent( | ||
| data=_auth_user_record_from_token_data(token_data["user_record"]), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [Logic & Guarding] Validate Rationale: auth_type="USER" if token_data.get("user_record") else "UNAUTHENTICATED",However, line 243 unconditionally accesses data=_auth_user_record_from_token_data(token_data["user_record"]),If an unauthenticated payload arrives without a user record, line 243 raises Since user_record_data = token_data.get("user_record")
if not isinstance(user_record_data, dict) or not user_record_data:
raise ValueError(f"Auth blocking event '{event_type}' requires a valid 'user_record'.")
return AuthBlockingEvent(
data=_auth_user_record_from_token_data(user_record_data),
...
auth_type="USER",
resource=AuthBlockingResource(
service="identitytoolkit.googleapis.com",
name=resource_name,
),
)This keeps
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding this check would move away from what Node does. I deployed this branch as before_user_created and before_user_signed_in functions, and every token from email sign-up and password sign-in included user_record. If it were ever missing, the handler would log Unhandled error 'user_record' and return a 500, as Node does for these triggers. AuthType already has both "USER" and "UNAUTHENTICATED", and auth_type is set the same way as in Node, so the email and SMS triggers will only need data to become optional. Would you still prefer to add this validation, or should we keep it as is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was conflicted myself on this, I'm fine keeping it as is and improving in the future if needed as we add email and SMS triggers. |
||
|
|
@@ -244,6 +254,11 @@ def _auth_blocking_event_from_token_data(event_type: str, token_data: dict[str, | |
| credential=_credential_from_token_data(token_data, _time.time()), | ||
| email_type=token_data.get("email_type"), | ||
| sms_type=token_data.get("sms_type"), | ||
| auth_type="USER" if token_data.get("user_record") else "UNAUTHENTICATED", | ||
|
IzaakGough marked this conversation as resolved.
Resolved
|
||
| resource=AuthBlockingResource( | ||
| service="identitytoolkit.googleapis.com", | ||
| name=resource_name, | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -363,7 +378,9 @@ def before_operation_handler( | |
| raise HttpsError(FunctionsErrorCode.INVALID_ARGUMENT, "Bad Request") | ||
| jwt_token = request.json["data"]["jwt"] | ||
| decoded_token = _token_verifier.verify_auth_blocking_token(jwt_token) | ||
| event = _auth_blocking_event_from_token_data(event_type, decoded_token) | ||
| event = _auth_blocking_event_from_token_data( | ||
| event_type, decoded_token, _os.environ.get("GCLOUD_PROJECT") | ||
| ) | ||
| auth_response: BeforeCreateResponse | BeforeSignInResponse | None = _with_init(func)(event) | ||
| if not auth_response: | ||
| return _jsonify({}) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.