Skip to content

refactor(oauth): extract CactiOAuth provider factory#7129

Merged
TheWitness merged 7 commits into
Cacti:developfrom
somethingwithproof:refactor/oauth-provider-factory
May 13, 2026
Merged

refactor(oauth): extract CactiOAuth provider factory#7129
TheWitness merged 7 commits into
Cacti:developfrom
somethingwithproof:refactor/oauth-provider-factory

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

@somethingwithproof somethingwithproof commented May 8, 2026

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.

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>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.php with getProvider() and getDefaultOptions() helpers.
  • Refactors oauth2.php and the OAuth2 branch of mailer() (lib/functions.php) to use the new helper and handle unknown providers via null.
  • Adds tests/Unit/CactiOAuthTest.php coverage 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, $provider becomes null and the code silently skips $mail->setOAuth(...) while still setting AuthType = 'XOAUTH2'. That will likely fail later with a less actionable PHPMailer error. Consider returning a clear error (similar to oauth2.php’s "Unknown OAuth2 provider") when $provider is null, 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,
				])
			);
		}

Comment thread oauth2.php
Comment thread lib/functions.php Outdated
Comment thread tests/Unit/CactiOAuthTest.php
Comment thread lib/CactiOAuth.php
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>
@somethingwithproof somethingwithproof changed the title refactor(oauth): extract Cacti\Security\CactiOAuth provider factory refactor(oauth): extract CactiOAuth provider factory May 8, 2026
@somethingwithproof
Copy link
Copy Markdown
Contributor Author

Restructured to use flat lib/CactiX.php convention to align with #7088, #7073, #7077, #7075.

@somethingwithproof
Copy link
Copy Markdown
Contributor Author

Queue note: this should follow #7124. After #7124 lands, rebase this PR and drop duplicated tests/Helpers/UnitStubs.php and unrelated phpstan baseline churn.

@TheWitness TheWitness merged commit aae7f0e into Cacti:develop May 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants