Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add negative test for dashboard RBAC on Model Registry #2039

Conversation

lugi0
Copy link
Contributor

@lugi0 lugi0 commented Nov 15, 2024

No description provided.

@lugi0 lugi0 added new test New test(s) added (PR will be listed in release-notes) do not merge Do not merge this yet please labels Nov 15, 2024
@lugi0 lugi0 self-assigned this Nov 15, 2024
Copy link
Contributor

github-actions bot commented Nov 15, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
573 0 0 573 100

@lugi0
Copy link
Contributor Author

lugi0 commented Nov 19, 2024

Verified in rhoai-test-flow/1567

@lugi0 lugi0 added verified This PR has been tested with Jenkins and removed do not merge Do not merge this yet please labels Nov 19, 2024
Launch Dashboard ${TEST_USER_3.USERNAME} ${TEST_USER_3.PASSWORD} ${TEST_USER_3.AUTH_TYPE}
... ${ODH_DASHBOARD_URL} ${BROWSER.NAME} ${BROWSER.OPTIONS}
Open Model Registry Dashboard Page allowed_user=${FALSE}
SeleniumLibrary.Page Should Contain Request access to model registries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but I wanted to surface it as an explicit step in this TC just for clarity when going through the logs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm okay, although the step will not run in case of unallowed user cos it would fail in the previous keyword.

Maybe an alternative approach would be to split the permission check from the keyword to open the Registry UI page.

Example

Open Model Registry Dashboard Page
User Should Not Be Allowed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes more sense to have it defined this way because if the user is allowed we need to refresh the page in case the permissions have not been reconciled yet, and so the keyword will still need to be passed some variable to define if the user is allowed or not.
Once you have that information in the keyword what makes the most sense IMHO is to have that logic defined in the keyword

@lugi0 lugi0 merged commit 4bc622f into red-hat-data-services:master Nov 20, 2024
8 checks passed
lenahorsley pushed a commit to lenahorsley/ods-ci that referenced this pull request Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new test New test(s) added (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants