Merged
Conversation
📝 WalkthroughWalkthroughReplaces sprintf-based realm scope strings with a global SCOPE_BASE_REALM constant across security scope classes, seeders, tests, and consumers; adds a new TeamScopes class; ensures SCOPE_BASE_REALM is defined in AppServiceProvider; and adds a Swagger UI build + PR-preview GitHub Actions workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant GH-Actions as GitHub Actions
participant BuildJob as openapi-generate (job)
participant ArtifactStorage as Actions Artifact
participant PagesJob as pages-preview (job)
participant GH-Pages as GitHub Pages
participant PR as Pull Request
rect rgba(120,180,240,0.5)
PR->>GH-Actions: open PR triggers workflows
GH-Actions->>BuildJob: run openapi-generate
BuildJob->>ArtifactStorage: upload `swagger-ui` artifact
end
rect rgba(120,220,140,0.5)
GH-Actions->>PagesJob: pages-preview runs on PR (downloads artifact)
PagesJob->>ArtifactStorage: download `swagger-ui`
PagesJob->>GH-Pages: publish under `public/openapi/pr-<PR_NUMBER>`
PagesJob->>PR: post comment with preview URL
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
smarcet
force-pushed
the
feature/prefixed-const-scopes
branch
from
0c5d732 to
dfbf1ab
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/ProtectedApiTestCase.php (2)
30-31:GroupsScopesis imported but never used.The import for
GroupsScopesis added but noGroupsScopesconstants are used in eitherAccessTokenServiceStuborAccessTokenServiceStub2.🧹 Suggested fix: Either remove the unused import or add the scope
If the scope should be included in test tokens:
SummitScopes::ReadBadgeScanValidate, + GroupsScopes::ReadData, RSVPInvitationsScopes::Write,Or remove the unused import:
use App\Security\SponsoredProjectScope; -use App\Security\GroupsScopes; use App\Security\TeamScopes;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ProtectedApiTestCase.php` around lines 30 - 31, The import for GroupsScopes is unused; either remove the unused import line "use App\Security\GroupsScopes;" or add the appropriate GroupsScopes constant(s) to the test token scopes in the stub classes. Locate AccessTokenServiceStub and AccessTokenServiceStub2 in this file and either append the relevant GroupsScopes::<CONSTANT> value(s) to their scopes arrays used when building tokens, or simply delete the GroupsScopes import if those stubs should not include group scopes.
82-85: Consider creating scope constants to complete the migration.The following scopes in
ProtectedApiTestCase.phpstill use$url . '/...'concatenation instead of constants (lines 83-85, 99-100):
public-clouds/readprivate-clouds/readconsultants/readmembers/invitations/readmembers/invitations/writeNo scope constants currently exist for these. Creating dedicated scope classes (e.g.,
PublicCloudsScopes,PrivateCloudsScopes,ConsultantsScopes) would provide better consistency and maintainability alongside the existingSummitScopes,MemberScopes, andTeamScopesclasses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ProtectedApiTestCase.php` around lines 82 - 85, Replace the ad-hoc "$url . '/...'" scope concatenations in ProtectedApiTestCase.php by introducing and using dedicated scope constant classes for the missing domains: add PublicCloudsScopes, PrivateCloudsScopes and ConsultantsScopes (each exposing named constants such as READ) and add MembersInvitations or extend MembersScopes to provide INVITATIONS_READ and INVITATIONS_WRITE constants; then update the $scopes array and any other places currently using "$url . '/public-clouds/read'", "$url . '/private-clouds/read'", "$url . '/consultants/read'", "$url . '/members/invitations/read'", and "$url . '/members/invitations/write'" to reference the new constants (e.g., PublicCloudsScopes::READ) instead of string concatenation.app/Http/Utils/FilterAvailableSummitsStrategy.php (1)
15-15: Remove unused import.The
Configfacade is no longer used in this file after the refactor.🧹 Suggested fix
use App\Security\SummitScopes; -use Illuminate\Support\Facades\Config; use models\oauth2\IResourceServerContext;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Http/Utils/FilterAvailableSummitsStrategy.php` at line 15, Remove the now-unused Config facade import from the top of FilterAvailableSummitsStrategy.php: locate the use Illuminate\Support\Facades\Config; statement in the file (used alongside the FilterAvailableSummitsStrategy class) and delete that import line so the file only retains necessary use statements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Providers/AppServiceProvider.php`:
- Around line 721-723: Normalize and validate the config value before defining
SCOPE_BASE_REALM: read config('app.scope_base_realm'), trim whitespace and
remove any trailing slashes, then ensure the resulting string is non-empty (or
fail loudly/log an error) before calling define('SCOPE_BASE_REALM', ...). Update
the code around the SCOPE_BASE_REALM definition in AppServiceProvider (where you
currently call define with the raw config value) to perform the trim/rtrim and
non-empty check first so generated scopes won't contain empty segments or
trailing slashes.
In `@tests/ProtectedApiTestCase.php`:
- Around line 194-195: The array of scopes contains a duplicate entry of
SummitScopes::WriteVideoData; remove the redundant SummitScopes::WriteVideoData
occurrence so the list only includes that scope once (locate the scope list
where SummitScopes::WriteVideoData appears twice and delete the duplicate
entry).
---
Nitpick comments:
In `@app/Http/Utils/FilterAvailableSummitsStrategy.php`:
- Line 15: Remove the now-unused Config facade import from the top of
FilterAvailableSummitsStrategy.php: locate the use
Illuminate\Support\Facades\Config; statement in the file (used alongside the
FilterAvailableSummitsStrategy class) and delete that import line so the file
only retains necessary use statements.
In `@tests/ProtectedApiTestCase.php`:
- Around line 30-31: The import for GroupsScopes is unused; either remove the
unused import line "use App\Security\GroupsScopes;" or add the appropriate
GroupsScopes constant(s) to the test token scopes in the stub classes. Locate
AccessTokenServiceStub and AccessTokenServiceStub2 in this file and either
append the relevant GroupsScopes::<CONSTANT> value(s) to their scopes arrays
used when building tokens, or simply delete the GroupsScopes import if those
stubs should not include group scopes.
- Around line 82-85: Replace the ad-hoc "$url . '/...'" scope concatenations in
ProtectedApiTestCase.php by introducing and using dedicated scope constant
classes for the missing domains: add PublicCloudsScopes, PrivateCloudsScopes and
ConsultantsScopes (each exposing named constants such as READ) and add
MembersInvitations or extend MembersScopes to provide INVITATIONS_READ and
INVITATIONS_WRITE constants; then update the $scopes array and any other places
currently using "$url . '/public-clouds/read'", "$url . '/private-clouds/read'",
"$url . '/consultants/read'", "$url . '/members/invitations/read'", and "$url .
'/members/invitations/write'" to reference the new constants (e.g.,
PublicCloudsScopes::READ) instead of string concatenation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c6942ce-858e-4b11-87a4-6779f87ac002
📒 Files selected for processing (14)
.github/workflows/l5-swagger-generate.ymlapp/Http/Utils/FilterAvailableSummitsStrategy.phpapp/ModelSerializers/Summit/SummitSerializer.phpapp/Providers/AppServiceProvider.phpapp/Security/CompanyScopes.phpapp/Security/GroupsScopes.phpapp/Security/MemberScopes.phpapp/Security/OrganizationScopes.phpapp/Security/SponsoredProjectScope.phpapp/Security/SummitScopes.phpapp/Security/TeamScopes.phpdatabase/seeders/ApiEndpointsSeeder.phpdatabase/seeders/ApiScopesSeeder.phptests/ProtectedApiTestCase.php
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-519/ This page is automatically updated on each push to this PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes OAuth scope strings across the app by removing sprintf-based realm prefixing and centralizing scope construction around a shared base realm constant. It also extends CI to generate and publish a Swagger UI preview for pull requests.
Changes:
- Replaced
sprintf($scope, $realm)patterns with scope constants that include the base realm (SCOPE_BASE_REALM). - Updated seed/test data to use the new centralized scope constants (and added Teams/Groups scope constants).
- Added a GitHub Actions workflow job to publish Swagger UI previews to GitHub Pages and comment the preview URL on PRs.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ProtectedApiTestCase.php | Switched hardcoded/sprintf scopes to centralized scope constants in protected API test stubs. |
| database/seeders/ApiScopesSeeder.php | Seeds scopes using the new scope constants (no more realm sprintf). |
| app/Security/TeamScopes.php | Introduces centralized Teams scope constants. |
| app/Security/SummitScopes.php | Converts Summit scopes from format strings to realm-prefixed constants. |
| app/Security/SponsoredProjectScope.php | Converts sponsored project scopes to realm-prefixed constants. |
| app/Security/OrganizationScopes.php | Converts organization scopes to realm-prefixed constants. |
| app/Security/MemberScopes.php | Converts member scopes to realm-prefixed constants. |
| app/Security/GroupsScopes.php | Converts groups scope to realm-prefixed constant. |
| app/Security/CompanyScopes.php | Converts company scopes to realm-prefixed constants. |
| app/Providers/AppServiceProvider.php | Defines SCOPE_BASE_REALM from config('app.scope_base_realm') during registration. |
| app/ModelSerializers/Summit/SummitSerializer.php | Uses centralized SummitScopes::ReadAllSummitData for access checks. |
| app/Http/Utils/FilterAvailableSummitsStrategy.php | Uses centralized SummitScopes::ReadAllSummitData for access checks. |
| .github/workflows/l5-swagger-generate.yml | Generates Swagger UI artifact and publishes a per-PR preview to GitHub Pages with PR comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+184
to
+209
| - name: Comment preview URL on PR | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| const prNumber = context.payload.pull_request.number; | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const baseUrl = `https://${owner}.github.io/${repo}`; | ||
| const previewPath = `/openapi/pr-${prNumber}/`; | ||
| const url = `${baseUrl}${previewPath}`; | ||
|
|
||
| const body = [ | ||
| '📘 **OpenAPI / Swagger preview**', | ||
| '', | ||
| `➡️ ${url}`, | ||
| '', | ||
| 'This page is automatically updated on each push to this PR.' | ||
| ].join('\n'); | ||
|
|
||
| await github.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: prNumber, | ||
| body, | ||
| }); |
tests/ProtectedApiTestCase.php
Outdated
| use App\Models\Foundation\Main\IGroup; | ||
| use App\Security\CompanyScopes; | ||
| use App\Security\SponsoredProjectScope; | ||
| use App\Security\GroupsScopes; |
Comment on lines
+125
to
+129
| <link rel="stylesheet" href="https://unpkg.com/swagger-ui-dist/swagger-ui.css" /> | ||
| </head> | ||
| <body> | ||
| <div id="swagger-ui"></div> | ||
| <script src="https://unpkg.com/swagger-ui-dist/swagger-ui-bundle.js"></script> |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/ProtectedApiTestCase.php (1)
194-195:⚠️ Potential issue | 🟡 Minor
SummitScopes::WriteVideoDatais still duplicated here.The fixture grants the same scope twice, so the test token no longer reflects a clean scope set.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ProtectedApiTestCase.php` around lines 194 - 195, The fixture duplicates SummitScopes::WriteVideoData in the granted scopes array, producing a non-unique scope set; update the fixture used in ProtectedApiTestCase by removing the duplicate SummitScopes::WriteVideoData entry (or replace it with the intended missing scope) so the test token reflects a clean, unique set of scopes—look for the array that lists SummitScopes::WriteVideoData twice and ensure each scope appears only once.
🧹 Nitpick comments (2)
.github/workflows/l5-swagger-generate.yml (2)
125-129: Pin the Swagger UI assets.Line 125 and Line 129 pull
swagger-ui-distfrom floatingunpkgURLs, so an upstream release can break this preview without any change in this repo. Pin an explicit version or publish the assets with the artifact.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/l5-swagger-generate.yml around lines 125 - 129, The workflow currently loads Swagger UI assets from floating unpkg URLs in the HTML snippet (the <link rel="stylesheet" href="https://unpkg.com/swagger-ui-dist/swagger-ui.css" /> and <script src="https://unpkg.com/swagger-ui-dist/swagger-ui-bundle.js"></script>), which risks breakage; update these references to pin a specific version (e.g., replace "unpkg.com/swagger-ui-dist/..." with "unpkg.com/swagger-ui-dist@<version>/...") or serve the CSS/JS from a repository artifact or CDN with an immutable URL so the preview cannot be broken by upstream releases.
184-209: Upsert the preview comment instead of creating a new one.Line 204 calls
createCommenton every run, so active PRs will accumulate duplicate preview messages. Prefer updating one sticky bot comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/l5-swagger-generate.yml around lines 184 - 209, Replace the unconditional github.rest.issues.createComment call with logic that first lists existing comments for the PR (use github.rest.issues.listComments with context.repo and context.payload.pull_request.number), search for a bot comment that starts with the preview header text (e.g. '📘 **OpenAPI / Swagger preview**'), and if found call github.rest.issues.update to replace that comment’s body; only fall back to github.rest.issues.createComment when no matching comment exists. Ensure you preserve the assembled body string and reference the existing comment id (comment.id) when calling issues.update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/l5-swagger-generate.yml:
- Around line 149-183: The pages-preview job runs on pull_request events but
will fail for forked PRs because the read-only GITHUB_TOKEN prevents
peaceiris/actions-gh-pages@v4 from pushing; update the job's if condition (job
name: pages-preview) to only run for same-repo PRs by adding a fork check, e.g.
change the existing if: github.event_name == 'pull_request' to include
github.event.pull_request.head.repo.fork == false (or equivalent comparison such
as github.event.pull_request.head.repo.full_name == github.repository) so the
peaceiris/actions-gh-pages@v4 step only executes for non-fork PRs.
In `@database/seeders/ApiScopesSeeder.php`:
- Around line 55-396: The seedSummitScopes() seeder is missing several new
SummitScopes constants so fresh DBs won't get corresponding ApiScope rows;
update the $scopes array in ApiScopesSeeder::seedSummitScopes() to include
entries for SummitScopes::WriteSubmissionInvitations,
SummitScopes::ReadSubmissionInvitations, SummitScopes::ReadBookableRoomsData,
SummitScopes::BookableRoomsReservation, SummitScopes::WriteBookableRoomsData,
SummitScopes::WriteNotifications, SummitScopes::WriteEventTypeData,
SummitScopes::WriteTracksData, SummitScopes::WriteTrackGroupsData,
SummitScopes::WriteRSVPTemplateData,
SummitScopes::WriteSummitSpeakerAssistanceData, and
SummitScopes::WriteTicketTypeData (use consistent short_description/description
patterns used by other entries and reference SummitScopes.php for exact constant
names).
- Around line 510-523: The organization scopes are being attached to the wrong
API because seedOrganizationScopes() resolves the API with ['name' =>
'companies']; update the lookup so $api =
EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name'
=> 'organizations']) (or the exact resource-server name used for organizations)
so OrganizationScopes::ReadOrganizationData and
OrganizationScopes::WriteOrganizationData are persisted against the correct API;
verify subsequent use of $api in this method still assigns scopes to that $api.
---
Duplicate comments:
In `@tests/ProtectedApiTestCase.php`:
- Around line 194-195: The fixture duplicates SummitScopes::WriteVideoData in
the granted scopes array, producing a non-unique scope set; update the fixture
used in ProtectedApiTestCase by removing the duplicate
SummitScopes::WriteVideoData entry (or replace it with the intended missing
scope) so the test token reflects a clean, unique set of scopes—look for the
array that lists SummitScopes::WriteVideoData twice and ensure each scope
appears only once.
---
Nitpick comments:
In @.github/workflows/l5-swagger-generate.yml:
- Around line 125-129: The workflow currently loads Swagger UI assets from
floating unpkg URLs in the HTML snippet (the <link rel="stylesheet"
href="https://unpkg.com/swagger-ui-dist/swagger-ui.css" /> and <script
src="https://unpkg.com/swagger-ui-dist/swagger-ui-bundle.js"></script>), which
risks breakage; update these references to pin a specific version (e.g., replace
"unpkg.com/swagger-ui-dist/..." with "unpkg.com/swagger-ui-dist@<version>/...")
or serve the CSS/JS from a repository artifact or CDN with an immutable URL so
the preview cannot be broken by upstream releases.
- Around line 184-209: Replace the unconditional
github.rest.issues.createComment call with logic that first lists existing
comments for the PR (use github.rest.issues.listComments with context.repo and
context.payload.pull_request.number), search for a bot comment that starts with
the preview header text (e.g. '📘 **OpenAPI / Swagger preview**'), and if found
call github.rest.issues.update to replace that comment’s body; only fall back to
github.rest.issues.createComment when no matching comment exists. Ensure you
preserve the assembled body string and reference the existing comment id
(comment.id) when calling issues.update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad32f831-4cf6-4bb1-9c02-0be36d2c5a9e
📒 Files selected for processing (14)
.github/workflows/l5-swagger-generate.ymlapp/Http/Utils/FilterAvailableSummitsStrategy.phpapp/ModelSerializers/Summit/SummitSerializer.phpapp/Providers/AppServiceProvider.phpapp/Security/CompanyScopes.phpapp/Security/GroupsScopes.phpapp/Security/MemberScopes.phpapp/Security/OrganizationScopes.phpapp/Security/SponsoredProjectScope.phpapp/Security/SummitScopes.phpapp/Security/TeamScopes.phpdatabase/seeders/ApiEndpointsSeeder.phpdatabase/seeders/ApiScopesSeeder.phptests/ProtectedApiTestCase.php
🚧 Files skipped from review as they are similar to previous changes (5)
- app/ModelSerializers/Summit/SummitSerializer.php
- app/Security/SponsoredProjectScope.php
- app/Providers/AppServiceProvider.php
- app/Security/TeamScopes.php
- app/Security/MemberScopes.php
Comment on lines
+149
to
+183
| pages-preview: | ||
| name: Publish Swagger UI to GitHub Pages | ||
| needs: openapi-generate | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest | ||
|
|
||
| permissions: | ||
| contents: write | ||
| issues: write | ||
| pull-requests: write | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Download Swagger UI artifact | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: swagger-ui | ||
| path: swagger-ui | ||
|
|
||
| - name: Build public directory for GitHub Pages | ||
| run: | | ||
| PR_PATH="openapi/pr-${{ github.event.number }}" | ||
| mkdir -p "public/${PR_PATH}" | ||
| cp -R swagger-ui/* "public/${PR_PATH}/" | ||
| echo "Built GitHub Pages content at public/${PR_PATH}" | ||
|
|
||
| - name: Publish to GitHub Pages | ||
| uses: peaceiris/actions-gh-pages@v4 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| publish_dir: ./public | ||
| keep_files: true | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions GITHUB_TOKEN fork pull request read-only permissions
💡 Result:
For workflows triggered by pull_request events from a fork, GitHub deliberately downgrades GITHUB_TOKEN to read-only and does not pass repository secrets (this is not overrideable via permissions: in the workflow). [1][2]
Key implications / options:
- You can’t “grant write” to
GITHUB_TOKENfor fork PR runs onpull_request; GitHub caps it at read-only for safety. [1][2] - If you need privileged automation on PRs (labeling, commenting, etc.), many repos use
pull_request_target, which runs in the base repo context and therefore can have read/writeGITHUB_TOKENand access to secrets unless you restrict it—but this is security-sensitive (don’t run untrusted fork code in that context). [1] - For private/internal repos, there are repository/enterprise settings to allow fork PR workflows and (optionally) “send write tokens” and/or “send secrets” to fork PR workflows. [3]
Sources: GitHub Actions docs on fork PR restrictions and pull_request_target behavior [1], GitHub changelog noting fork PR tokens are always read-only [2], and GitHub settings/policy docs for fork PR workflows in private repos (including “send write tokens”) [3].
References:
[1] (docs.github.com)
[2] (github.blog)
[3] (docs.github.com)
Citations:
- 1: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows?azure-portal=true&utm_source=openai
- 2: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/?utm_source=openai
- 3: https://docs.github.com/en/enterprise-server%403.17/admin/enforcing-policies/enforcing-policies-for-your-enterprise/enforcing-policies-for-github-actions-in-your-enterprise?utm_source=openai
🌐 Web query:
peaceiris/actions-gh-pages write permissions requirements
💡 Result:
To use peaceiris/actions-gh-pages with GITHUB_TOKEN, your workflow/job must grant the token write access to repository contents (so it can push commits/tags to the publishing branch, usually gh-pages). The README explicitly calls out fixing 403 Write access to repository not granted by adding: permissions: contents: write. (github.com)
Example:
jobs:
deploy:
permissions:
contents: write
steps:
- uses: peaceiris/actions-gh-pages@v4
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: ./publicIf you still get “permission denied” errors, also check the repository setting Settings → Actions → General → Workflow permissions and ensure it’s not restricted to read-only (many repos/orgs default to read-only). (stackoverflow.com)
If you deploy to an external repository (external_repository:), GITHUB_TOKEN won’t work; you must use a deploy key or a personal access token instead. (github.com)
Citations:
- 1: https://github.com/peaceiris/actions-gh-pages
- 2: https://github.com/peaceiris/actions-gh-pages
- 3: https://stackoverflow.com/questions/72851548/permission-denied-to-github-actionsbot?utm_source=openai
- 4: https://github.com/peaceiris/actions-gh-pages
Guard the deploy job for same-repo PRs.
pull_request runs from forks receive a read-only GITHUB_TOKEN, which peaceiris/actions-gh-pages@v4 cannot use to push to the repository. This job will fail for external contributors. Add a guard condition to prevent execution on fork PRs:
- if: github.event_name == 'pull_request'
+ if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repositoryIf fork previews are required, use pull_request_target in a separate, trusted workflow instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/l5-swagger-generate.yml around lines 149 - 183, The
pages-preview job runs on pull_request events but will fail for forked PRs
because the read-only GITHUB_TOKEN prevents peaceiris/actions-gh-pages@v4 from
pushing; update the job's if condition (job name: pages-preview) to only run for
same-repo PRs by adding a fork check, e.g. change the existing if:
github.event_name == 'pull_request' to include
github.event.pull_request.head.repo.fork == false (or equivalent comparison such
as github.event.pull_request.head.repo.full_name == github.repository) so the
peaceiris/actions-gh-pages@v4 step only executes for non-fork PRs.
Comment on lines
510
to
523
| private function seedOrganizationScopes(){ | ||
| $current_realm = Config::get('app.scope_base_realm'); | ||
|
|
||
| $api = EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name' => 'companies']); | ||
|
|
||
| $scopes = [ | ||
| [ | ||
| 'name' => sprintf(OrganizationScopes::ReadOrganizationData, $current_realm), | ||
| 'name' => OrganizationScopes::ReadOrganizationData, | ||
| 'short_description' => 'Get Organizations Data', | ||
| 'description' => 'Grants read only access for Organization Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(OrganizationScopes::WriteOrganizationData, $current_realm), | ||
| 'name' => OrganizationScopes::WriteOrganizationData, | ||
| 'short_description' => 'Write Companies Data', | ||
| 'description' => 'Grants write access for Organization Data', |
There was a problem hiding this comment.
Attach organization scopes to the organizations API.
Line 512 still resolves ['name' => 'companies'] inside seedOrganizationScopes(), so both organization scopes are persisted under the wrong resource server.
🛠️ Suggested fix
- $api = EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name' => 'companies']);
+ $api = EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name' => 'organizations']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@database/seeders/ApiScopesSeeder.php` around lines 510 - 523, The
organization scopes are being attached to the wrong API because
seedOrganizationScopes() resolves the API with ['name' => 'companies']; update
the lookup so $api =
EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name'
=> 'organizations']) (or the exact resource-server name used for organizations)
so OrganizationScopes::ReadOrganizationData and
OrganizationScopes::WriteOrganizationData are persisted against the correct API;
verify subsequent use of $api in this method still assigns scopes to that $api.
chore(ci): add swagger preview
smarcet
force-pushed
the
feature/prefixed-const-scopes
branch
from
60225ec to
68ead3c
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-519/ This page is automatically updated on each push to this PR. |
1 similar comment
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-519/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/l5-swagger-generate.yml (1)
149-183:⚠️ Potential issue | 🟠 MajorGuard
pages-previewfor same-repo PRs only.At Line 152, the job runs for all
pull_requestevents. Fork PRs typically cannot push withGITHUB_TOKEN, so publish/comment steps can fail.🔧 Suggested fix
- if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repositoryFor GitHub Actions workflows triggered by pull_request from forks, is GITHUB_TOKEN write access restricted and are repository secrets withheld? Also confirm whether peaceiris/actions-gh-pages requires write permission to push to gh-pages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/l5-swagger-generate.yml around lines 149 - 183, The pages-preview job runs for all pull_request events which fails for forked PRs because GITHUB_TOKEN/secrets and write permissions are restricted; update the pages-preview job (job name pages-preview and the Publish to GitHub Pages step using peaceiris/actions-gh-pages@v4) to run only for same-repo PRs by adding a guard to the job-level if expression that checks both the event is a pull_request and that github.event.pull_request.head.repo.full_name (or equivalently head.repo.owner == github.repository_owner) equals github.repository, so the publish/comment steps only run for PRs originating from the same repository.database/seeders/ApiScopesSeeder.php (1)
510-523:⚠️ Potential issue | 🟠 MajorOrganization scopes are attached to the wrong API.
Line 512 resolves the API with
['name' => 'companies'], but this method seeds organization scopes. The scopes should be attached to theorganizationsAPI resource server instead.🛠️ Suggested fix
- $api = EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name' => 'companies']); + $api = EntityManager::getRepository(\App\Models\ResourceServer\Api::class)->findOneBy(['name' => 'organizations']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/seeders/ApiScopesSeeder.php` around lines 510 - 523, The seedOrganizationScopes function is resolving the wrong API ('companies') before seeding organization scopes; update the EntityManager lookup in seedOrganizationScopes (where it calls EntityManager::getRepository(...)->findOneBy(['name' => 'companies'])) to resolve the 'organizations' API instead so the scopes (OrganizationScopes::ReadOrganizationData, OrganizationScopes::WriteOrganizationData, etc.) are attached to the organizations resource server.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@database/seeders/ApiScopesSeeder.php`:
- Around line 55-396: The seeder's $scopes array in ApiScopesSeeder is missing
several SummitScopes constants (e.g., WriteSubmissionInvitations,
ReadSubmissionInvitations, ReadBookableRoomsData, BookableRoomsReservation,
WriteBookableRoomsData, WriteNotifications, WriteEventTypeData, WriteTracksData,
WriteTrackGroupsData, WriteRSVPTemplateData, WriteSummitSpeakerAssistanceData,
WriteTicketTypeData); add entries for each missing SummitScopes::Constant into
the $scopes array with appropriate short_description and description values
(matching the style of existing entries) so ApiScope rows are created for those
constants when the seeder runs. Ensure each new array element uses the same key
names ('name', 'short_description', 'description') as the other entries in the
$scopes list.
In `@tests/ProtectedApiTestCase.php`:
- Around line 148-151: The scopes array contains a duplicate entry of
SummitScopes::UpdateRegistrationOrders; remove the redundant
SummitScopes::UpdateRegistrationOrders line so each scope is unique (or replace
with the intended missing scope if it was a copy/paste mistake). Locate the
scopes list in ProtectedApiTestCase (where SummitScopes entries are declared)
and delete the second occurrence to deduplicate the array.
- Around line 250-254: AccessTokenServiceStub2 contains a duplicated scope entry
(SummitScopes::UpdateRegistrationOrders) in its scopes list; locate the scopes
array in the AccessTokenServiceStub2 definition inside
tests/ProtectedApiTestCase.php and remove the redundant
SummitScopes::UpdateRegistrationOrders so each scope appears only once, ensuring
the list mirrors the corrected version used in AccessTokenServiceStub.
---
Duplicate comments:
In @.github/workflows/l5-swagger-generate.yml:
- Around line 149-183: The pages-preview job runs for all pull_request events
which fails for forked PRs because GITHUB_TOKEN/secrets and write permissions
are restricted; update the pages-preview job (job name pages-preview and the
Publish to GitHub Pages step using peaceiris/actions-gh-pages@v4) to run only
for same-repo PRs by adding a guard to the job-level if expression that checks
both the event is a pull_request and that
github.event.pull_request.head.repo.full_name (or equivalently head.repo.owner
== github.repository_owner) equals github.repository, so the publish/comment
steps only run for PRs originating from the same repository.
In `@database/seeders/ApiScopesSeeder.php`:
- Around line 510-523: The seedOrganizationScopes function is resolving the
wrong API ('companies') before seeding organization scopes; update the
EntityManager lookup in seedOrganizationScopes (where it calls
EntityManager::getRepository(...)->findOneBy(['name' => 'companies'])) to
resolve the 'organizations' API instead so the scopes
(OrganizationScopes::ReadOrganizationData,
OrganizationScopes::WriteOrganizationData, etc.) are attached to the
organizations resource server.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb4a64ef-558b-40b1-b2a3-fae4941e5b32
📒 Files selected for processing (14)
.github/workflows/l5-swagger-generate.ymlapp/Http/Utils/FilterAvailableSummitsStrategy.phpapp/ModelSerializers/Summit/SummitSerializer.phpapp/Providers/AppServiceProvider.phpapp/Security/CompanyScopes.phpapp/Security/GroupsScopes.phpapp/Security/MemberScopes.phpapp/Security/OrganizationScopes.phpapp/Security/SponsoredProjectScope.phpapp/Security/SummitScopes.phpapp/Security/TeamScopes.phpdatabase/seeders/ApiEndpointsSeeder.phpdatabase/seeders/ApiScopesSeeder.phptests/ProtectedApiTestCase.php
🚧 Files skipped from review as they are similar to previous changes (6)
- app/ModelSerializers/Summit/SummitSerializer.php
- app/Security/GroupsScopes.php
- app/Security/MemberScopes.php
- app/Providers/AppServiceProvider.php
- app/Security/TeamScopes.php
- app/Security/CompanyScopes.php
Comment on lines
55
to
396
| $scopes = [ | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadSummitData, $current_realm), | ||
| 'name' => SummitScopes::ReadSummitData, | ||
| 'short_description' => 'Get Summit Data', | ||
| 'description' => 'Grants read only access for Summits Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadAllSummitData, $current_realm), | ||
| 'name' => SummitScopes::ReadAllSummitData, | ||
| 'short_description' => 'Get All Summits Data', | ||
| 'description' => 'Grants read only access for All Summits Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::MeRead, $current_realm), | ||
| 'name' => SummitScopes::MeRead, | ||
| 'short_description' => 'Get own summit member data', | ||
| 'description' => 'Grants read only access for our own summit member data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::AddMyFavorites, $current_realm), | ||
| 'name' => SummitScopes::AddMyFavorites, | ||
| 'short_description' => 'Allows to add Summit events as favorite', | ||
| 'description' => 'Allows to add Summit events as favorite', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteMyFavorites, $current_realm), | ||
| 'name' => SummitScopes::DeleteMyFavorites, | ||
| 'short_description' => 'Allows to remove Summit events as favorite', | ||
| 'description' => 'Allows to remove Summit events as favorite', | ||
| ], | ||
| // enter/leave event | ||
| [ | ||
| 'name' => sprintf(SummitScopes::EnterEvent, $current_realm), | ||
| 'name' => SummitScopes::EnterEvent, | ||
| 'short_description' => '', | ||
| 'description' => '', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::LeaveEvent, $current_realm), | ||
| 'name' => SummitScopes::LeaveEvent, | ||
| 'short_description' => '', | ||
| 'description' => '', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteMetrics, $current_realm), | ||
| 'name' => SummitScopes::WriteMetrics, | ||
| 'short_description' => '', | ||
| 'description' => '', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadMetrics, $current_realm), | ||
| 'name' => SummitScopes::ReadMetrics, | ||
| 'short_description' => '', | ||
| 'description' => '', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::AddMyRSVP, $current_realm), | ||
| 'name' => SummitScopes::AddMyRSVP, | ||
| 'short_description' => 'Allows to add Summit events as RSVP', | ||
| 'description' => 'Allows to add Summit events as RSVP', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteMyRSVP, $current_realm), | ||
| 'name' => SummitScopes::DeleteMyRSVP, | ||
| 'short_description' => 'Allows to remove Summit events from RSVP', | ||
| 'description' => 'Allows to remove Summit events from RSVP', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::AddMySchedule, $current_realm), | ||
| 'name' => SummitScopes::AddMySchedule, | ||
| 'short_description' => 'Allows to add Summit events to my schedule', | ||
| 'description' => 'Allows to add Summit events to my schedule', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteMySchedule, $current_realm), | ||
| 'name' => SummitScopes::DeleteMySchedule, | ||
| 'short_description' => 'Allows to remove Summit events from my schedule', | ||
| 'description' => 'Allows to remove Summit events from my schedule', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::AddMyScheduleShareable, $current_realm), | ||
| 'name' => SummitScopes::AddMyScheduleShareable, | ||
| 'short_description' => 'Allows create a shareable link from my schedule', | ||
| 'description' => 'Allows create a shareable link from my schedule', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteMyScheduleShareable, $current_realm), | ||
| 'name' => SummitScopes::DeleteMyScheduleShareable, | ||
| 'short_description' => 'Allows to delete shareable links from my schedule', | ||
| 'description' => 'Allows to delete shareable links from my schedule', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::AddMyEventFeedback, $current_realm), | ||
| 'name' => SummitScopes::AddMyEventFeedback, | ||
| 'short_description' => 'Allows to create event feedback', | ||
| 'description' => 'Allows to create event feedback', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteMyEventFeedback, $current_realm), | ||
| 'name' => SummitScopes::DeleteMyEventFeedback, | ||
| 'short_description' => 'Allows to delete event feedback', | ||
| 'description' => 'Allows to delete event feedback', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::SendMyScheduleMail, $current_realm), | ||
| 'name' => SummitScopes::SendMyScheduleMail, | ||
| 'short_description' => 'Allows to send my schedule share email', | ||
| 'description' => 'Allows to send my schedule share email', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteSummitData, $current_realm), | ||
| 'name' => SummitScopes::WriteSummitData, | ||
| 'short_description' => 'Write Summit Data', | ||
| 'description' => 'Grants write access for Summits Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteRegistrationData, $current_realm), | ||
| 'name' => SummitScopes::WriteRegistrationData, | ||
| 'short_description' => 'Write Registration Data', | ||
| 'description' => 'Grants write access for Registration Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteEventData, $current_realm), | ||
| 'name' => SummitScopes::WriteEventData, | ||
| 'short_description' => 'Write Summit Events', | ||
| 'description' => 'Grants write access for Summits Events', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePresentationData, $current_realm), | ||
| 'name' => SummitScopes::WritePresentationData, | ||
| 'short_description' => 'Write Summit Presentations', | ||
| 'description' => 'Grants write access for Summits Presentations', | ||
| ], | ||
| array( | ||
| 'name' => sprintf('%s/summits/delete-event', $current_realm), | ||
| [ | ||
| 'name' => SummitScopes::DeleteEventData, | ||
| 'short_description' => 'Delete Summit Events', | ||
| 'description' => 'Grants delete access for Summits Events', | ||
| ), | ||
| array( | ||
| 'name' => sprintf('%s/summits/publish-event', $current_realm), | ||
| ], | ||
| [ | ||
| 'name' => SummitScopes::PublishEventData, | ||
| 'short_description' => 'Publish/UnPublish Summit Events', | ||
| 'description' => 'Grants Publish/UnPublish access for Summits Events', | ||
| ), | ||
| ], | ||
| [ | ||
| 'name' => sprintf('%s/summits/read-external-orders', $current_realm), | ||
| 'name' => SummitScopes::ReadSummitsConfirmExternalOrders, | ||
| 'short_description' => 'Allow to read External Orders', | ||
| 'description' => 'Allow to read External Orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf('%s/summits/confirm-external-orders', $current_realm), | ||
| 'name' => SummitScopes::WriteSummitsConfirmExternalOrders, | ||
| 'short_description' => 'Allow to confirm External Orders', | ||
| 'description' => 'Allow to confirm External Orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf('%s/summits/write-videos', $current_realm), | ||
| 'name' => SummitScopes::WriteVideoData, | ||
| 'short_description' => 'Allow to write presentation videos', | ||
| 'description' => 'Allow to write presentation videos', | ||
| ], | ||
| [ | ||
| 'name' => sprintf('%s/summits/read-notifications', $current_realm), | ||
| 'name' => SummitScopes::ReadNotifications, | ||
| 'short_description' => 'Allow to read summit notifications', | ||
| 'description' => 'Allow to read summit notifications', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteSpeakersData, $current_realm), | ||
| 'name' => SummitScopes::WriteSpeakersData, | ||
| 'short_description' => 'Write Speakers Data', | ||
| 'description' => 'Grants write access for Speakers Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadSpeakersData, $current_realm), | ||
| 'name' => SummitScopes::ReadSpeakersData, | ||
| 'short_description' => 'Read Speakers Data', | ||
| 'description' => 'Grants read access for Speakers Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteMySpeakersData, $current_realm), | ||
| 'name' => SummitScopes::WriteMySpeakersData, | ||
| 'short_description' => 'Write My Speakers Profile Data', | ||
| 'description' => 'Grants write access for My Speaker Profile Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadMySpeakersData, $current_realm), | ||
| 'name' => SummitScopes::ReadMySpeakersData, | ||
| 'short_description' => 'Read My Speakers Profile Data', | ||
| 'description' => 'Grants read access for My Speaker Profile Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteAttendeesData, $current_realm), | ||
| 'name' => SummitScopes::WriteAttendeesData, | ||
| 'short_description' => 'Write Attendees Data', | ||
| 'description' => 'Grants write access for Attendees Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePromoCodeData, $current_realm), | ||
| 'name' => SummitScopes::WritePromoCodeData, | ||
| 'short_description' => 'Write Promo Codes Data', | ||
| 'description' => 'Grants write access for Promo Codes Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteLocationsData, $current_realm), | ||
| 'name' => SummitScopes::WriteLocationsData, | ||
| 'short_description' => 'Write Summit Locations Data', | ||
| 'description' => 'Grants write access for Summit Locations Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteLocationBannersData, $current_realm), | ||
| 'name' => SummitScopes::WriteLocationBannersData, | ||
| 'short_description' => 'Write Summit Location Banners Data', | ||
| 'description' => 'Grants write access for Summit Location Banners Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteTrackTagGroupsData, $current_realm), | ||
| 'name' => SummitScopes::WriteTrackTagGroupsData, | ||
| 'short_description' => 'Write Summit Track Tag Groups Data', | ||
| 'description' => 'Grants write access for Summit Track Tag Groups Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteTrackQuestionTemplateData, $current_realm), | ||
| 'name' => SummitScopes::WriteTrackQuestionTemplateData, | ||
| 'short_description' => 'Write Summit Track Question Template Data', | ||
| 'description' => 'Grants write access for Summit Track Question Template Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePresentationVideosData, $current_realm), | ||
| 'name' => SummitScopes::WritePresentationVideosData, | ||
| 'short_description' => 'Write Summit Presentation Videos Data', | ||
| 'description' => 'Grants write access for Summit Presentation Videos Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePresentationSlidesData, $current_realm), | ||
| 'name' => SummitScopes::WritePresentationSlidesData, | ||
| 'short_description' => 'Write Summit Presentation Slides Data', | ||
| 'description' => 'Grants write access for Summit Presentation Slides Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePresentationLinksData, $current_realm), | ||
| 'name' => SummitScopes::WritePresentationLinksData, | ||
| 'short_description' => 'Write Summit Presentation Links Data', | ||
| 'description' => 'Grants write access for Summit Presentation Links Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePresentationMaterialsData, $current_realm), | ||
| 'name' => SummitScopes::WritePresentationMaterialsData, | ||
| 'short_description' => 'Write Summit Presentation Materials Data', | ||
| 'description' => 'Grants write access for Summit Materials Links Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadMyBookableRoomsReservationData, $current_realm), | ||
| 'name' => SummitScopes::ReadMyBookableRoomsReservationData, | ||
| 'short_description' => 'Read my bookable rooms reservations', | ||
| 'description' => 'Read my bookable rooms reservations', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteMyBookableRoomsReservationData, $current_realm), | ||
| 'name' => SummitScopes::WriteMyBookableRoomsReservationData, | ||
| 'short_description' => 'Write my bookable rooms reservations', | ||
| 'description' => 'Write my bookable rooms reservations', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::CreateOfflineRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::CreateOfflineRegistrationOrders, | ||
| 'short_description' => 'Create summit offline registration orders', | ||
| 'description' => 'Create summit offline registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::CreateRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::CreateRegistrationOrders, | ||
| 'short_description' => 'Create summit registration orders', | ||
| 'description' => 'Create summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::DeleteRegistrationOrders, | ||
| 'short_description' => 'Delete summit registration orders', | ||
| 'description' => 'Delete summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DeleteMyRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::DeleteMyRegistrationOrders, | ||
| 'short_description' => 'Delete my summit registration orders', | ||
| 'description' => 'Delete my summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadMyRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::ReadMyRegistrationOrders, | ||
| 'short_description' => 'Read my summit registration orders', | ||
| 'description' => 'Read my summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::ReadRegistrationOrders, | ||
| 'short_description' => 'Read summit registration orders', | ||
| 'description' => 'Read summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::UpdateRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::UpdateRegistrationOrders, | ||
| 'short_description' => 'Update summit registration orders', | ||
| 'description' => 'Update summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::UpdateMyRegistrationOrders, $current_realm), | ||
| 'name' => SummitScopes::UpdateMyRegistrationOrders, | ||
| 'short_description' => 'Update my summit registration orders', | ||
| 'description' => 'Update my summit registration orders', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::UpdateRegistrationOrdersBadges, $current_realm), | ||
| 'name' => SummitScopes::UpdateRegistrationOrdersBadges, | ||
| 'short_description' => 'Update summit registration orders badges', | ||
| 'description' => 'Update summit registration orders badges', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::PrintRegistrationOrdersBadges, $current_realm), | ||
| 'name' => SummitScopes::PrintRegistrationOrdersBadges, | ||
| 'short_description' => 'print summit registration orders badges', | ||
| 'description' => 'print summit registration orders badges', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadMyBadgeScan, $current_realm), | ||
| 'name' => SummitScopes::ReadMyBadgeScan, | ||
| 'short_description' => 'read my badge scans', | ||
| 'description' => 'read my badge scans', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadBadgeScanValidate, $current_realm), | ||
| 'name' => SummitScopes::ReadBadgeScanValidate, | ||
| 'short_description' => 'validate badge scan', | ||
| 'description' => 'validate badge scan', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteMyBadgeScan, $current_realm), | ||
| 'name' => SummitScopes::WriteMyBadgeScan, | ||
| 'short_description' => 'allow to share my badge with sponsors', | ||
| 'description' => 'allow to share my badge with sponsors', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadBadgeScan, $current_realm), | ||
| 'name' => SummitScopes::ReadBadgeScan, | ||
| 'short_description' => 'read badge scans', | ||
| 'description' => 'read badge scans', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteBadgeScan, $current_realm), | ||
| 'name' => SummitScopes::WriteBadgeScan, | ||
| 'short_description' => 'write badge scans', | ||
| 'description' => 'write badge scans', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadPaymentProfiles, $current_realm), | ||
| 'name' => SummitScopes::ReadPaymentProfiles, | ||
| 'short_description' => 'read summit payment profiles', | ||
| 'description' => 'read summit payment profiles', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WritePaymentProfiles, $current_realm), | ||
| 'name' => SummitScopes::WritePaymentProfiles, | ||
| 'short_description' => 'write summit payment profiles', | ||
| 'description' => 'write summit payment profiles', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteRegistrationInvitations, $current_realm), | ||
| 'name' => SummitScopes::WriteRegistrationInvitations, | ||
| 'short_description' => 'write summit registration invitation', | ||
| 'description' => 'write summit registration invitation', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadRegistrationInvitations, $current_realm), | ||
| 'name' => SummitScopes::ReadRegistrationInvitations, | ||
| 'short_description' => 'read summit registration invitation', | ||
| 'description' => 'read summit registration invitation', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadMyRegistrationInvitations, $current_realm), | ||
| 'name' => SummitScopes::ReadMyRegistrationInvitations, | ||
| 'short_description' => 'read my summit registration invitation', | ||
| 'description' => 'read my summit registration invitation', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::DoVirtualCheckIn, $current_realm), | ||
| 'name' => SummitScopes::DoVirtualCheckIn, | ||
| 'short_description' => 'Allow virtual Check In', | ||
| 'description' => 'Allow virtual Check In', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::Allow2PresentationAttendeeVote, $current_realm), | ||
| 'name' => SummitScopes::Allow2PresentationAttendeeVote, | ||
| 'short_description' => 'Allow Attendee Vote on Presentation', | ||
| 'description' => 'Allow Attendee Vote on Presentation', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::ReadAttendeeNotesData, $current_realm), | ||
| 'name' => SummitScopes::ReadAttendeeNotesData, | ||
| 'short_description' => 'Read Attendee Notes Data', | ||
| 'description' => 'Grants read access for Attendee Notes Data', | ||
| ], | ||
| [ | ||
| 'name' => sprintf(SummitScopes::WriteAttendeeNotesData, $current_realm), | ||
| 'name' => SummitScopes::WriteAttendeeNotesData, | ||
| 'short_description' => 'Write Attendee Notes Data', | ||
| 'description' => 'Grants write access for Attendee Notes Data', | ||
| ] |
There was a problem hiding this comment.
Several SummitScopes constants are missing from the seeder.
The following constants defined in app/Security/SummitScopes.php are not seeded, which means fresh databases won't have ApiScope rows for them:
WriteSubmissionInvitationsReadSubmissionInvitationsReadBookableRoomsDataBookableRoomsReservationWriteBookableRoomsDataWriteNotificationsWriteEventTypeDataWriteTracksDataWriteTrackGroupsDataWriteRSVPTemplateDataWriteSummitSpeakerAssistanceDataWriteTicketTypeData
Consider adding these to the $scopes array to ensure completeness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@database/seeders/ApiScopesSeeder.php` around lines 55 - 396, The seeder's
$scopes array in ApiScopesSeeder is missing several SummitScopes constants
(e.g., WriteSubmissionInvitations, ReadSubmissionInvitations,
ReadBookableRoomsData, BookableRoomsReservation, WriteBookableRoomsData,
WriteNotifications, WriteEventTypeData, WriteTracksData, WriteTrackGroupsData,
WriteRSVPTemplateData, WriteSummitSpeakerAssistanceData, WriteTicketTypeData);
add entries for each missing SummitScopes::Constant into the $scopes array with
appropriate short_description and description values (matching the style of
existing entries) so ApiScope rows are created for those constants when the
seeder runs. Ensure each new array element uses the same key names ('name',
'short_description', 'description') as the other entries in the $scopes list.
Comment on lines
+148
to
+151
| SummitScopes::UpdateRegistrationOrders, | ||
| SummitScopes::CreateOfflineRegistrationOrders, | ||
| SummitScopes::DeleteRegistrationOrders, | ||
| SummitScopes::UpdateRegistrationOrders, |
There was a problem hiding this comment.
Duplicate scope entry: SummitScopes::UpdateRegistrationOrders appears twice.
Lines 148 and 151 both contain SummitScopes::UpdateRegistrationOrders. This is likely a copy-paste error and should be deduplicated.
🐛 Proposed fix
SummitScopes::UpdateRegistrationOrders,
SummitScopes::CreateOfflineRegistrationOrders,
SummitScopes::DeleteRegistrationOrders,
- SummitScopes::UpdateRegistrationOrders,
SummitScopes::UpdateMyRegistrationOrders,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SummitScopes::UpdateRegistrationOrders, | |
| SummitScopes::CreateOfflineRegistrationOrders, | |
| SummitScopes::DeleteRegistrationOrders, | |
| SummitScopes::UpdateRegistrationOrders, | |
| SummitScopes::UpdateRegistrationOrders, | |
| SummitScopes::CreateOfflineRegistrationOrders, | |
| SummitScopes::DeleteRegistrationOrders, | |
| SummitScopes::UpdateMyRegistrationOrders, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ProtectedApiTestCase.php` around lines 148 - 151, The scopes array
contains a duplicate entry of SummitScopes::UpdateRegistrationOrders; remove the
redundant SummitScopes::UpdateRegistrationOrders line so each scope is unique
(or replace with the intended missing scope if it was a copy/paste mistake).
Locate the scopes list in ProtectedApiTestCase (where SummitScopes entries are
declared) and delete the second occurrence to deduplicate the array.
Comment on lines
+250
to
+254
| SummitScopes::UpdateRegistrationOrders, | ||
| SummitScopes::UpdateMyRegistrationOrders, | ||
| SummitScopes::CreateOfflineRegistrationOrders, | ||
| SummitScopes::DeleteRegistrationOrders, | ||
| SummitScopes::UpdateRegistrationOrders, |
There was a problem hiding this comment.
Duplicate scope entry: SummitScopes::UpdateRegistrationOrders appears twice in AccessTokenServiceStub2.
Lines 250 and 254 both contain SummitScopes::UpdateRegistrationOrders. Same issue as in AccessTokenServiceStub.
🐛 Proposed fix
SummitScopes::UpdateRegistrationOrders,
SummitScopes::UpdateMyRegistrationOrders,
SummitScopes::CreateOfflineRegistrationOrders,
SummitScopes::DeleteRegistrationOrders,
- SummitScopes::UpdateRegistrationOrders,
SummitScopes::WriteBadgeScan,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SummitScopes::UpdateRegistrationOrders, | |
| SummitScopes::UpdateMyRegistrationOrders, | |
| SummitScopes::CreateOfflineRegistrationOrders, | |
| SummitScopes::DeleteRegistrationOrders, | |
| SummitScopes::UpdateRegistrationOrders, | |
| SummitScopes::UpdateRegistrationOrders, | |
| SummitScopes::UpdateMyRegistrationOrders, | |
| SummitScopes::CreateOfflineRegistrationOrders, | |
| SummitScopes::DeleteRegistrationOrders, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ProtectedApiTestCase.php` around lines 250 - 254,
AccessTokenServiceStub2 contains a duplicated scope entry
(SummitScopes::UpdateRegistrationOrders) in its scopes list; locate the scopes
array in the AccessTokenServiceStub2 definition inside
tests/ProtectedApiTestCase.php and remove the redundant
SummitScopes::UpdateRegistrationOrders so each scope appears only once, ensuring
the list mirrors the corrected version used in AccessTokenServiceStub.
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.
ref: https://app.clickup.com/t/86b8x25q0
Summary by CodeRabbit
New Features
Chores