Add optional setup_command input to Laravel test workflow - #13
Merged
Conversation
Consumer repos sometimes need app-specific setup (e.g. generating Passport RSA keys) after the environment is prepared but before tests run. Adds a new no-op-by-default setup_command input instead of forcing consumers to prefix their test_command. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aquarion
added a commit
to istic/Alchemistic
that referenced
this pull request
Jul 28, 2026
Temporarily points at the shared-workflows feature branch (istic/shared-workflows#13) that adds setup_command; revert this ref to @main once that PR merges. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aquarion
added a commit
to istic/Alchemistic
that referenced
this pull request
Jul 28, 2026
…ry layer) (#32) * 📖 Add design spec for Alchemistic as an OIDC provider Documents the plan to make Alchemistic issue OpenID Connect tokens for first-party Istic SSO, built on Laravel Passport with a thin custom OIDC layer (id_token, userinfo, discovery). * 📖 Add implementation plan for OIDC provider Task-by-task plan built on Laravel Passport with a custom id_token/userinfo/jwks/discovery layer. Also corrects the spec: Passport has no built-in JWKS endpoint, so /oauth/jwks is custom-built rather than reused. * 🎇 Install Laravel Passport Adds laravel/passport ^13.7, runs install:api --passport to migrate the oauth_* tables and generate signing keys, and registers the api guard (passport driver) in config/auth.php as the foundation for the OIDC provider work. * 🎇 Add Passport token support to User model * 🎇 Add first-party OAuth client model that skips consent * 🎇 Configure Passport scopes, client model, and route overrides * 🎇 Add OIDC signing key / JWK helper Also bootstraps the Laravel app for tests/Unit (Pest previously only extended Tests\TestCase for Feature tests), since Passport::keyPath() needs storage_path() which requires a booted application. * 🎇 Add id_token builder and AccessTokenCreated listener Adds IdTokenBuilder to sign RS256 id_tokens with standard OIDC claims (sub, aud, iss, iat, exp, name, email, email_verified, permissions), plus PendingIdToken (a request-scoped holder) and AttachOidcIdToken, the AccessTokenCreated listener that builds an id_token and stashes it for the token controller to read back, skipping when there's no user or the openid scope wasn't granted. * 🎇 Add token controller that attaches id_token to the response * 🎇 Register OAuth authorize and token routes * 🎇 Add end-to-end authorization code + PKCE flow test Exercises the whole OIDC provider stack via real HTTP requests through /oauth/authorize and /oauth/token, rather than Passport's actingAs shortcuts. Uncovered that Passport's AuthorizationController always injects an AuthorizationViewResponse dependency (even when the request will be auto-approved for first-party clients), which has no default binding once Passport::ignoreRoutes() is used instead of the package's own routes. Wired a minimal fallback via Passport::authorizationView() in AppServiceProvider — this app intentionally doesn't build a consent screen, but the container still needs something bound so a stray non-first-party client doesn't 500 instead of failing gracefully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * 🎇 Add /oauth/userinfo endpoint * 🎇 Add OIDC discovery document and JWKS endpoints * 🪳 Reject grant types other than authorization_code/refresh_token Final review found that Passport's AuthorizationServer wires up the client_credentials grant unconditionally, letting confidential clients mint tokens outside the Authorization Code + PKCE flow the design explicitly scopes this provider to. TokenController now rejects any other grant_type with a standard unsupported_grant_type error before forwarding to Passport, and resets the pending id_token holder before each request to avoid a stale id_token leaking into an unrelated response under a long-lived worker (Octane/Swoole) in the future, even though this app currently runs PHP-FPM where it can't happen. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * 🪳 Generate Passport RSA keys before running tests in CI CI's shared laravel-tests workflow doesn't run passport:keys, so storage/oauth-*.key never existed and every OIDC test that reads the keys threw "Invalid key supplied". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * 🔄️ Use shared workflow's new setup_command instead of prefixing tests Temporarily points at the shared-workflows feature branch (istic/shared-workflows#13) that adds setup_command; revert this ref to @main once that PR merges. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * 🪳 Generate Passport RSA keys in ci.yml's test job This job is separate from the shared laravel-tests workflow and has its own test setup, so it needed the same fix independently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * 🪳 Address PR review findings: nonce, logging, CORS, and defensive guards Fixes from the multi-agent review of the OAuth/OIDC provider branch: - Plumb the OIDC `nonce` from /oauth/authorize through to the id_token, via a custom AuthCodeRepository that stashes it against the auth code's identifier and an AuthCodeIdDecrypter that recovers it at /oauth/token using Passport's own encryption key. Most OIDC client libraries reject an id_token missing an echoed nonce. - Publish config/cors.php with oauth/* and .well-known/* added to the default paths, so browser-based first-party clients aren't silently blocked by CORS. - Log (rather than silently return) when AttachOidcIdToken can't find the just-created access token or its user — these should never happen and previously failed with zero diagnostic trail. - Fail loudly with an actionable message when the Passport RSA key files are missing/unreadable or malformed, instead of a confusing generic error or TypeError. - Guard against a null authenticated user and non-JSON upstream responses in UserInfoController/TokenController. - routes/api.php: explicit return type on the /api/user closure. - /oauth/userinfo now also accepts POST per the OIDC spec. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
aquarion
marked this pull request as ready for review
July 28, 2026 21:49
aquarion
added a commit
to istic/Alchemistic
that referenced
this pull request
Jul 28, 2026
istic/shared-workflows#13 (setup_command support) has merged, so this no longer needs to pin to a feature branch. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
setup_commandinput to the reusablelaravel-tests.ymlworkflow, run after the DB/keys/permissions setup steps but before tests (or Dusk migrations) execute.php artisan passport:keys --forcerun before its OIDC tests can read the RSA keys — prefixingtest_commandworked but conflated "how to run tests" with "how to set up the environment".Test plan
setup_command: "php artisan passport:keys --force"and CI passes.