refactor: replace CheckAccess with assertScope() — delete 250-line hardcoded scope map#26
Closed
herpaderpaldent wants to merge 8 commits into
Closed
refactor: replace CheckAccess with assertScope() — delete 250-line hardcoded scope map#26herpaderpaldent wants to merge 8 commits into
herpaderpaldent wants to merge 8 commits into
Conversation
…erface Scope enforcement now lives where scope is defined. Each generated execute() calls $transport->assertScope(self::REQUIRED_SCOPE) before any HTTP call. The transport (EsiClient) implements the check against the JWT token. Changes: - EsiClient: implement assertScope(?string $scope): void - null = public endpoint → no-op - non-null = must be present in JWT scopes → ScopeAccessDeniedException - EsiClient: remove CheckAccess wiring from constructor and withToken() - EsiClient: remove hasAccess() pre-flight from invoke() - EsiScopeAccessDeniedException: now extends ScopeAccessDeniedException from esi-schema for backward compatibility - Delete CheckAccess.php (250-line hardcoded scope map — no longer needed) - Delete CheckAccessTest.php (replaced by assertScope tests in EsiClientTest) - Bump seatplus/esi-schema: ^1.1 (requires assertScope in EsiTransportInterface) - Update GeneratedResourcesTest: use real JWT tokens, remove CheckAccess mock - Update EsiClientTest: replace access-denied test with assertScope tests Requires seatplus/esi-schema ^1.1 (PR #4). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Until esi-schema PR #4 is merged and 1.1.0 tagged on Packagist, use the branch dev alias so CI can resolve the dependency. Will be reverted to ^1.1 once tag exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…formats workflow - Add docblock to EsiClient::assertScope() explaining null semantics - Update formats.yml to also trigger on composer.json changes and pull_request targeting feat/esi-rate-limit-overhaul Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… sufficient Explicit dev-* constraints override minimum-stability per-package, so setting minimum-stability=dev globally was unnecessary and caused pest-plugin-type-coverage to resolve a dev build instead of stable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gist) Replaces the temporary dev-feat/assert-scope alias now that esi-schema PR #4 has been merged and tagged 1.1.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ption from esi-schema directly No backwards compatibility needed. Callers should catch Seatplus\EsiSchema\Contracts\ScopeAccessDeniedException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… spec The ESI OpenAPI 3.1 spec (esi.evetech.net/meta/openapi.yaml) defines: servers: [url: https://esi.evetech.net] paths: /alliances, /characters/{character_id}/assets, … No version prefix anywhere. Versioning is handled by X-Compatibility-Date header (already sent by GuzzleFetcher). The /latest/ prefix was a Swagger 2.0 basePath artifact. Changes: - buildDataUri(): /{path}/ instead of /latest/{path}/ - EsiConfiguration: default compatibility_date to '2025-12-16' (matches esi-schema generation date; update when regenerating) - Update tests accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
Changes merged directly into feat/esi-rate-limit-overhaul (PR #22). |
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
Replaces the 250-line hardcoded
CheckAccess.phpscope map withassertScope()— a method onEsiTransportInterfacethat lets each generated operation class enforce its own scope requirement.Changes
CheckAccess.php— 250 lines of(method, path) → scopemaintenance burden goneEsiScopeAccessDeniedException.php— superseded byScopeAccessDeniedExceptionfromseatplus/esi-schemaEsiClient::assertScope(?string $scope)— checks JWT scopes; null = public endpoint (no-op)hasAccess()pre-flight frominvoke()— check now lives in eachexecute()callseatplus/esi-schema→^1.1— requires theassertScope()interface addition/latest/URL prefix — ESI OpenAPI 3.1 paths have no version segmentcompatibility_dateto'2025-12-16'— correct versioning header sent automatically^8.3formats.ymlWhy
Each of the 208 generated operation classes already knows
REQUIRED_SCOPE. Having a second parallel map inCheckAccesswas a maintenance liability (any new/changed endpoint required manual updates in two places).Test coverage
All 113 tests pass.
esi-schema 1.1.0ships 209 tests verifyingassertScope()is called on all 208 operation classes.