External Authentication Feature

[MikeAlhayek]

External Authentication Feature

We've introduced a new feature called External Authentication. This feature has been separated from the existing Users feature to improve dependency management and to provide an option to disable external authentication by default. As a result, the User Registration feature no longer needs to be enabled unless you specifically want to allow site registration.

This feature is only available on-demand and cannot be manually enabled or disabled. It is automatically enabled when a feature requiring external authentication is activated.

We included a fast-forward migration to enable this feature automatically if previously needed. The following settings have been relocated to new classes as part of this update:

• The properties UseScriptToSyncRoles and SyncRolesScript have been moved from LoginSettings to the new ExternalLoginSettings class.
• The property UseExternalProviderIfOnlyOneDefined has also been moved from LoginSettings to ExternalLoginSettings.

In addition, several properties have been moved from RegistrationSettings to a new ExternalRegistrationSettings class:

• NoPasswordForExternalUsers is now ExternalRegistrationSettings.NoPassword.
• NoUsernameForExternalUsers is now ExternalRegistrationSettings.NoUsername.
• NoEmailForExternalUsers is now ExternalRegistrationSettings.NoEmail.
• UseScriptToGenerateUsername is now ExternalRegistrationSettings.UseScriptToGenerateUsername.
• GenerateUsernameScript is now ExternalRegistrationSettings.GenerateUsernameScript.

Also, note the following updates in ExternalLoginSettings:

• UseScriptToSyncRoles has been renamed to ExternalLoginSettings.UseScriptToSyncProperties.
• SyncRolesScript has been renamed to ExternalLoginSettings.SyncPropertiesScript.

!!! note
When updating recipes to configure LoginSettings or RegistrationSettings, ensure that the settings reflect the new class structure.

User Registration Feature

The User Registration feature is no longer required if you only want to enable external authentication.

The following properties of RegistrationSettings are now deprecated and will be removed in the next major release:

• UsersCanRegister
• NoPasswordForExternalUsers
• NoUsernameForExternalUsers
• NoEmailForExternalUsers
• UseScriptToGenerateUsername
• GenerateUsernameScript

Previously, the UsersCanRegister property controlled which types of registration were allowed. With this update, this property is obsolete and will be removed in a future release. To enable site registration now, simply activate the User Registration feature.

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[MikeAlhayek]

@mvarblow if you are able to, please test out this PR.

[mvarblow]

@MikeAlhayek The NRE is fixed. However, I'm still able to register even though the User Registration feature is disabled.

[MikeAlhayek]

@mvarblow
Yes this is expected. This type of registration is only for external users and can't be disabled since it's a critical aspect of the external login. However, you should not be able to click on "Register" on the site and register a new user using a form. In other words, the /register route will not be available unless you enabled the registration feature.

[mvarblow]

@MikeAlhayek So this is an expected change in behavior? If so, I would suggest that this change should not got into 2.1, but wait for 3.0. This would be a breaking change for us. We have a few different configurations that our clients use -- one very common configuration is that external sign-in is enabled but user registration is disabled. An administrator must provision all users, including external users. This is supported in Orchard 2.0 (and earlier). In those releases, if I disable the User Registration feature then it also disables registration for external users.

(The bug in earlier releases that I was trying to fix is very subtle. If I left user registration disabled when I set up the site then user registration was blocked for both local and external users. Again, we counted on this behavior. However, if enabled user registration and set the UsersCanRegister property to anything other than NoRegistration before disabling the user registration feature then regular user registration was correctly disabled but external user authentication was not.)

If we're going to change the behavior as you suggest, could we at least control it via a setting in the external authentication feature, give admins the option to turn off registration for external users?

[MikeAlhayek]

So this is an expected change in behavior? If so, I would suggest that this change should not got into 2.1, but wait for 3.0. This would be a breaking change for us. We have a few different configurations that our clients use -- one very common configuration is that external sign-in is enabled but user registration is disabled. An administrator must provision all users, including external users. This is supported in Orchard 2.0 (and earlier). In those releases, if I disable the User Registration feature then it also disables registration for external users.

In your case, how can new user login for the very first time if they can't login? Typically with external authentication, new users will need to be registered into the app so a local account is created.

[mvarblow]

@MikeAlhayek We have two supported options -

