fix: a default profile is one the administrator could have assigned - #914
Merged
Merged
Conversation
Three doors set the profile a user is auto-provisioned into, and none asked whether the administrator setting it could have granted that profile by hand. UserProfileService::assertAssignableBy() is that check, and every user create/edit door already calls it — six call sites across web and API. ConfigLdap's import and save had USER_CREATE but not assignability; ConfigAuth's save, which sets ssoDefaultProfile for every SSO auto-provisioned user, had neither. CONFIG_LDAP and CONFIG_GENERAL answer isConfigGeneral() while USER_CREATE answers isMgmUsers() — independent bits — so a delegated administrator holding both could name a profile stronger than their own and have every directory or SSO user provisioned into it. USER_CREATE is 'may create users at all'; it says nothing about how much a particular profile grants. The SSO guard fires only when the settings change, copied from ConfigLdap's, so an administrator of the rest of that page can still save it.
blaipr
deleted the
fix/a-default-profile-is-one-the-admin-could-assign
branch
September 7, 2026 23:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three doors set the profile a user is auto-provisioned into, and none of them asked whether the
administrator setting it could have granted that profile by hand.
UserProfileService::assertAssignableBy()is the check: unless you areisAdminApp, you cannotassign a profile that
grantsBeyond()your own. Every user create/edit door already calls it — bothweb save controllers and both API ones, six call sites. These three did not:
USER_CREATEConfigLdap\ImportControllerConfigLdap\SaveControllerldapDefaultProfile, read byUser::createOnLogin()for every future LDAP sign-inConfigAuth\SaveControllerssoDefaultProfile, the same for SSOCONFIG_LDAPandCONFIG_GENERALanswerisConfigGeneral();USER_CREATEanswersisMgmUsers().Those are independent profile bits, so a delegated administrator holding both — an ordinary
arrangement — could name a profile stronger than their own and have every directory or SSO user
provisioned into it.
USER_CREATEanswers "may create users at all"; it says nothing about how mucha particular profile grants, which is the whole point of the other check.
The SSO door is the weakest of the three: it had neither half, so
isConfigGeneral()alone wasenough. It now guards both, and only when the settings actually change — copied from
ConfigLdap\SaveController, so an administrator of the rest of the page can still save it.Tests
Four, one per gap, each with a
UserProfileServicewhoseassertAssignableBy()refuses:USER_CREATEis refused;Mutation-verified: reverting
src/fails all four and nothing else.