Add option to respect Drupal's registration configuration when auto-registration is enabled

It depends on how you setup the cas module. Looking at auto register user setting it states...

Enable to automatically create local Drupal accounts for first-time CAS logins. If disabled, users must be pre-registered before being allowed to log in.

I am not certain how a pre-register would work as I do not use it. I have auto register enabled. User gets into our site from the sso running cas server and the sso makes use of a migration from a custom database. The flow kinda looks like this

1. user orders a subscription from an order site
2. user is stored in custom user db
3. sso runs a user migration on a cron schedule of 2 minutes
4. sso migration runs a post import script to set roles
5. every 24 hrs another script runs to expire roles based on subscription data
6. users logging into the website are redirected to sso for login using the cas login

We also use the cas attribute module to map roles and migrate additional information like company name, title, first and last names, etc..

Well, at the very least, I think we could add a message to the CAS configuration page for auto-registration that indicates that the core settings is not respected. That would avoid any confusion.

Beyond that, I'm inclined to keep such a change out of the CAS module since it seems like a somewhat niche use-case. If you could expand on your organizations specific use case maybe that would help me see how this would be useful?

You can already programmatically do this by subscribing to the preregister event and changing their status to 0 there, and that's only a small bit of code.

If you can programmatically determine if user attempting to log in to your site is a valid user for that site, then wouldn't you prefer to not have the local accounts created at all, just like you're doing now with the pre register event? If instead the site allowed registration and blocked the users that are not allowed, then someone would have to go in and periodically clean out the invalid users.

If you could control the error message that's displayed based on various different scenarios, I suspect that you'd be much happier, no?

If so, I think the best use of time would be spent on fixing #3013524: Allow specifying custom error and login messages, which would allow site admins to specify custom wording for the messages that the CAS module prints out.

I committed #3013524: Allow specifying custom error and login messages, which allows you to customize the messaging displayed to the user when a module prevents registration. It displays it as an error message instead of a status message. If you'd like to display a status message instead, you could blank out the error message that CAS provides in the settings, and then continue adding your own via your subscriber.

I think I'm going to close this as won't fix due to the workarounds in place and to avoid adding complexity to the module configuration.

I think this is a legit request. Recently, we were spammed by tons of accounts being created in Drupal but we have no control over the CAS server. We would like to be able to moderate registration of the users that are auto-creating accounts. Trying to get some code.

• Needs tests
• BC issue: Some sites might have the Who can register accounts? set to "Visitors, but administrator approval is required", knowing that, with CAS module, that setting have no effect. If we merge this change their new accounts will be suddenly moderated. Even their config is wrong, we have to deal with this but how? Should we provide an update script and set the option to "Visitors"? Then the behavior will not change.

I think this is a legit request. Recently, we were spammed by tons of accounts being created in Drupal but we have no control over the CAS server.

- why do you have auto registration turned on?

> why do you have auto registration turned on?

Because we only allow CAS registration.

No, what I mean is, why do you have CAS configured to allow anyone with a CAS account to have an account auto created? Why don't you require that an administrator manually register users to the site that require access?

I'm just playing devil's advocate to verify we really need the additional complexity this adds

Everyone on Earth with a CAS account should be able to register. There's no admin pre-register. People are coming on the site and, if they want to login, they click the link, are redirected to the CAS server where they login or register a new CAS user. Then they are redirected back to our site. If they have a local account they are logging in. In the case there's no local account, we (CAS) auto-create one and log them in. But in some critical circumstances, admins want to moderate the registration. This is a native Drupal feature. As I told, we cannot do that on CAS Server level as the server is a service that we don't control. We would like to turn the switch and CAS to honour that configuration.

Okay. I guess it makes sense to add this support for such cases. It's just a little odd of course, because the user already "logged in" via CAS, but is being denied an account on the site. But I understand the use case now.

For existing sites, the safest solution to to make this opt-in via a feature flag checkbox that's revealed if the user enables auto-registration. "Respect user moderation setting" or something? I think implementing this change without considering the number of existing sites that might break is not a good idea

Added tests.

For existing sites, the safest solution to to make this opt-in via a feature flag checkbox that's revealed if the user enables auto-registration. "Respect user moderation setting" or something?

That works when CAS is being configured. But how about the sites already configured with auto-registration and admin approval set?

Then the new feature flag we have set will be "off", thus disabling this new event subscriber, and behavior is unchanged. Basically I'm suggesting we have a checkbox to controls if this event subscriber is used or not.

@bkosborne, a kill-switch is a good idea, thanks

Nice work, MR solves the issue for us. It is RTBC.

Two things from my review:

Missing post-update hook

Can you add a post_update hook to set new config option in cas.settings.yml for existing sites? I understand it's false by default, and retrieving it from settings when it doesn't exist will return null (which is falsy), but I think it's best if we set the default config anyway on existing sites.

Confusing behavior

After a user tries logging in via CAS a second time (after their account was registered but blocked), they are denied login, but there's no error message displayed. That's because you're returning an empty string for this type of login failure in ServiceController::getLoginErrorMessage(). I understand you did this because your event subscriber returns a message instead. However, it only does this when the account is initially being registered. After it's already registered, the event subscriber doesn't fire (which is makes sense), but now the user gets no error message.

How about altering this code:

    if ($this->isAdminApprovalNeeded() && $account->isBlocked()) {
      // Cannot log in until the admins are not approving the new account. Note
      // that CAS module provides, by default, the normal Drupal behavior by
      // showing a status message and, if configured, sending email
      // notifications to user and admins. This is achieved by listening to
      // ExternalAuthEvents::REGISTER event. Third-party may override this
      // behavior by providing a subscriber with a higher priority, implementing
      // their logic and stopping the event propagation.
      // @see \Drupal\cas\Subscriber\CasAdminApprovalRegistrationSubscriber
      $this->casHelper->log(LogLevel::DEBUG, 'Login denied as new account needs admin approval.');
      throw new CasLoginException("Cannot login, admin approval is required for new accounts", CasLoginException::ADMIN_APPROVAL_REQUIRED);
    }

So it only executes if it's a brand new account being registered (not for existing account logins).

Then, existing accounts that attempt to login but are already blocked will allow this existing code to execute:

    // Check if the retrieved user is blocked before moving forward.
    if (!$account->isActive()) {
      throw new CasLoginException(sprintf('The username %s has not been activated or is blocked.', $account->getAccountName()), CasLoginException::ACCOUNT_BLOCKED);
    }

Which will present the proper account blocked message to the user (which admins can configure).

In the previous commit the text fields were replaced with textareas, we need to adapt config schema for them.

Addressed remarks from #21. Ready for review.

Merged in the latest 2.x to this branch, and tests still pass. Looks good to me. Thanks all.