(1) The admin pre-provisions a user with the correct email address; when the user signs in for the first time OrchardCore finds the user by email; the user is prompted to enter their password (which the admin provided them after provisioning the account) to link the external ID. This is standard Orchard Core.

(2) We added our own module that gives the admin the ability to manage external IDs for users via the OrchardCore dashboard. The admin pre-provisions the user and also adds the external ID (supplying the Entra ID user ID). This is what most of our clients do because it doesn't require the user to enter an Orchard password on first sign-in. They usually have local sign-in disabled, so users don't normally need to know (or have) a local password.

[MikeAlhayek]

@mvarblow I added a new setting DisableNewRegistrations to the ExternalRegistrationSettings which will allow you to achieve your use case.

In your case, the migration should set DisableNewRegistrations to true, it will disable "User Registrations" feature and the "External Authentication" will be auto enabled by Microsoft EntraID feature.

Can you please test it out again?

[MikeAlhayek]

@mvarblow just a suggestion to your approach (which seems to require lots of work from admin). I suggest you test the following setup instead:

1. Don't provision the users manually.
2. Enable external registration
3. On the ExternalRegistrationSettings use the following properties:
   • Use the SyncPropertiesScript to sync username/id/email with EntraID.
   • Set UseGenerateUsernameScript to true
   • Use the GenerateUsernameScript to generate a user name based on the claims from Entra ID.
   • Set NotPassword to true.
   • Set NoUsername to true.
   • Set NoEmail to true.
4. Then optionally, on the ExternalLoginSettings you can use these properties if needed
   • Set UseScriptToSyncProperties to true.
   • Set SyncPropertiesScript to set other properties if needed like roles, claims, phone number or anything else that you may need.

[mvarblow]

@MikeAlhayek Yes, we've considered that approach. Unfortunately, there are settings that need to be defined for users before they use the app which can't easily be pulled from Entra ID. Entra ID could be extended to contain the necessary information but that's often not practical for our clients. I'm sure we will revisit this in the future though.

Regarding your latest commit, I appreciate you adding that setting. That's perfect for us and gives the flexibility to keep things working as before. Thanks!

The migration didn't seem to set the new property as I expected. I did have to, after swapping to the PR branch, manually check that option. I think you expected it to be set by the migration?

And I noticed a small cosmetic issue. The notification (when I sign in using a new external user after I checked the Disable New External Authentication box) spans the whole page while the rest of the form is width limited. This looks a bit sloppy. But it does not appear related to this merge request.

[mvarblow]

@MikeAlhayek I see what's happening in the migration. It looks fine. It's not detecting my special case (you enable user registration and allow external user registration; then later disable user registration). But it at least gives me an easy way to explicitly turn on and off external user registration. This is great! Thanks.

[MikeAlhayek]

@mvarblow yes I think it's because you already run the same migration so it wont run again. You can modify the migration document if you need manually in the database to force it to run again.

If you see no issue with this PR (since you are testing it, please approve it) just so we have you as a tester

[Piedone]

@mvarblow could you please do a final check?

[mvarblow]

@MikeAlhayek Actually, I'm reverting the site each time I test the PR. So the migration is running. I have confirmed that in the debugger. Remember, in my case the UserRegistration feature is already disabled. The problem I experienced in Orchard 2.0 is that external registration is still allowed after I disabled the UserRegistration feature.

When I use your PR, the migration runs and sees that the UsersCanRegister property value is 2 (how it was set before I disabled the UserRegistration feature previously). It also sees that the UserRegistration is disabled, so it doesn't need to disable UserRegistration. I can manually disable new registrations from the external authentication feature after the fact.

To automatically fix the problem I pointed out originally, the migration would have to see that the UserRegistration feature is disabled and, regardless of the old UserCanRegister property value, decide to set DisableNewRegistrations to true.

As it is, your PR helps me. But I have to manually make that settings change after deploying the updated OrchardCore version.

To automate the fix-up I think you'd need to change:

            DisableNewRegistrations = enumValue == 0,

to

            DisableNewRegistrations = enumValue == 0 || !await featuresManager.IsFeatureEnabledAsync(UserConstants.Features.UserRegistration),

[MikeAlhayek]

@mvarblow I agree with the request and made the change. Can you please confirm that it is working for you?

[mvarblow]

@MikeAlhayek Looks good! Thanks for this!