Skip to content

feat(auth): Sync user profile pictures from SAML on login#118088

Open
Asynchronite wants to merge 5 commits into
getsentry:masterfrom
Asynchronite:feat/scim-profile-picture-attribute
Open

feat(auth): Sync user profile pictures from SAML on login#118088
Asynchronite wants to merge 5 commits into
getsentry:masterfrom
Asynchronite:feat/scim-profile-picture-attribute

Conversation

@Asynchronite

@Asynchronite Asynchronite commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds optional syncing of a user's profile picture from their SAML identity
provider during SSO login. When an organization's SAML provider maps the new
avatar attribute to a SAML assertion attribute carrying base64 image data,
that image becomes the user's avatar on each login.

This is opt-in and provider-scoped: organizations whose SAML attribute
mapping does not include avatar are completely unaffected, and non-SAML SSO
providers (Google, GitHub, etc.) are never touched.

What changed

  • SAML2Provider.build_identity now extracts an avatar attribute (when
    mapped) and attaches the validated image to the identity as
    identity["avatar"].
  • _validate_saml_avatar validates the assertion value: base64-decodes it
    (accepting an optional data:<mimetype>;base64, prefix), and checks the
    format (PNG/JPEG/GIF) and size (SENTRY_MAX_AVATAR_SIZE). It does not
    enforce dimensions, since IdPs send varying sizes and stored avatars are
    resized on read. It never raises — a malformed picture is dropped so it
    can't block login.
  • AuthIdentityHandler._apply_sso_avatar applies the picture on login,
    wired into all SAML login paths: handle_existing_identity (repeat logins),
    handle_attach_identity (linking), and handle_new_user (first-time). It is
    gated on provider.is_saml, and any failure is logged and swallowed so SSO
    login always completes.
  • user_service.update_user_avatar (new RPC) stores the decoded image as an
    uploaded UserAvatar, or resets the user to the default letter avatar when
    given an empty value. The avatar lives in the control silo alongside the auth
    pipeline, so no cross-silo image transfer is needed.

The base64 image is used transiently to set the avatar and is not persisted
to the AuthIdentity row.

How to enable

Map the avatar attribute to the IdP attribute carrying the user's base64 photo
in the SAML provider's attribute mapping, e.g.:

{
    Attributes.IDENTIFIER: "id",
    Attributes.USER_EMAIL: "email",
    Attributes.FIRST_NAME: "first",
    Attributes.LAST_NAME: "last",
    Attributes.AVATAR: "photo",  # IdP attribute holding base64 image data
}

Testing

  • _validate_saml_avatar: valid PNG, data: URI stripping, non-image, invalid base64, empty/missing.
  • build_identity: avatar present when mapped + valid, omitted when invalid, omitted when unmapped.
  • user_service.update_user_avatar: upload + reset-to-letter-avatar.
  • Login application: applied for SAML, skipped for non-SAML providers, no-op when no avatar is present, and failures swallowed (login not broken).

Notes / follow-ups

  • handle_existing_identity runs on every SAML login, so if the IdP sends the photo on each assertion the avatar is re-decoded and re-stored every login (new file blob, old one deleted). Correct but wasteful at scale; a follow-up could hash the incoming image and skip re-upload when unchanged.
  • No UI is added to the generic SAML setup form for the avatar mapping; it is configured via the provider's attribute mapping. A setup-form field could be added later (frontend-only change).

Closing issues

Closes #104496.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Add optional syncing of a user's profile picture from their SAML identity
provider during SSO login.

When an organization's SAML provider maps the new `avatar` attribute (via
its attribute mapping) to a SAML assertion attribute carrying base64 image
data, that image is stored as the user's avatar each time they log in. This
is opt-in: providers that don't map the `avatar` attribute are unaffected.

SAML2Provider.build_identity extracts and validates the photo. Validation
(_validate_saml_avatar) base64-decodes the value, accepts an optional
`data:<mimetype>;base64,` prefix, and checks the format (PNG/JPEG/GIF) and
size (SENTRY_MAX_AVATAR_SIZE). It deliberately does not enforce dimensions,
since IdPs send varying sizes and stored avatars are resized on read. It
never raises -- a malformed picture is dropped so it can't block login.

The validated image is carried on the identity as `identity["avatar"]` and
applied by AuthIdentityHandler._apply_sso_avatar across all SAML login
paths (existing-identity, attach-identity, and new-user). Application is
gated on the provider being SAML, and any failure is logged and swallowed
so SSO login always completes. The image is not persisted to the
AuthIdentity row.

Storage goes through a new user_service.update_user_avatar RPC, which
decodes the base64 image and saves it as an uploaded UserAvatar (or resets
to the default letter avatar when given an empty value). The avatar lives
in the control silo alongside the auth pipeline, so no cross-silo image
transfer is required.

Tests cover avatar validation, build_identity extraction (mapped, invalid,
and unmapped cases), the RPC (upload + reset), and login application
(applied for SAML, skipped for non-SAML, no-op without an avatar, and
failures swallowed).
@Asynchronite Asynchronite requested review from a team as code owners June 19, 2026 12:47
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 19, 2026
Comment thread tests/sentry/auth/test_helper.py Outdated
A previous edit that appended ApplySSOAvatarTest accidentally moved the
tail of test_inactive_identity_authenticated_request_shows_confirmation
onto the end of test_avatar_failure_does_not_break_login. This:

- broke test_avatar_failure_does_not_break_login with a NameError
  (auth_identity/inactive_user/attacker are undefined there), failing the
  backend test suite and the F821/mypy name-defined checks, and
- silently dropped the inactive-user identity-ownership assertions, so that
  test no longer verified the AuthIdentity still points at the original
  user rather than the attacker (which also left `attacker` unused, F841).

Move the three ownership assertions back into
test_inactive_identity_authenticated_request_shows_confirmation where their
variables are defined, and let test_avatar_failure_does_not_break_login end
at its own assertion.

Also collapse the now-short update_user_avatar signatures and the test_saml2
import onto single lines so ruff-format leaves them unchanged.
Comment thread src/sentry/users/services/user/impl.py
- Move the inactive-user identity-ownership assertions back into
  test_inactive_identity_authenticated_request_shows_confirmation; they had
  been orphaned onto test_avatar_failure_does_not_break_login, causing a
  NameError and dropping the ownership check.
- Sort the UserAvatar import in test_helper.py (ruff I001).
- Collapse the update_user_avatar signatures, the test_user_impl None call,
  and the test_saml2 import to single lines to match ruff-format.
- Correct the update_user_avatar docstring to reference the SAML login flow
  instead of the earlier SCIM approach.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4a34499. Configure here.

Comment thread src/sentry/auth/providers/saml2/provider.py
- Sync the denormalized User.avatar_url/User.avatar_type fields when storing
  a SAML avatar, mirroring UserAvatarEndpoint, so readers of the User model
  don't go stale (previously only UserAvatar was written).
- Broaden _validate_saml_avatar to catch any exception from Pillow (not just
  OSError) so hostile image data (e.g. DecompressionBombError) drops the
  avatar instead of breaking SSO login.

Tests assert the denormalized User fields on upload and clear, and that an
unexpected Pillow error during validation is swallowed.
@Asynchronite

Copy link
Copy Markdown
Contributor Author

Should be good now
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow SAML/SCIM providers to provide a profile picture URL when provisioning users

1 participant