Skip to content

fix(auth): allow assigning SSO default role to current user's role or below#99595

Merged
cathteng merged 3 commits into
masterfrom
cathy/auth/default-role-choices
Oct 3, 2025
Merged

fix(auth): allow assigning SSO default role to current user's role or below#99595
cathteng merged 3 commits into
masterfrom
cathy/auth/default-role-choices

Conversation

@cathteng

@cathteng cathteng commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Fixes VULN-79 and RTC-1127

@cathteng cathteng requested review from a team September 16, 2025 17:45
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 16, 2025
@cathteng cathteng requested a review from a team September 16, 2025 17:45
cursor[bot]

This comment was marked as outdated.

@codecov

codecov Bot commented Sep 16, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../sentry/web/frontend/organization_auth_settings.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #99595      +/-   ##
==========================================
+ Coverage   79.70%   81.24%   +1.54%     
==========================================
  Files        8589     8592       +3     
  Lines      380508   380645     +137     
  Branches    24122    24122              
==========================================
+ Hits       303270   309257    +5987     
+ Misses      76876    71026    -5850     
  Partials      362      362              

@cathteng cathteng changed the title fix(auth): assign SSO default role for current user's role or below fix(auth): allow assigning SSO default role to current user's role or below Sep 16, 2025
cursor[bot]

This comment was marked as outdated.

Comment on lines 501 to +512
assert resp.status_code == 200

# no update occurred. owner is not an option from the dropdown

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.

Are you checking the dropdown to make sure that owner is not in the dropdown? It looks like the update to the post on line 506 just doesn't update?

@cathteng cathteng force-pushed the cathy/auth/default-role-choices branch from 5fbcf67 to b68ae98 Compare October 2, 2025 18:55

@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.

Bug: SSO Form Role Inconsistency

On the SSO settings form, a non-superuser's default_role field uses organization.default_role as its initial value. If this role is not manageable by the current user, it won't be in the filtered role_choices, leading to an inconsistent form state where the initial selection is unavailable.

src/sentry/web/frontend/organization_auth_settings.py#L81-L92

)
if provider.is_saml and provider.name != "SAML2":
# Generic SAML2 provider already includes the certificate field in it's own configure view
x509cert = forms.CharField(
label="x509 public certificate",
widget=forms.Textarea,
help_text=_("The SAML certificate for your Identity Provider"),
required=False,
disabled=disabled,
)

Fix in Cursor Fix in Web


@cathteng cathteng merged commit f548fce into master Oct 3, 2025
65 checks passed
@cathteng cathteng deleted the cathy/auth/default-role-choices branch October 3, 2025 15:03
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants