Skip to content

Add OAuth2 resource indicator helpers#8

Merged
loks0n merged 13 commits into
utopia-php:mainfrom
ChiragAgg5k:codex/oauth2-resource-indicators
Jun 19, 2026
Merged

Add OAuth2 resource indicator helpers#8
loks0n merged 13 commits into
utopia-php:mainfrom
ChiragAgg5k:codex/oauth2-resource-indicators

Conversation

@ChiragAgg5k

@ChiragAgg5k ChiragAgg5k commented Jun 19, 2026

Copy link
Copy Markdown
Member

Summary

Adds reusable OAuth2 resource indicator helpers to Utopia Auth and streamlines RFC 9068 access-token audiences as arrays.

Changes

  • Adds Utopia\Auth\OAuth2\ResourceIndicators for RFC 8707 resource normalization, validation, subset checks, set comparison, and audience construction.
  • Adds InvalidResourceException with ERROR_CODE = 'invalid_target' so OAuth endpoints can map malformed resources without parsing messages.
  • Updates AccessToken::issue() to accept array audiences only and reject empty audience lists before signing.
  • Keeps requested resource audiences strict: audience() falls back to the default only when no resource was requested.
  • Documents isSubsetOf() as the RFC 8707 refresh/downscope check.
  • Consolidates GitHub Actions into one CI workflow with SHA-pinned actions/checkout usage.
  • Updates PHPStan and Pint for PHP 8.5 compatibility, removes unused Psalm, and points README static analysis docs to composer check.

Test Plan

  • composer install
  • composer check
  • composer lint
  • ./vendor/bin/phpunit --configuration phpunit.xml tests
  • php -l src/Auth/OAuth2/InvalidResourceException.php
  • php -l src/Auth/OAuth2/ResourceIndicators.php
  • php -l src/Auth/Issuers/Asymmetric/AccessToken.php
  • php -l tests/Auth/OAuth2/ResourceIndicatorsTest.php
  • php -l tests/Auth/Issuers/Asymmetric/AccessTokenTest.php
  • ruby -e 'require "yaml"; YAML.load_file(".github/workflows/ci.yml")'
  • git diff --check

@greptile-apps

greptile-apps Bot commented Jun 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces ResourceIndicators (RFC 8707 resource normalisation, validation, subset/equality checks, and audience construction) and InvalidResourceException alongside an updated AccessToken::issue() that now takes array audiences only and guards against empty or non-list inputs.

  • ResourceIndicators::from() normalises and deduplicates resource URIs; isSubsetOf() / equals() support RFC 8707 downscope checks; audience() returns the requested resources or wraps the default when none were requested.
  • AccessToken::issue() validates that the audience array is non-empty and contains only non-empty strings before building the RFC 9068 JWT payload.
  • The existing per-workflow CI files are replaced by a single ci.yml; all three jobs include a git checkout HEAD^2 step that re-pins execution to the unmerged PR branch rather than the merge commit.

Confidence Score: 4/5

The PHP library code is solid, but the new unified CI workflow tests the unmerged PR branch in every job, so merge-induced breakage would go undetected.

The ResourceIndicators and AccessToken changes are well-guarded and tested. The only real defect is in ci.yml: all three jobs run git checkout HEAD^2, which checks out the PR branch tip rather than the merge commit that GitHub Actions produces by default. A breaking merge would not be caught, and CI could report green on code that fails after merging.

.github/workflows/ci.yml — the git checkout HEAD^2 step in every job needs to be removed to restore merge-commit validation.

Important Files Changed

Filename Overview
.github/workflows/ci.yml New unified CI workflow; all three jobs execute git checkout HEAD^2, which tests the unmerged PR branch instead of the merge commit, defeating integration validation.
src/Auth/OAuth2/ResourceIndicators.php New RFC 8707 helper class with validation, subset/equality checks, and audience construction; has unreachable duplicate-detection code in the private constructor.
src/Auth/OAuth2/InvalidResourceException.php Minimal exception class with RFC 8707 error code constant; no issues.
src/Auth/Issuers/Asymmetric/AccessToken.php Updated issue() to accept array-only audiences with explicit empty-array and non-list guards; logic is correct.
tests/Auth/OAuth2/ResourceIndicatorsTest.php Tests normalisation, invalid-resource rejection, subset/equality, and audience construction; coverage is adequate for the happy paths.
tests/Auth/Issuers/Asymmetric/AccessTokenTest.php Updated tests cover multi-element audiences, empty-audience rejection, claim ordering, scope injection protection, and signature validity.

Reviews (6): Last reviewed commit: "Consolidate CI workflow" | Re-trigger Greptile

Comment thread src/Auth/OAuth2/ResourceIndicators.php Outdated
Comment thread src/Auth/OAuth2/ResourceIndicators.php Outdated
Comment thread src/Auth/OAuth2/ResourceIndicators.php Outdated
Comment thread src/Auth/OAuth2/ResourceIndicators.php
Comment thread src/Auth/Issuers/Asymmetric/AccessToken.php
@loks0n loks0n merged commit 76bd615 into utopia-php:main Jun 19, 2026
4 checks passed
@ChiragAgg5k ChiragAgg5k deleted the codex/oauth2-resource-indicators branch June 19, 2026 12:11
Comment thread .github/workflows/ci.yml
with:
fetch-depth: 2

- run: git checkout HEAD^2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 CI tests unmerged branch, not the merge result

git checkout HEAD^2 resets every job to the PR branch tip rather than the merge commit that GitHub Actions checks out by default. This means integration breakage — e.g., a conflict resolution in the merge commit, or a breaking change on main that landed between branch creation and merge — goes untested. CI can report green while the actual merged code fails to build or passes tests with different behaviour. The same step is present in all three jobs (codeql, linter, tests). Removing these three git checkout HEAD^2 steps restores the standard behaviour of validating the merged result.

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.

2 participants