[DRAFT] feat: [UIE-9051] - IAM RBAC block non-beta route access - #12644
[DRAFT] feat: [UIE-9051] - IAM RBAC block non-beta route access#12644abailly-akamai wants to merge 4 commits into
Conversation
|
|
||
| if (location.pathname === '/iam') { | ||
| navigate({ to: '/iam/users' }); | ||
| } |
There was a problem hiding this comment.
Default tab redirect (doing this one at the route level can lead to infinite loops)
| if (!isIAMEnabled) { | ||
| return <NotFound />; | ||
| } | ||
|
|
There was a problem hiding this comment.
Previous handling which we can now get rid of
| iamUsersRoute, | ||
| iamUsersCatchAllRoute, | ||
| iamRolesCatchAllRoute, | ||
| ]), |
There was a problem hiding this comment.
also cleaned up the route organization 👍
| router.update({ | ||
| context: { | ||
| accountSettings, | ||
| isIAMEnabled, |
There was a problem hiding this comment.
adding this here as well as we're close to merging the main router (we're still using the migration one)
| params: { username }, | ||
| replace: true, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
This redirect solution at the route level is a bit more robust than what was put forward in #12643
They achieve more or less the same but this way is less bug prone and more so the "right" way to do it
There was a problem hiding this comment.
Nice, thanks @abailly-akamai! I like the router based approach better than embedding code. I've noticed an inconsistency with hard links/refresh compared to click navigation.
router.mov
There was a problem hiding this comment.
Ah yeah... I see what is happening. The issue is that we need to make use of beforeLoad as async. When doing a navigation call we already have the true value of isIAMEnabled, but on an initial page load beforeLoad will not wait for our profile and permissions queries to resolve before making a decision since they are happening during hydration. So we need to instead await those queries at the route level, with a custom promise since we can't use a hook in the route definition.
It works now, and this gives us a decent util to manage our redirects in the future (if more are to come) to but I understand this increases the complexity (managing the logic in two places) a bit so I won't be offended if we want to go your route as well. Welcoming suggestions, and up to you!
There was a problem hiding this comment.
Awesome thanks! Ya I like blocking at the route #12656
Cloud Manager UI test results🔺 3 failing tests on test run #3 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/cloudpulse/timerange-verification.spec.ts,cypress/e2e/core/linodes/linode-storage.spec.ts,cypress/e2e/core/account/restricted-user-details-pages.spec.ts" |
|||||||||||||||||||||||
3 participants
Description 📝
Highlight the Pull Request's context and intentions.
Changes 🔄
List any change(s) relevant to the reviewer.
Scope 🚢
Upon production release, changes in this PR will be visible to:
Target release date 🗓️
Please specify a release date (and environment, if applicable) to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.
Preview 📷
Include a screenshot
<img src="" />or video<video src="" />of the change.🔒 Use the Mask Sensitive Data setting for security.
💡 For changes requiring multiple steps to validate, prefer a video for clarity.
How to test 🧪
Prerequisites
(How to setup test environment)
Reproduction steps
(How to reproduce the issue, if applicable)
Verification steps
(How to verify changes)
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