No email is sent to administrator when account approval is needed

+1 for this. We've been working around this problem with Rules, but it isn't compatible with 10.3 right now. If openid_connect could do this for us, then we would have no need for Rules on most sites. I tested the patch on a site this morning.

I moved the patch into an MR since DrupalCI shut down in favor of GitLabCI. No need to give me credit.

I sat down at my desk today prepared to write a test for this, but I stopped and thought about the matter first. It occurred to me that maybe this problem ought to be handled in the External Authorization module instead. Other auth modules probably have this same problem. In fact, I know they do. The LDAP module also extends Ext. Auth. We used LDAP before switching to Azure Entra and needed to have a user registration rule back then. Actually, we just kept using the old rule that we had from when we used LDAP after switching.

But why stop there? Why is this necessary in the first place? It's because Core doesn't have a centralized function for reacting to the creation of users. The emails are sent out by the submit function of the user registration form. I don't know that this makes sense anymore. It feels like we're all dealing with leftover code from the old days. Core itself has to deal with this deficiency. The User module's JSON API integration also has to send out emails. If I knew the best way to change Core to fix this, then I would have already submitted an issue there. I'm still thinking about it.

Anyway, there's a strong argument that this module shouldn't be patched to take on this function. Speaking as an end user, I'd be totally grateful if it did get patched so I can uninstall Rules and have one less dependency. But as a maintainer of other modules, I'd understand if you wanted to tell everyone "Go use Rules or ECA to send out emails until Core changes." If you go that route, then I think it should be added to documentation.

I did an investigation into whether this could be done by Core. The logic for sending emails is too intertwined with the User registration form. There's still an argument for doing it in Core, but it would be a pain.

I also checked External Auth. I didn't understand that it's mostly a helper module. Since it doesn't really do anything to check whether a user should be allowed to register, then it probably isn't the right module to be sending the emails.

That means openid_connect really ought to be responsible for sending the email.

That said, there's a problem with writing the test for this. The relevant test cases in OpenIDConnectTest::dataProviderForCompleteAuthorization() are currently commented out because of a mocking issue. This is no surprise because testCompleteAuthorization() is very complicated, which itself is a reflection of how complicated OpenIDConnect::completeAuthorization() is. So we can't really test this right now, at least not with the existing test. The test needs to be fixed, which I think means simplifying completeAuthorization() as much as possible first.

That said, if we could get the current MR merged and then add a test for it as a follow-up, then you wouldn't hear any complaint from me. Otherwise, this issue should be postponed.

Postponing this on 📌 Uncomment test cases in dataProviderForCompleteAuthorization() Active since I was able to repair those test cases.

The test issue was merged, so this issue is unblocked.

I don't know how to test this. We don't rely on _user_mail_notify's return value. In fact, hardly anything in Core does either. We could do that, but we would still have to mock the function, which means we would return a mocked value. I'm not sure there's any benefit in doing that.

I'm removing the "Needs tests" tag, but if someone else comes up with an idea, then feel free to implement it. In the absence of any brilliant ideas about how to test this my only contribution will be to ensure that the tests pass with the function call added.