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

non-admin user acess with usergroup #15489

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

amolpati30
Copy link
Contributor

@amolpati30 amolpati30 commented Jun 23, 2024

Problem Statement

With user which is non-admin but it is in usergroup which should give him administrator role is possible to access lot of pages, but giving Permission denied

Solution

It verifies if a non-admin user in the user group can access the WebUI.

Dependent PR: SatelliteQE/airgun#1461

@amolpati30 amolpati30 added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Jun 23, 2024
@amolpati30 amolpati30 requested a review from a team as a code owner June 23, 2024 18:39
@amolpati30 amolpati30 force-pushed the non-admin_permission branch 2 times, most recently from 8387a61 to 5a8daf2 Compare June 23, 2024 19:01
tests/foreman/ui/test_ansible.py Outdated Show resolved Hide resolved
Copy link
Contributor

@lhellebr lhellebr left a comment

Choose a reason for hiding this comment

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

ACK after fixing Gaurav's good comments

u_name = gen_string('alpha')
ug_name = gen_string('alpha')
password = gen_string('alpha')
user = target_sat.api.User(login=u_name, password=password, admin=False).create()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use gen_string('alpha') here directly instead of u_name and use user.login in test below? and same for usergroup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used gen_string('alpha') directly instead of creating a new variable for user and usergroup.

wait_for(lambda: session.browser.refresh(), timeout=5)
ansible_roles_table = session.host_new.get_ansible_roles(target_sat.hostname)
assert ansible_roles_table[0]['Name'] == SELECTED_ROLE
request.addfinalizer(user.delete)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you move this finalizer to the top immediately after user is created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After creating the user, the finalizer was used

ansible_roles_table = session.host_new.get_ansible_roles(target_sat.hostname)
assert ansible_roles_table[0]['Name'] == SELECTED_ROLE
request.addfinalizer(user.delete)
assert user.login == u_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert user.login == u_name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this line.

ug_name = gen_string('alpha')
password = gen_string('alpha')
user = target_sat.api.User(login=u_name, password=password, admin=False).create()
target_sat.api.UserGroup(name=ug_name, user=[user], admin=True).create()
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we verify is user is part of usergroup after usergroup is created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assertion is added that user is part of usergroup

Comment on lines 547 to 548
result = target_sat.cli.Auth.logout()
assert 'Logged out' in result
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we remove this as UI session will login with created user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this unnecessary logout line

@@ -519,6 +519,43 @@ def test_positive_assign_and_remove_ansible_role_to_hostgroup(
session.hostgroup.delete(name)
assert not target_sat.api.HostGroup().search(query={'search': f'name={name}'})

@pytest.mark.tier3
def test_positive_non_admin_access(
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we change this test name to something meaningful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test name has been changed.

@Gauravtalreja1 Gauravtalreja1 added the QETestCoverage Issues and PRs relating to a Satellite bug label Jul 9, 2024
@amolpati30 amolpati30 force-pushed the non-admin_permission branch from fb88a4f to f069dd2 Compare July 9, 2024 12:03
@amolpati30 amolpati30 requested a review from a team as a code owner July 9, 2024 12:03
@amolpati30 amolpati30 requested a review from Gauravtalreja1 July 9, 2024 12:23
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_ansible.py -k test_positive_non_admin_user_access_with_usergroup
airgun: 1461

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7678
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_ansible.py -k test_positive_non_admin_user_access_with_usergroup --external-logging
Test Result : ====== 1 passed, 25 deselected, 36 warnings, 1 error in 677.62s (0:11:17) ======

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Jul 9, 2024
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_non_admin_user_access_with_usergroup
airgun: 1461

@amolpati30
Copy link
Contributor Author

"trigger": "test-robottelo"
"pytest": "-k 'test_positive_non_admin_user_access_with_usergroup' tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt"
"airgun": "1461"

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7743
Build Status: SUCCESS
PRT Comment: pytest /opt/app-root/src/robottelo/tests/foreman -v -m build_sanity --external-logging
Test Result : ======= 12 passed, 5518 deselected, 5629 warnings in 1512.40s (0:25:12) ========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Jul 13, 2024
@devendra104
Copy link
Member

"trigger": "test-robottelo"
"pytest": "-k 'test_positive_non_admin_user_access_with_usergroup' tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt"
"airgun": "1461"

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7745
Build Status: SUCCESS
PRT Comment: pytest /opt/app-root/src/robottelo/tests/foreman -v -m build_sanity --external-logging
Test Result : ======= 12 passed, 5518 deselected, 5627 warnings in 1491.40s (0:24:51) ========

@Gauravtalreja1 Gauravtalreja1 merged commit 070b1dc into SatelliteQE:master Jul 15, 2024
12 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
Bump tenacity from 8.4.2 to 8.5.0 (#15587)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 070b1dc)
github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
Bump tenacity from 8.4.2 to 8.5.0 (#15587)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 070b1dc)
Gauravtalreja1 pushed a commit that referenced this pull request Jul 16, 2024
non-admin user acess with usergroup (#15489)

Bump tenacity from 8.4.2 to 8.5.0 (#15587)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 070b1dc)

Co-authored-by: amolpati30 <[email protected]>
jyejare pushed a commit to jyejare/robottelo that referenced this pull request Oct 19, 2024
Bump tenacity from 8.4.2 to 8.5.0 (SatelliteQE#15587)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR QETestCoverage Issues and PRs relating to a Satellite bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants