Conversation
…e checks
Security:
- /api/users/{user}/... routes acted on the authenticated caller instead of
the user in the URL (Authenticatable type-hint can't be route-bound), so
any holder of assign-roles could grant themselves super-admin. The
controller now resolves {user} against the configured user model, returns
404 for unknown IDs, and 404s cross-tenant targets for tenant callers.
Breaking:
- POST assign endpoints and AuthorizationService::assignRolesToUser /
assignPermissionsToUser now add instead of replace. New PUT routes and
syncRolesForUser / syncPermissionsForUser replace.
- hasRole / hasAnyRole / hasAllRoles are literal for super-admins; the bypass
remains in permission checks, Gate, middleware, and AuthorizationService.
- KeystonePermission::forTenant() returns only that tenant's rows and respects
the tenant scope, matching KeystoneRole::forTenant().
Fixed:
- role:/permission: middleware throw AuthenticationException for guests instead
of redirecting to an undefined login route.
- Permission cache is cleared automatically on permission create/update/delete;
removed dead per-user cache clearing.
- PermissionRegistrarTest updated for the keystone.permissions.all.v2 key.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add relation generics to HasKeystone, KeystoneRole, and KeystonePermission so role/permission collections are typed; replace higher-order proxies with typed closures. - RolePermissionController::resolveUser() returns Model&Authenticatable and responses read attributes via getKey()/getAttribute(). - RoleService::getUserRoles() / PermissionService::getUserPermissions() load through the relation methods. - Commands: drop impossible null-coalesce/ternary fallbacks; resolveGuard() only reads --guard when the command defines it. - Analyse app/ alongside src/ so the HasKeystone trait is checked in context; document tenant_id on the test-app User model. - Fix guest-redirect test ordering broken by Laravel 13.33 installing its default login redirect when the HTTP kernel is resolved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In multi-tenant mode, names passed to assignment methods were resolved in the authenticated caller's tenant scope. A tenant-less caller (global admin, console command, queued job) could attach another tenant's same-named role or permission to a user, and a name existing both in-tenant and globally resolved arbitrarily. Raised in PR #2 review (discussion r4134379038). Names now resolve in the tenant of whatever receives them (user, role, or permission): the receiver's own tenant row wins over a global row, a tenant-less receiver matches only globals, and unmatched names throw ModelNotFoundException. Model instances are used as given; single-tenant installs are unchanged. Adds KeystoneRole/KeystonePermission::findByNameForTenant(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ctions Addresses PR #2 review (discussions r4134378963, r4134379102). - RolePermissionController user actions accept Model|string. An app-level Route::model/Route::bind('user') previously handed the controller a model that was coerced to its JSON string and 404'd on lookup. Bound users are now used as given; the cross-tenant 404 check still applies. - KeystonePermission clears the registrar cache immediately and again after the surrounding transaction commits or rolls back, so neither a concurrent refill from pre-commit rows nor an in-transaction fill of rolled-back rows survives for the cache TTL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new release date is incorrectly set one year in the future.
Review effort: Balanced
Findings: None
What changed in this PR
Documents the upcoming 0.11.0 release, including authorization changes, security fixes, and migration guidance.
Changes:
- Adds release notes for role, permission, API, and cache changes.
- Provides migration guidance for breaking changes.
| File | Description |
|---|---|
CHANGELOG.md |
Adds the 0.11.0 changelog entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Uh oh!
There was an error while loading. Please reload this page.