refactor(oauth): extract CactiOAuth provider factory#7129
Merged
TheWitness merged 7 commits intoMay 13, 2026
Merged
Conversation
Replace the inline provider switch in oauth2.php and the OAuth branch of mailer() in lib/functions.php with two helpers: - CactiOAuth::getProvider($name, $params) returns the configured provider instance for `google`, `azure`, `yahoo`, or `microsoft`, or null when the configured provider is unknown. - CactiOAuth::getDefaultOptions($name) returns the scope set each provider expects. Behaviour is preserved; both call sites now branch on a null return rather than relying on a `$provider` variable that may have fallen through the switch unset. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Cacti’s OAuth2 provider selection logic by extracting provider construction and default scope options into a centralized Cacti\Security\CactiOAuth helper, and updating the existing OAuth entrypoints to use it. This touches the mail OAuth/identity surface (provider selection and scopes) and adds unit coverage for the new helper.
Changes:
- Introduces
lib/Security/CactiOAuth.phpwithgetProvider()andgetDefaultOptions()helpers. - Refactors
oauth2.phpand the OAuth2 branch ofmailer()(lib/functions.php) to use the new helper and handle unknown providers vianull. - Adds
tests/Unit/CactiOAuthTest.phpcoverage for provider selection and default option scopes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
lib/Security/CactiOAuth.php |
Adds centralized OAuth provider factory and default-scope selection. |
oauth2.php |
Replaces the inline provider switch with calls into CactiOAuth. |
lib/functions.php |
Replaces duplicated provider switch in mailer() with CactiOAuth usage. |
tests/Unit/CactiOAuthTest.php |
Adds unit tests for provider selection and default options. |
Comments suppressed due to low confidence (1)
lib/functions.php:5635
- If the configured provider name is unknown,
$providerbecomesnulland the code silently skips$mail->setOAuth(...)while still settingAuthType = 'XOAUTH2'. That will likely fail later with a less actionable PHPMailer error. Consider returning a clear error (similar tooauth2.php’s "Unknown OAuth2 provider") when$providerisnull, instead of proceeding without OAuth configuration.
$provider = \Cacti\Security\CactiOAuth::getProvider($providerName, $params);
if ($provider !== null) {
$mail->setOAuth(
new PHPMailer\PHPMailer\OAuth([
'provider' => $provider,
'clientId' => $clientId,
'clientSecret' => $clientSecret,
'refreshToken' => $refreshToken,
'userName' => $email,
])
);
}
Same as PR-A: appends 11 PHPStan ignoreErrors entries that exist on upstream develop but are not yet baselined, so this branch's CI does not regress on phpstan analyse --level 6. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Rename lib/Security/CactiOAuth.php to lib/CactiOAuth.php and drop the Cacti\Security namespace. Update the two callers (oauth2.php and lib/functions.php mailer) to use the unqualified class name. Behavior is unchanged: only the FQCN was rewritten. The flat layout matches every other lib/Cacti*.php class on develop and in the canonical PRs (Cacti#7088, Cacti#7073, Cacti#7077, Cacti#7075). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Contributor
Author
Contributor
Author
TheWitness
approved these changes
May 13, 2026
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.
Part F of six PRs splitting #7123. Depends on #7124.
Extracts the OAuth2 provider switch from oauth2.php and the mailer()
function in lib/functions.php into lib/CactiOAuth.php. The factory
returns a configured league/oauth2-client AbstractProvider instance
(or null for unknown providers).
Behavior is unchanged. The two callers were rewritten to use the
unqualified class name; no other oauth2.php / lib/functions.php
logic was touched.
Restructured to the flat lib/CactiX.php convention used by #7088 /
#7073 / #7077 / #7075.