Release/1.1.2 - #34
Merged
Merged
Conversation
9 tasks
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.
Description
Release 1.1.2.
release/1.1.2ismaster(at1.1.1) plus exactly one commit — nothing elserides along:
PRE-3631: expose the OAuth id_token on TokenOutputWhat it does
Exposes the OpenID Connect
id_tokenonTokenOutput, so a consuming plugin can identify whoauthorized a connection, not just which account the resulting token authorizes.
The Sylius plugin needs to display the connected PayPlug account's email on its gateway-configuration
admin screen. That email is not reachable from anywhere it currently looks:
GET /accountreturns onlyid,company_ref,country,object,is_live,configuration,permissions,payment_methods— no email, at any depth (verified live against several QA accounts).client_credentialstoken used for every background API call authenticates a machine, so itnames no user either.
The only carrier is the
id_tokenfrom the interactive authorization-code exchange, whichOAuth2Client::requestToken()was discarding before constructing itsTokenOutput.Output/TokenOutput— new nullableidTokenproperty, set from a 4th constructor argumentdefaulting to
null.Auth/OAuth2Client::requestToken()— readsid_tokenoff the token-endpoint response when presentand passes it through.
Two deliberate design points:
idTokenis a trailing argument with a default, so everypre-existing 3-argument caller keeps working unchanged.
requestToken()is shared byexchangeAuthorizationCode()and
getClientCredentialsToken(); only the former can ever produce anid_token, so asserting on itwould reject a perfectly usable client-credentials response. UPC does not parse the JWT — consumers
decode whichever claim they need.
Related Issue
Ticket: PRE-3631
Type of Change
release/*branch targetingmaster)✅ Quality Checklist
Local Environment & Hooks
make install).(PRE|SMP)-XXXX: descriptionpattern.(feature|fix|hotfix|refactor)/(PRE|SMP)-XXXX...or(release|patch)/x.y.z.Testing & Code Quality
make cs-fix).make stan— PHPStan level 8).make test).src/ortests/(no typed properties, arrowfunctions, constructor property promotion,
match,enum).CI/CD Deployment Context
compatibilitymatrix(PHP 7.1 / 7.4 / 8.0 / 8.1 / 8.2) and the
qualityjob.Notes for Reviewer
Upgrade impact on plugins that bump to 1.1.2 and change nothing: none.
TokenOutputisfinal, so nobody can have subclassed it with a 3-argument constructor that thenew signature would invalidate.
nulldefault; the new property is purely additive. Existingreads of
accessToken/expiresIn/tokenTypeare untouched.isset($data['id_token']) && \is_string(...)— it cannot throw and adds no failurepath. A response without an
id_tokenbehaves exactly as before.TokenManagerstores only the bare access-token string, neverthe object, so the token-cache format is unchanged. Nothing in UPC serializes,
json_encodes orget_object_vars()aTokenOutputeither.?stringis 7.1 syntax) — confirmed bymake verify-71, on top of the CIcompatibility matrix.
They also wouldn't see a value:
idTokenis non-null only when a caller usesexchangeAuthorizationCode()and passes a scope containingopenid/email. Today PrestaShopconsumes UPC only for
PhoneHelperand the exception types and runs its own OAuth through thepayplug-phpSDK; WooCommerce and Magento don't depend on UPC at all. Sylius is the sole consumer ofthe auth layer.
One behaviour change that could reach a plugin passively: if a consumer ever dumps a whole
TokenOutputfor debugging (print_r,var_dump,json_encode, or a logger that serializes contextobjects), an
id_tokenwould now appear in that output where nothing did before — and an id_token isa JWT carrying PII (
email,sub). No current consumer does this, and it only affects theauthorization-code path. Flagging it rather than pre-emptively adding a redacting
__debugInfo(),which would also hide the field from the consumer that legitimately wants it. Say if you'd rather it
be redacted.
Where the tests are:
tests/Output/TokenOutputTest.php— assignment, thenulldefault, and one case asserting theconstructor explicitly does not validate
idToken(an empty one must not invalidate an otherwiseusable response).
tests/Auth/OAuth2ClientTest.php—id_tokensurfaced from an authorization-code response, leftnullwhen the response omits it, and leftnullforclient_credentials. Those last two guard theoptionality: they fail the day someone makes
id_tokenrequired in the sharedrequestToken().make qualitygreen locally on this exact tree: cs-lint 0/104 fixable, PHPStan level 8 clean,339 tests / 904 assertions.
Downstream: the Sylius plugin's PRE-3631 branch pins
^1.1.2and cannot go green until this ismerged and the
1.1.2tag is pushed onmaster.CLAUDE.mdis updated in the same commit (Output/andsrc/Auth/bullets), per this repo'srule that a category's documentation moves with the code.