feat: add BaseServiceAccountProvider for RFC 7523 JWT-bearer auth - #3442
Merged
Conversation
Core shipped OAuth2, OAuth1, and HTTP Basic providers but had no service-account primitive, so every consumer hand-rolled JWT assembly, RS256 signing, and token caching inside its own ability class. Three independent copies exist today - GoogleAnalyticsAbilities, GoogleSearchConsoleAbilities, and VenueDiscoveryAbilities in a different plugin entirely. They drifted, and the drift produced a real bug: GA cached its token with get_transient() while GSC used get_site_transient() for the identical flow, so on a 10-site network the same credential minted up to 10 tokens. This class owns the mechanism. Vendor specifics - token endpoint and scopes - stay in the subclass. Two properties are structural rather than left to subclasses: - Tokens cache network-wide. A service account credential is stored via get_site_option(), so the token it mints is valid across the network. Making this a per-consumer decision is what produced the GA bug. - Scope is part of the cache key, so one credential can serve several consumers without evicting each other. Also adds private_key, service_account_json, and credentials_json to ENCRYPTED_FIELDS. Every OAuth access token on the platform was encrypted at rest while service account RSA private keys sat plaintext in wp_sitemeta - protection inverted relative to risk, since a private key is long-lived and unscoped where a token is short-lived and scoped. Supports optional domain-wide delegation via a subject claim, omitted unless configured. Sixteen tests including cryptographic signature verification against a real RSA keypair. A JWT that parses but does not verify is exactly the failure this primitive exists to stop recurring. Refs #3437
Three findings, all real. PHPCS flagged base64_encode() as possible obfuscation. RFC 7515 requires base64url for JWT segments, so this is a wire format. Suppressed with the reason recorded, matching the existing convention in BaseAuthProvider and JobsCommand. PHPStan flagged '' === $tag as always false. $tag is filled by reference by openssl_encrypt(), which static analysis cannot model. The check is real - an empty auth tag produces an undecryptable envelope - so it is kept and made type-honest rather than removed. The line is pre-existing, pulled into lint scope by the ENCRYPTED_FIELDS addition. The test runner reported zero executed tests. The concrete stub was named TestServiceAccountProvider, which matches PHPUnit's Test* class discovery prefix; it was collected as a test class, contributed no test methods, and the suite reported nothing ran. Renamed to StubServiceAccountProvider.
strlen() did not help - PHPStan still infers $tag as non-empty because openssl_encrypt() fills it by reference, so any comparison against the empty case narrows to always-false. empty() is not folded the same way. The guard itself is unchanged in behavior and still catches an empty auth tag, which would otherwise produce an envelope that cannot be decrypted.
The sandbox bootstrap failed loading the test file: the stub subclass inherited BaseAuthProvider::get_config_fields() as abstract and did not implement it, so the class was not concrete and the whole shard reported zero executed tests. Implementing it in the primitive rather than the stub is the right level. Every service account flow takes the same input - a key file and an optional delegated subject - so the concrete default belongs with the mechanism. Subclasses override only when a vendor needs something different. A minimal subclass now needs to implement exactly one method, get_token_endpoint(), which is the intended contract.
setUpBeforeClass() called openssl_pkey_new(), which returns false in the sandbox PHP build, so openssl_pkey_get_details() got false and the whole class errored with a TypeError before any assertion ran. Commits a 2048-bit test keypair as a fixture instead. This also makes the signature assertion deterministic rather than depending on runtime key generation. The key is a test fixture and protects nothing; it exists so test_assertion_signature_verifies_against_the_public_key can do real cryptographic verification rather than assert JWT shape.
Two test shards failed at 'install Homeboy extension' with exit code 100 and review test not_run, while sibling shards on the same commit passed. Nothing in the diff is involved - the failure is before any test executes.
extract_credential() required both client_email and private_key before treating discrete fields as the configured credential. A config with only client_email fell through to the JSON branch and returned datamachine_service_account_missing - telling an operator no credential was configured when one was, just incompletely. Any discrete credential field now means discrete fields were configured. Completeness is checked separately in get_service_account(), which produces the actionable "missing X" error. Caught by test_incomplete_credential_is_a_specific_error, which is the kind of bug that would otherwise surface as a confusing support question rather than a stack trace.
chubes4
added a commit
to Extra-Chill/data-machine-business
that referenced
this pull request
Sep 4, 2026
…121) * refactor: migrate Google service account auth to the shared provider Deletes two hand-rolled copies of the RFC 7523 JWT-bearer flow - GoogleAnalyticsAbilities and GoogleSearchConsoleAbilities - in favour of chubes4/data-machine's BaseServiceAccountProvider. Net 211 lines removed, 122 added, most of the additions being the vendor provider that replaces both copies. GoogleServiceAccountAuth supplies only Google's token endpoint and the stored credential. Core owns claim assembly, RS256 signing, the token exchange, and network-wide caching. Fixes a second instance of the same drift the copies caused. GA cleared its cached token on config save with delete_transient(), which is site-scoped, while the token it was clearing is network-wide - so a config change never actually invalidated the cache. GSC used delete_site_transient() correctly. Both now clear through the provider, which owns the cache scope, so the question cannot be answered differently in two places again. Legacy credential storage is preserved. The provider falls back to each consumer's existing option (datamachine_ga_config, datamachine_gsc_config) when no provider-scoped config exists, so current installs keep working with no migration step. New configuration flows through provider storage, where the private key is encrypted at rest. The per-ability TOKEN_TRANSIENT constants are removed; both were dead after migration and both external call sites now route through the provider. Depends on Extra-Chill/data-machine#3442. Refs #119 * refactor: inherit service account config fields from the primitive get_config_fields() now has a concrete default in BaseServiceAccountProvider, since every service account flow takes the same input. The vendor provider only needed the token endpoint and the legacy option fallback.
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
1 participant
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.
Refs #3437. The core half — consumer migration follows in
data-machine-businessandextrachill-events, which is where the deletions land.Problem
Core shipped
BaseOAuth2Provider,BaseOAuth1Provider, andHttpBasicAuthProviderbut had no service-account primitive:RFC 7523 (
urn:ietf:params:oauth:grant-type:jwt-bearer) is the standard server-to-server grant. With no base to extend, every consumer hand-rolled it inside an ability class. Three copies exist:GoogleAnalyticsAbilitiesdata-machine-businessGoogleSearchConsoleAbilitiesdata-machine-businessVenueDiscoveryAbilitiesextrachill-events— a different pluginWorth stressing: downstream is not ignoring an available abstraction. It composes core correctly everywhere core provides one (
GoogleAuth extends BaseOAuth2Provider,SlackAuth/DiscordAuth extends BaseAuthProvider). The hand-rolling happens precisely where core is silent.The drift was a real bug
GA and GSC are near-identical — same claims, same endpoint, differing only in scope string, error prefix, brace style, and this:
On a 10-site network GA re-minted the same token per site: up to 10× the token-endpoint round trips and 10× the RSA signings for one credential. Nothing reconciled the copies because nothing could. Symptom fixed in
Extra-Chill/data-machine-business#120; this removes the class of bug.Design
BaseServiceAccountProvider extends BaseAuthProviderowns claim assembly, RS256 signing, the jwt-bearer exchange, and caching. Subclasses supply onlyget_token_endpoint().Two properties are structural, not per-consumer decisions:
get_site_option(), so its token is valid across the network. Leaving this to each consumer is exactly what produced the GA bug.Also accepts a raw service-account JSON blob, since providers hand one out and operators paste it verbatim.
Optional domain-wide delegation via
sub, omitted unless configured — an emptysubwould make every request look like an impersonation attempt.Encryption gap closed
Added
private_key,service_account_json,credentials_jsontoENCRYPTED_FIELDS.Every OAuth access token on the platform was encrypted at rest while service-account RSA private keys sat plaintext in
wp_sitemeta. That is inverted relative to risk: a private key is long-lived and unscoped; a token is short-lived and scoped.Tests
16 tests. The signature is verified cryptographically against a real RSA keypair, not asserted by shape — a JWT that parses but does not verify is precisely the failure this exists to prevent.
Covers: RS256 header, claim contents,
exp/iatrelationship,subomitted-by-default and sent-when-configured, network-wide cache reuse, per-scope isolation, cache clearing, four specific error codes, JSON-blob credentials, and that the new fields are marked for encryption.Compatibility
Purely additive. No existing provider changes behavior. The three existing implementations keep working until migrated.
Follow-up
Consumer migration is where lines actually disappear — roughly 130 lines of duplicated JWT code across two repos collapse into
extends BaseServiceAccountProviderplus a token endpoint. Tracked inExtra-Chill/data-machine-business#119.