Fix bug with private profiles

Author: alexiri

astra_app/core/email_context.py

@@ -72,6 +72,46 @@ def user_email_context_from_user(*, user: FreeIPAUser) -> dict[str, str]:
     }
 
 
+def user_email_delivery_context(*, username: str) -> dict[str, str]:
+    """Return user email context for operational delivery paths.
+
+    Privacy settings control profile visibility, but delivery pathways must
+    always resolve the real address when available.
+    """
+
+    normalized_username = str(username or "").strip()
+    if not normalized_username:
+        return {"username": "", "first_name": "", "last_name": "", "full_name": "", "email": ""}
+
+    try:
+        user = FreeIPAUser.get(normalized_username, respect_privacy=False)
+    except Exception as exc:
+        reason_code = str(exc.__class__.__name__ or "").strip() or "freeipa_unavailable"
+        logger.warning(
+            "Delivery email context degraded due to FreeIPA lookup failure",
+            extra={
+                "event": "astra.email.delivery_context.degraded",
+                "component": "email",
+                "outcome": "warning",
+                "reason_code": reason_code,
+                "freeipa_operation": "FreeIPAUser.get",
+                "correlation_id": "email_context.user_email_delivery_context",
+            },
+        )
+        user = None
+
+    if user is None:
+        return {
+            "username": normalized_username,
+            "first_name": "",
+            "last_name": "",
+            "full_name": normalized_username,
+            "email": "",
+        }
+
+    return user_email_context_from_user(user=user)
+
+
 def organization_email_context_from_organization(*, organization: Organization) -> dict[str, str]:
     """Return canonical organization variables for sponsor-facing templated emails.
 

astra_app/core/freeipa/user.py

@@ -349,7 +349,7 @@ def _try(label: str, fn):
         return None
 
     @classmethod
-    def get(cls, username):
+    def get(cls, username, *, respect_privacy: bool = True):
         """
         Fetch a single user by username.
         """
@@ -359,7 +359,7 @@ def get(cls, username):
         cached_data = cache.get(cache_key)
 
         if cached_data is not None:
-            return cls(username, cached_data)
+            return cls(username, cached_data, respect_privacy=respect_privacy)
 
         try:
             user_data = _with_freeipa_service_client_retry(
@@ -368,7 +368,7 @@ def get(cls, username):
             )
             if user_data is not None:
                 cache.set(cache_key, user_data)
-                return cls(username, user_data)
+                return cls(username, user_data, respect_privacy=respect_privacy)
         except Exception as e:
             logger.exception(
                 f"Failed to get user username={username}: {e}",

astra_app/core/membership_request_workflow.py

@@ -65,7 +65,7 @@ class MembershipTarget:
 def _build_membership_target(membership_request: MembershipRequest) -> MembershipTarget:
     """Resolve the notification target from a membership request."""
     if membership_request.target_kind == MembershipRequest.TargetKind.user:
-        user = FreeIPAUser.get(membership_request.requested_username)
+        user = FreeIPAUser.get(membership_request.requested_username, respect_privacy=False)
         return MembershipTarget(
             email=user.email if user is not None else "",
             email_context=user_email_context_from_user(user=user) if user is not None else {},
@@ -250,8 +250,35 @@ def _send_membership_request_notification(
     membership_type: MembershipType,
     template_name: str,
     extra_context: dict[str, object] | None = None,
+    log_context: dict[str, object] | None = None,
 ) -> object | None:
     if not target.email:
+        request_id = None
+        action = "unknown"
+        if log_context:
+            request_id = log_context.get("request_id")
+            action = str(log_context.get("action") or action)
+        warning_extra = {
+            "event": "astra.membership.email.skipped_missing_recipient",
+            "component": "membership",
+            "outcome": "warning",
+            "template_name": template_name,
+            "target_kind": "organization" if target.target_organization is not None else "user",
+            "target_username": target.target_username,
+            "target_organization_id": target.target_organization.pk if target.target_organization is not None else None,
+        }
+        if log_context:
+            warning_extra.update(log_context)
+        logger.warning(
+            "Membership email skipped because recipient email is missing request_id=%s action=%s target_kind=%s target_username=%s target_organization_id=%s template_name=%s",
+            request_id,
+            action,
+            "organization" if target.target_organization is not None else "user",
+            target.target_username,
+            target.target_organization.pk if target.target_organization is not None else None,
+            template_name,
+            extra=warning_extra,
+        )
         return None
 
     context: dict[str, object] = {
@@ -405,7 +432,7 @@ def record_membership_request_created(
 
         if send_submitted_email:
             try:
-                target = FreeIPAUser.get(membership_request.requested_username)
+                target = FreeIPAUser.get(membership_request.requested_username, respect_privacy=False)
             except Exception as e:
                 logger.exception(
                     "%s: FreeIPAUser.get failed for submitted email request_id=%s target=%r",
@@ -440,6 +467,23 @@ def record_membership_request_created(
                             extra=current_exception_log_fields(),
                         )
                         email_error = e
+                elif target is not None:
+                    logger.warning(
+                        "%s: submitted email skipped due to missing recipient address request_id=%s target=%r",
+                        log_prefix,
+                        membership_request.pk,
+                        membership_request.requested_username,
+                        extra={
+                            "event": "astra.membership.email.skipped_missing_recipient",
+                            "component": "membership",
+                            "outcome": "warning",
+                            "template_name": settings.MEMBERSHIP_REQUEST_SUBMITTED_EMAIL_TEMPLATE_NAME,
+                            "request_id": membership_request.pk,
+                            "target_kind": "user",
+                            "target_username": membership_request.requested_username,
+                            "action": "submitted",
+                        },
+                    )
 
         _try_record_email_note(
             membership_request=membership_request,
@@ -663,6 +707,23 @@ def approve_membership_request(
                 previous_expires_at=previous_expires_at,
             )
             _ensure_configured_email_template_exists(template_name=template_name)
+        elif send_approved_email:
+            logger.warning(
+                "%s: approved email skipped due to missing recipient address request_id=%s org_id=%s",
+                log_prefix,
+                membership_request.pk,
+                org.pk,
+                extra={
+                    "event": "astra.membership.email.skipped_missing_recipient",
+                    "component": "membership",
+                    "outcome": "warning",
+                    "template_name": settings.MEMBERSHIP_REQUEST_APPROVED_EMAIL_TEMPLATE_NAME,
+                    "request_id": membership_request.pk,
+                    "target_kind": "organization",
+                    "target_organization_id": org.pk,
+                    "action": "approved",
+                },
+            )
 
         old_membership = (
             Membership.objects.select_related("membership_type")
@@ -731,7 +792,7 @@ def approve_membership_request(
             )
 
         try:
-            user = FreeIPAUser.get(membership_request.requested_username)
+            user = FreeIPAUser.get(membership_request.requested_username, respect_privacy=False)
         except Exception:
             logger.exception(
                 "%s: FreeIPAUser.get failed request_id=%s target=%r",
@@ -759,6 +820,23 @@ def approve_membership_request(
                 previous_expires_at=previous_expires_at,
             )
             _ensure_configured_email_template_exists(template_name=template_name)
+        elif send_approved_email:
+            logger.warning(
+                "%s: approved email skipped due to missing recipient address request_id=%s target=%r",
+                log_prefix,
+                membership_request.pk,
+                membership_request.requested_username,
+                extra={
+                    "event": "astra.membership.email.skipped_missing_recipient",
+                    "component": "membership",
+                    "outcome": "warning",
+                    "template_name": settings.MEMBERSHIP_REQUEST_APPROVED_EMAIL_TEMPLATE_NAME,
+                    "request_id": membership_request.pk,
+                    "target_kind": "user",
+                    "target_username": membership_request.requested_username,
+                    "action": "approved",
+                },
+            )
         group_add_payload = (membership_request.requested_username, membership_type.group_cn)
 
         email_context = (
@@ -978,14 +1056,19 @@ def reject_membership_request(
 
     email_error: Exception | None = None
     sent_email = None
-    if send_rejected_email and target.email:
+    if send_rejected_email:
         try:
             sent_email = _send_membership_request_notification(
                 target=target,
                 membership_type=membership_type,
                 template_name=settings.MEMBERSHIP_REQUEST_REJECTED_EMAIL_TEMPLATE_NAME,
                 extra_context=freeform_message_email_context(key="rejection_reason", value=reason),
+                log_context={
+                    "request_id": membership_request.pk,
+                    "action": "rejected",
+                },
             )
+
         except Exception as e:
             logger.exception(
                 "%s: sending rejected email failed request_id=%s",
@@ -1182,6 +1265,10 @@ def put_membership_request_on_hold(
                     **freeform_message_email_context(key="rfi_message", value=message),
                     "application_url": application_url,
                 },
+                log_context={
+                    "request_id": membership_request.pk,
+                    "action": "rfi",
+                },
             )
         except Exception as e:
             logger.exception(
@@ -1191,6 +1278,16 @@ def put_membership_request_on_hold(
                 extra=current_exception_log_fields(),
             )
             email_error = e
+    elif send_rfi_email:
+        _send_membership_request_notification(
+            target=target,
+            membership_type=membership_type,
+            template_name=settings.MEMBERSHIP_REQUEST_RFI_EMAIL_TEMPLATE_NAME,
+            log_context={
+                "request_id": membership_request.pk,
+                "action": "rfi",
+            },
+        )
 
     log = _create_status_change_log(
         membership_request=membership_request,

astra_app/core/templates/core/_settings_tab_privacy.html

@@ -9,17 +9,6 @@ <h2 class="h4">Privacy</h2>
 		<button type="submit" class="btn btn-primary">Save privacy settings</button>
 	</form>
 
-	{% if privacy_warnings %}
-		<div class="alert alert-warning" role="alert">
-			<strong>Manual review required.</strong>
-			<ul class="mb-0 mt-2">
-				{% for warning in privacy_warnings %}
-					<li>{{ warning }}</li>
-				{% endfor %}
-			</ul>
-		</div>
-	{% endif %}
-
 	<div class="card border-danger">
 		<div class="card-body">
 			<h3 class="h5">Delete my account</h3>
@@ -45,6 +34,17 @@ <h3 class="h5">Delete my account</h3>
 					<button type="submit" class="btn btn-danger">Submit deletion request</button>
 				</form>
 			{% endif %}
+
+			{% if privacy_warnings %}
+				<div class="alert alert-warning mt-3" role="alert">
+					<strong>Manual review required.</strong>
+					<ul class="mb-0 mt-2">
+						{% for warning in privacy_warnings %}
+							<li>{{ warning }}</li>
+						{% endfor %}
+					</ul>
+				</div>
+			{% endif %}
 		</div>
 	</div>
 </div>

astra_app/core/templates/core/user_profile.html

@@ -79,8 +79,8 @@
 
             <h3 class="profile-username mb-1">{{ fu.get_full_name }}</h3>
             <div class="text-muted" id="user_username">{{ fu.username }}</div>
-            {% if fu.email %}
-              <div class="mt-1" id="user_mail"><a href="mailto:{{ fu.email }}">{{ fu.email }}</a></div>
+            {% if profile_email %}
+              <div class="mt-1" id="user_mail"><a href="mailto:{{ profile_email }}">{{ profile_email }}</a></div>
             {% endif %}
 
             {% if is_self %}

astra_app/core/tests/test_fasisprivate_anonymize.py

@@ -222,6 +222,55 @@ def test_user_profile_shows_membership_card_for_private_profile_committee_viewer
         html = resp.content.decode("utf-8")
         self.assertIn("Membership</strong>", html)
 
+    def test_user_profile_shows_email_for_private_profile_committee_viewer(self) -> None:
+        factory = RequestFactory()
+        request = factory.get("/user/bob/")
+        request.user = SimpleNamespace(
+            is_authenticated=True,
+            get_username=lambda: "reviewer",
+            groups_list=[settings.FREEIPA_MEMBERSHIP_COMMITTEE_GROUP],
+        )
+
+        set_current_viewer_username("reviewer")
+        try:
+            private_profile = FreeIPAUser(
+                "bob",
+                {
+                    "uid": ["bob"],
+                    "givenname": ["Bob"],
+                    "sn": ["User"],
+                    "mail": ["bob@example.org"],
+                    "fasIsPrivate": ["TRUE"],
+                    "memberof_group": ["packagers"],
+                },
+            )
+        finally:
+            clear_current_viewer_username()
+
+        full_profile = FreeIPAUser(
+            "bob",
+            {
+                "uid": ["bob"],
+                "givenname": ["Bob"],
+                "sn": ["User"],
+                "mail": ["bob@example.org"],
+                "fasIsPrivate": ["TRUE"],
+                "memberof_group": ["packagers"],
+            },
+        )
+
+        with (
+            patch("core.views_users._get_full_user", autospec=True, return_value=private_profile),
+            patch("core.views_users.FreeIPAUser.get", autospec=True, return_value=full_profile),
+            patch("core.views_users.FreeIPAGroup.all", autospec=True, return_value=[]),
+            patch("core.views_users.has_enabled_agreements", autospec=True, return_value=False),
+        ):
+            resp = views_users.user_profile(request, "bob")
+
+        self.assertEqual(resp.status_code, 200)
+        html = resp.content.decode("utf-8")
+        self.assertIn("bob@example.org", html)
+
     def test_user_profile_shows_membership_card_for_private_profile_committee_viewer_without_groups_list(self) -> None:
         factory = RequestFactory()
         request = factory.get("/user/bob/")
@@ -256,7 +305,11 @@ def test_user_profile_shows_membership_card_for_private_profile_committee_viewer
 
         with (
             patch("core.views_users._get_full_user", autospec=True, return_value=bob),
-            patch("core.views_users.FreeIPAUser.get", autospec=True, side_effect=lambda username: reviewer if username == "reviewer" else None),
+            patch(
+                "core.views_users.FreeIPAUser.get",
+                autospec=True,
+                side_effect=lambda username, **_kwargs: reviewer if username == "reviewer" else None,
+            ),
             patch("core.views_users.FreeIPAGroup.all", autospec=True, return_value=[]),
             patch("core.views_users.has_enabled_agreements", autospec=True, return_value=False),
         ):