chore(oidc): show extra_data in debug mode#3769
Open
Mohamed-Hacene wants to merge 1 commit intomainfrom
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR updates logging in the social account authentication adapter, replacing structured logging of derived fields with raw IdP email data and adding more granular debug logs for database lookup results and account connection outcomes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/iam/adapter.py`:
- Around line 92-96: The logger.debug in pre_social_login is currently passing
the entire IdP payload via extra_data=extra; change it to avoid dumping full
provider responses by logging only the keys or a whitelist of safe fields
instead (e.g., use something like extra_keys=list(extra.keys()) or extract and
log only specific safe attributes), and update the logger.debug call in
pre_social_login / the adapter to include that keys-only or whitelisted summary
rather than the full extra dict.
- Around line 136-150: The logger.debug calls in pre_social_login are emitting
raw PII (idp_email, idp_email_repr, db_email); update the tracing in the
pre_social_login flow so it no longer logs raw email strings—modify the
logger.debug calls around User.objects.get and the subsequent match block to log
non-PII derived diagnostics such as normalized_email_hash or truncated hash,
domain (after normalization), case/whitespace flags, and user_id/is_active,
while removing idp_email, idp_email_repr and db_email fields; ensure the same
change is applied to the other debug block around lines 163-167 so no raw email
values are written to logs.
Comment on lines
92
to
96
| logger.debug( | ||
| "pre_social_login: extra_data received", | ||
| extra_data_keys=list(extra.keys()), | ||
| has_userinfo="userinfo" in extra, | ||
| has_id_token="id_token" in extra, | ||
| extra_data=extra, | ||
| provider=sociallogin.account.provider, | ||
| ) |
Contributor
There was a problem hiding this comment.
Do not dump the full IdP payload to logs.
extra_data=extra will persist the full provider response whenever debug logging is enabled. This project has no log-redaction layer, and the failure path just below already shows the safer pattern by logging keys only.
🛡️ Suggested change
logger.debug(
"pre_social_login: extra_data received",
- extra_data=extra,
provider=sociallogin.account.provider,
+ extra_data_keys=list(extra.keys()),
+ userinfo_keys=list(extra.get("userinfo", {}).keys()),
+ id_token_keys=list(extra.get("id_token", {}).keys()),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.debug( | |
| "pre_social_login: extra_data received", | |
| extra_data_keys=list(extra.keys()), | |
| has_userinfo="userinfo" in extra, | |
| has_id_token="id_token" in extra, | |
| extra_data=extra, | |
| provider=sociallogin.account.provider, | |
| ) | |
| logger.debug( | |
| "pre_social_login: extra_data received", | |
| provider=sociallogin.account.provider, | |
| extra_data_keys=list(extra.keys()), | |
| userinfo_keys=list(extra.get("userinfo", {}).keys()), | |
| id_token_keys=list(extra.get("id_token", {}).keys()), | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/iam/adapter.py` around lines 92 - 96, The logger.debug in
pre_social_login is currently passing the entire IdP payload via
extra_data=extra; change it to avoid dumping full provider responses by logging
only the keys or a whitelist of safe fields instead (e.g., use something like
extra_keys=list(extra.keys()) or extract and log only specific safe attributes),
and update the logger.debug call in pre_social_login / the adapter to include
that keys-only or whitelisted summary rather than the full extra dict.
Comment on lines
+136
to
+150
| logger.debug( | ||
| "pre_social_login: resolved email from IdP", | ||
| idp_email=email_address, | ||
| idp_email_repr=repr(email_address), | ||
| provider=sociallogin.account.provider, | ||
| ) | ||
| try: | ||
| user = User.objects.get(email__iexact=email_address) | ||
| logger.debug( | ||
| "pre_social_login: user matched", | ||
| idp_email=email_address, | ||
| db_email=user.email, | ||
| user_id=str(user.id), | ||
| is_active=user.is_active, | ||
| ) |
Contributor
There was a problem hiding this comment.
Remove raw email values from these lookup traces.
idp_email, idp_email_repr, and db_email add long-lived PII to both the success and failure paths. If the goal is diagnosing normalization issues, derived fields like domain/whitespace/case flags are enough and avoid storing the actual address.
🛡️ Suggested change
+ normalized_email = email_address.strip()
+ email_domain = (
+ normalized_email.partition("@")[2].lower() if "@" in normalized_email else None
+ )
logger.debug(
"pre_social_login: resolved email from IdP",
- idp_email=email_address,
- idp_email_repr=repr(email_address),
provider=sociallogin.account.provider,
+ email_domain=email_domain,
+ had_surrounding_whitespace=normalized_email != email_address,
+ had_uppercase=normalized_email != normalized_email.casefold(),
)
@@
logger.debug(
"pre_social_login: user matched",
- idp_email=email_address,
- db_email=user.email,
user_id=str(user.id),
is_active=user.is_active,
+ email_domain=email_domain,
)
@@
logger.debug(
"pre_social_login: user not found - check DB for this email",
- idp_email=email_address,
- idp_email_repr=repr(email_address),
provider=sociallogin.account.provider,
+ email_domain=email_domain,
+ had_surrounding_whitespace=normalized_email != email_address,
+ had_uppercase=normalized_email != normalized_email.casefold(),
)Also applies to: 163-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/iam/adapter.py` around lines 136 - 150, The logger.debug calls in
pre_social_login are emitting raw PII (idp_email, idp_email_repr, db_email);
update the tracing in the pre_social_login flow so it no longer logs raw email
strings—modify the logger.debug calls around User.objects.get and the subsequent
match block to log non-PII derived diagnostics such as normalized_email_hash or
truncated hash, domain (after normalization), case/whitespace flags, and
user_id/is_active, while removing idp_email, idp_email_repr and db_email fields;
ensure the same change is applied to the other debug block around lines 163-167
so no raw email values are written to logs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes