Skip to content

chore(oidc): show extra_data in debug mode #3769

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.

Sign up for GitHub

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

Open
Mohamed-Hacene wants to merge 1 commit into
base: main
Choose a base branch
from
+23 −11
Open

chore(oidc): show extra_data in debug mode #3769

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 23 additions & 11 deletions backend/iam/adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ def pre_social_login(self, request, sociallogin):
extra = sociallogin.account.extra_data
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,
)
Comment on lines 92 to 96
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

# Primary lookup (legacy format)
Expand Down Expand Up @@ -135,23 +133,37 @@ def pre_social_login(self, request, sociallogin):
return Response(
{"message": "Email not provided."}, status=HTTP_401_UNAUTHORIZED
)
logger.info(
"pre_social_login: resolved email for user lookup",
email_domain=email_address.split("@")[-1]
if "@" in email_address
else "unknown",
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,
)
Comment on lines +136 to +150
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

sociallogin.user = user
sociallogin.connect(request, user)
logger.info(
"pre_social_login: social account connected",
provider=sociallogin.account.provider,
user_id=str(user.id),
)
except User.DoesNotExist:
logger.error(
"pre_social_login: user not found",
email_domain=email_address.split("@")[-1]
if "@" in email_address
else "unknown",
provider=sociallogin.account.provider,
)
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,
)
return Response(
Expand Down
Loading