feat(iam): admit protected routes with route-local Permission guards - #65
Conversation
- Clarified the role of Router-level Authentication and Permission guards in access management. - Removed references to deprecated API bindings in the MenuRecord interface. - Enhanced error handling documentation to reflect changes in authentication and authorization processes. - Updated frontend tests to align with the new structure and removed obsolete API binding checks. - Cleaned up Cargo.toml and Cargo.lock by removing unused dependencies.
- Eliminated the implementation of From trait for iam::access::CatalogError to AppError, streamlining error handling in the mappings module.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39d0d69293
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| next.policy.set_watcher(Box::new(watcher)); | ||
| } | ||
| *self.policy.write().await = next; | ||
| *self.snapshot.write().await = next; |
There was a problem hiding this comment.
When a periodic or Redis-triggered reload reads User/Role status, then a concurrent set_user_status or set_role_status updates the active snapshot before this assignment, the older candidate replaces that newer state here. Disabling a User or Role can therefore leave its previous permissions active until the next periodic reload (30 seconds in the server), because self-originated notifications are ignored. Serialize local status mutations with reloads or reject stale candidates before swapping them.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
| .await?; | ||
| snapshot.users.remove(&user_id); | ||
| drop(snapshot); | ||
| self.notify_reload(); |
There was a problem hiding this comment.
In a multi-process deployment, this is the last reload notification emitted for account deletion, but Accounts::delete does not delete the authoritative sys_users row until after remove_user returns. A peer that handles this notification in that interval reloads the User as enabled, and no later notification corrects it, so an existing token remains admitted to Self-Service routes until periodic repair. Emit a final notification after the User row has been deleted.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
…oad notifications - Added reload lock handling in user and role removal methods to ensure consistency during updates. - Updated the Accounts service to notify reload after user deletion. - Introduced a public notify_reload method in the Authorization service for better encapsulation of reload logic. - Improved documentation in the IAM architecture to clarify the reload mechanism and its impact on policy freshness.
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- Updated the user deletion method to return an error if the specified user ID does not exist in the database. - Enhanced error handling to improve robustness in the Accounts service.
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…pdates - Introduced tests to verify that the `reload` and `replace_user_roles` functions respect the `mutation_lock`, ensuring that concurrent updates do not overwrite changes. - Added checks to confirm that role assignments and permission mutations remain visible in the active snapshot after the lock is released. - Removed outdated tests related to concurrent reloads that were previously in the `authorization.rs` file.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58eab3fd35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Updated the EnforcementEngine to clarify that network I/O operations occur outside the mutation critical section, ensuring that only snapshot/watcher installations are serialized. - Added a comment to enhance code readability regarding the mutation lock's role in the context of Redis interactions.
1 participant
Summary
permission(code, MethodRouter). The Access Catalog no longer stores or exposes HTTP method/path bindings, and Router construction no longer looks them up.sys_menu_apis(migrations/0003_drop_menu_api_bindings.sql). Self-Service stays authenticated but unguarded. Remove the unusedCatalogError → AppErrormapping so catalog tree errors stay on init (IamInitError) and Role Access input (INVALID_ROLE_ACCESS).Test plan
0003_drop_menu_api_bindings.sql(DROP TABLE sys_menu_apis)401 LOGIN_REQUIRED; authenticated405stays wrapped; unknown path stays404GET /api/users/me) and is denied on management routes (403 PERMISSION_DENIED)GET/HEADshare the same guard; denial audit records the actual request pathsys_audit_eventsstill returns403and does not override the denial500 INTERNAL_SERVER_ERROR, handler not called)cargo fmt --all -- --checkcargo test --workspacecd apps/desktop && pnpm test