OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.43k stars 2.4k forks source link

Redirect loop instead of error message when using external identity provider #16181

Open mvarblow opened 5 months ago

mvarblow commented 5 months ago

Describe the bug

If you enable an external identity provider such as Microsoft Entra ID and the configure the user settings to disable local sign-in and user registration, then users who have not been provisioned will not receive the "Site does not allow user registration" error as expected. Instead, they end up in an infinite redirect loop.

To Reproduce

Steps to reproduce the behavior:

  1. Start and Orchard Core site and enable the Microsoft Entra ID Authentication feature - see https://docs.orchardcore.net/en/latest/guides/microsoft-entra-id-integration/.
  2. Click on Security > Settings > User Login
  3. Check "Use external provider for login". Click Save.
  4. Click on Security > Settings> User Registration and ensure that "Configure users registration" is set to NoRegistration. Click Save.
  5. In a new incognito window, open the site and attempt to access the admin page. You'll be redirected to Microsoft Entra ID to sign in and then sent back to the Orchard Core site. The Orchard Core site will redirect you back to Entra ID. This time you won't be prompted to sign in since you're already signed in, you'll just be redirected back to the Orchard Core site. Orchard Core will redirect you to Entra ID. ....

Expected behavior

If you look in the Orchard Core log file you'll see a warning: "Site does not allow user registration." This warning will be repeated many times, depending on how long the user allowed the redirect loop to continue as they waited for the page to appear. I would expect that the user should receive an error message similar to what was logged instead of being continuously redirected. If I locate the code in AccountController which is responsible for generating that log entry, it appears that this is what the author of the code also intended. Though I think they didn't consider this scenario where the "use external provider for login" option was enabled.

image

If "use external provider for login" were disabled, then the redirect wouldn't happen. Instead, the user would see the sign-in page with the error message. To fix this, it seems that the AccountController needs to account for this possibility and replace the RedirectToLogin response with a view which can display the error message.

Piedone commented 5 months ago

This looks indeed like something should be handled better. Although enabling external login, not having users pre-created, and not enabling registration together is also an incorrect configuration. Or did you do it deliberately?

mvarblow commented 5 months ago

This looks indeed like something should be handled better. Although enabling external login, not having users pre-created, and not enabling registration together is also an incorrect configuration. Or did you do it deliberately?

In this case, we are expecting that users will be pre-created. An administrator needs to provision the user manually to ensure their account is set up correctly in the OrchardCore site. But there will occasionally be cases where a user tries to sign in before their OrchardCore account has been provisioned. In this case, we'd expect an error. What the user gets instead is a page that seems to the user to be loading "forever". Of course it's actually in a redirect loop.

Piedone commented 5 months ago

I see, thanks.

github-actions[bot] commented 5 months ago

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.