Summary

Add a toggle to turn off automatic user provisioning for SAML.

Review Request #14394 — Created April 1, 2025 and updated 2 days, 8 hours ago

Information

Owner

david*

Repository

Review Board

Branch

release-6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description*

This change adds a new configuration toggle for SAML integration to
allow turning off automatic user provisioning. When we first implemented
SSO via SAML, our expectation is that application entitlement would be
managed entirely via the IdP, but that's not always the case. This
change makes it so admins can disable user provisioning during the SAML
authentication flow, allowing them to manage users through other means.

This also fixes up an incorrectly copy/pasted label in the
authentication settings form.

Testing Done


Ran unit tests.
Verified that the new setting correctly disabled user provisioning.

Commits

Summary	ID
Add a toggle to turn off automatic user provisioning for SAML. This change adds a new configuration toggle for SAML integration to allow turning off automatic user provisioning. When we first implemented SSO via SAML, our expectation is that application entitlement would be managed entirely via the IdP, but that's not always the case. This change makes it so admins can disable user provisioning during the SAML authentication flow, allowing them to manage users through other means. Testing Done: - Ran unit tests. - Verified that the new setting correctly disabled user provisioning.	005d48d5e80c08afcb5158105cba3ae61d905f3c

Summary

Add a toggle to turn off automatic user provisioning for SAML.

This change adds a new configuration toggle for SAML integration to allow turning off automatic user provisioning. When we first implemented SSO via SAML, our expectation is that application entitlement would be managed entirely via the IdP, but that's not always the case. This change makes it so admins can disable user provisioning during the SAML authentication flow, allowing them to manage users through other means. Testing Done: - Ran unit tests. - Verified that the new setting correctly disabled user provisioning.

005d48d5e80c08afcb5158105cba3ae61d905f3c

Issues

Description	From	Last Updated
The change itself looks good. Although I'm curious about how does an admin manually provision a user?	maubin	4 days, 6 hours ago
We should localize this string. Also, what happens if we don't have a username here?	chipx86	2 days, 9 hours ago
We get more room for the text if we do this form: help_text=_( '...' '...' ),	chipx86	2 days, 14 hours ago
This looks like leftover debug code.	chipx86	2 days, 14 hours ago
Since we're passing in HTTP POST data, we should be explicit about string values going in so there are no …	chipx86	2 days, 14 hours ago
There are no open issues

flake8 passed.

JSHint passed.

The issue has been dropped. Show all issues

The change itself looks good. Although I'm curious about how does an admin manually provision a user?

david 4 days, 6 hours ago

Most probable answer is either through the admin UI or a script that can POST to /api/users/

maubin 3 days, 13 hours ago

I see. We should add a paragraph about this new setting in our docs (https://www.reviewboard.org/docs/manual/latest/admin/configuration/authentication-settings/#saml-2-0-authentication), with a brief note about how to manually provision users.

david 2 days, 8 hours ago

We don't currently document these other individual settings. I worry that adding more content to this page as it currently stands will obfuscate the more important things (actually getting SAML working). I think the help text is good enough for now and I'll make a note to myself to revamp this section of the docs.

reviewboard/accounts/sso/backends/saml/forms.py (Diff revision 1)

The issue has been resolved. Show all issues

We should localize this string.

Also, what happens if we don't have a username here?

david 2 days, 8 hours ago

We should always have it, since we specify the username in the form's initial data. I suppose there's possibly a case where this might say "None" if things go weird.

I'm adding some additional fixes to make it so we shouldn't even get to this part. I'll leave this here to make sure if we do end up with the form (for example, if the toggle was done mid-sequence), we won't accidentally create a user when we're not supposed to.

reviewboard/accounts/sso/backends/saml/forms.py (Diff revision 1)

The issue has been resolved. Show all issues

We get more room for the text if we do this form:

help_text=_(
    '...'
    '...'
),

reviewboard/accounts/tests/test_saml_forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This looks like leftover debug code.
```

reviewboard/accounts/tests/test_saml_views.py (Diff revision 1)

The issue has been resolved. Show all issues

Since we're passing in HTTP POST data, we should be explicit about string values going in so there are no surprises with serialization.

Change Summary:


Add handling a little earlier so we don't show the link-user form at all.
Fix a bad label in the auth settings form.

Description:

		This change adds a new configuration toggle for SAML integration to
		allow turning off automatic user provisioning. When we first implemented
		SSO via SAML, our expectation is that application entitlement would be
		managed entirely via the IdP, but that's not always the case. This
		change makes it so admins can disable user provisioning during the SAML
		authentication flow, allowing them to manage users through other means.
	+
	+	This also fixes up an incorrectly copy/pasted label in the
	+	authentication settings form.

Commits:

	Summary	ID
	Add a toggle to turn off automatic user provisioning for SAML. This change adds a new configuration toggle for SAML integration to allow turning off automatic user provisioning. When we first implemented SSO via SAML, our expectation is that application entitlement would be managed entirely via the IdP, but that's not always the case. This change makes it so admins can disable user provisioning during the SAML authentication flow, allowing them to manage users through other means. Testing Done: - Ran unit tests. - Verified that the new setting correctly disabled user provisioning.	922e6febacf2ca490f6e8bbf360202d15f90c84b
	Add a toggle to turn off automatic user provisioning for SAML. This change adds a new configuration toggle for SAML integration to allow turning off automatic user provisioning. When we first implemented SSO via SAML, our expectation is that application entitlement would be managed entirely via the IdP, but that's not always the case. This change makes it so admins can disable user provisioning during the SAML authentication flow, allowing them to manage users through other means. Testing Done: - Ran unit tests. - Verified that the new setting correctly disabled user provisioning.	005d48d5e80c08afcb5158105cba3ae61d905f3c

Diff:

Revision 2 (+250 -24)

Show changes

	reviewboard/accounts/sso/backends/saml/forms.py
	reviewboard/accounts/sso/backends/saml/sso_backend.py
	reviewboard/accounts/sso/backends/saml/views.py
	reviewboard/accounts/tests/test_saml_forms.py
	reviewboard/accounts/tests/test_saml_views.py
	reviewboard/templates/accounts/sso/link-user-provision.html
	reviewboard/templates/accounts/sso/link-user-provision-disabled.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.