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.

Sign up for GitHub

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

Jump to bottom

effective user with non-ascii password #13840

Merged
merged 1 commit into from
Apr 18, 2024
Merged

effective user with non-ascii password #13840

merged 1 commit into from
Apr 18, 2024

Conversation

pondrejk
Copy link
Contributor

Problem Statement

covers bz# 2258968

Solution

testing rex with effective user with non-ascii password

@pondrejk pondrejk 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.15.z Introduced in or relating directly to Satellite 6.15 labels Jan 19, 2024
@pondrejk pondrejk self-assigned this Jan 19, 2024
@pondrejk pondrejk requested a review from a team as a code owner January 19, 2024 14:30
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@pondrejk
Copy link
Contributor Author

local result against patched stream

pytest tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]
======================================== test session starts ========================================

collected 1 item                                                                                    

tests/foreman/cli/test_remoteexecution.py .                                                   [100%]

@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@pondrejk pondrejk marked this pull request as draft January 22, 2024 09:37
@pondrejk
Copy link
Contributor Author

pondrejk commented Jan 22, 2024 •
edited
Loading

moving to draft until the tested changes appear in stream

Gauravtalreja1
Gauravtalreja1 reviewed Feb 19, 2024
View reviewed changes
tests/foreman/cli/test_remoteexecution.py
"""
client = rex_contenthost
# create a user on client via remote job
username = gen_string('alpha')
password = gen_string('cjk')
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
password = gen_string('cjk')
password = gen_string('alpha')

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 covered bz 2258968 asks for using something nonASCII as a password...

tests/foreman/cli/test_remoteexecution.py
@@ -187,6 +190,7 @@ def test_positive_run_job_effective_user(self, rex_contenthost, module_target_sa
'inputs': f"command=touch /home/{username}/{filename}",
'search-query': f"name ~ {client.hostname}",
'effective-user': f'{username}',
'effective-user-password': f'{password}',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, rex_contenthost is already registered with GR method which configures REX as default for root user with ssh-key based auth, so just wondering if we could add some assertions here to verify if the file is created by an effective user only, like file owner or group owner, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, I believe I'm doing that few lines bellow for user, I added a check for group as well.

lhellebr
lhellebr reviewed Feb 26, 2024
View reviewed changes
tests/foreman/cli/test_remoteexecution.py Outdated
filename = gen_string('alpha')
make_user_job = module_target_sat.cli_factory.job_invocation(
{
'job-template': 'Run Command - Script Default',
'inputs': f"command=useradd -m {username}",
'inputs': f"command=useradd -m {username} -p {password} -G wheel",
Copy link
Contributor

Choose a reason for hiding this comment

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

Given running a job worked previously without the password specified (Using sudo?), are we positive that the password is really being used in this new version?

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 made some changes to reduce the possibility of secret sudo, but the only way I could find to be 100% sure is a negative assert -- running rex without pwd specified, so I added that as well

@pondrejk pondrejk force-pushed the eff-user-with-pwd branch 2 times, most recently from 72c118d to 291b7ec Compare March 14, 2024 11:09
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@pondrejk pondrejk force-pushed the eff-user-with-pwd branch from 291b7ec to c07ff59 Compare March 26, 2024 14:57
@pondrejk
Copy link
Contributor Author

Awaiting stream snap 51, at theat point we should get passing prt

@pondrejk pondrejk removed the AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing label Mar 27, 2024
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@pondrejk pondrejk added 6.15.z Introduced in or relating directly to Satellite 6.15 and removed 6.15.z Introduced in or relating directly to Satellite 6.15 labels Mar 27, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6217
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8] --external-logging
Test Result : ================== 1 failed, 4 warnings in 1091.27s (0:18:11) ==================

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Mar 27, 2024
@pondrejk pondrejk force-pushed the eff-user-with-pwd branch from c07ff59 to 571ac93 Compare March 27, 2024 15:33
@pondrejk pondrejk added the AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing label Mar 27, 2024
@Satellite-QE Satellite-QE removed the PRT-Failed Indicates that latest PRT run is failed for the PR label Mar 27, 2024
@pondrejk pondrejk marked this pull request as ready for review March 27, 2024 15:35
@pondrejk pondrejk force-pushed the eff-user-with-pwd branch from 571ac93 to 7645dc8 Compare March 27, 2024 15:35
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6224
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8] --external-logging
Test Result : ================== 1 passed, 4 warnings in 1130.62s (0:18:50) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Mar 27, 2024
@pondrejk
Copy link
Contributor Author

@lhellebr @Gauravtalreja1 prt now passing, re-review welcome

@pondrejk pondrejk force-pushed the eff-user-with-pwd branch from 7645dc8 to 5fe8212 Compare April 8, 2024 07:28
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 8, 2024
@pondrejk pondrejk enabled auto-merge (squash) April 15, 2024 07:00
@pondrejk pondrejk disabled auto-merge April 15, 2024 07:01
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6522
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8] --external-logging
Test Result : ================= 1 passed, 13 warnings in 1111.73s (0:18:31) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 16, 2024
@pondrejk
Copy link
Contributor Author

prt passed but some urelated problem made robotelo-runner status red, discusedd in automation channel. Retriggering prt in hope that all will be well again

@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8]

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6555
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_remoteexecution.py::TestRemoteExecution::test_positive_run_job_effective_user[rhel8] --external-logging
Test Result : ================= 1 passed, 13 warnings in 1095.98s (0:18:15) ==================

@pondrejk pondrejk force-pushed the eff-user-with-pwd branch from 5fe8212 to b5a3fca Compare April 18, 2024 13:52
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 18, 2024
@jameerpathan111 jameerpathan111 merged commit 1e68738 into SatelliteQE:master Apr 18, 2024
9 checks passed
@Satellite-QE Satellite-QE mentioned this pull request Apr 18, 2024
Closed
github-actions bot pushed a commit that referenced this pull request Apr 18, 2024
@pondrejk @web-flow
effective user with non-ascii password (#13840)
b9000d5 
(cherry picked from commit 1e68738)
jyejare pushed a commit to jyejare/robottelo that referenced this pull request Oct 19, 2024
@pondrejk @jyejare
effective user with non-ascii password (SatelliteQE#13840)
b6202d8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gauravtalreja1 Gauravtalreja1 Gauravtalreja1 left review comments

@lhellebr lhellebr lhellebr left review comments

@Griffin-Sullivan Griffin-Sullivan Griffin-Sullivan approved these changes

@jameerpathan111 jameerpathan111 jameerpathan111 approved these changes

Assignees

@pondrejk pondrejk

Labels
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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pondrejk @Satellite-QE @lhellebr @jameerpathan111 @Gauravtalreja1 @Griffin-Sullivan