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

Add Hammer support for invalidating users JWTs #17468

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Hammer support for invalidating users JWTs #17468

wants to merge 1 commit into from

Conversation

shweta83
Copy link
Contributor

Problem Statement

Hammer support for invalidating JWTs : SAT-30385

Solution

Added support for JWT invalidate in CLI

Related Issues

@shweta83 shweta83 added No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master labels Jan 30, 2025
@shweta83 shweta83 requested a review from a team as a code owner January 30, 2025 12:14
amolpati30
amolpati30 requested changes Feb 5, 2025
View reviewed changes
robottelo/cli/user.py Outdated Show resolved Hide resolved
@shweta83 shweta83 force-pushed the jwt_invalidate_hammer branch from b166c15 to 770cb4a Compare February 5, 2025 09:48
@shweta83 shweta83 requested a review from a team as a code owner February 5, 2025 09:48
@shweta83 shweta83 force-pushed the jwt_invalidate_hammer branch 3 times, most recently from 3de9987 to 3938ea8 Compare February 5, 2025 09:52
@shweta83 shweta83 requested a review from girijaasoni February 5, 2025 09:55
@shweta83 shweta83 force-pushed the jwt_invalidate_hammer branch 3 times, most recently from 09bb698 to 7c0d2de Compare February 5, 2025 10:24
@shweta83
Copy link
Contributor Author

shweta83 commented Feb 5, 2025

trigger: test-robottelo
pytest: tests/foreman/cli/test_user.py -k test_positive_invalidating_users_tokens

jameerpathan111
jameerpathan111 requested changes Feb 5, 2025
View reviewed changes
tests/foreman/cli/test_user.py

:Verifies: SAT-30385
"""
admin_login = gen_string('alpha')
Copy link
Contributor

Choose a reason for hiding this comment

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

@shweta83 do you think it's possible to parameterize the test for admin and non-admin user? I see some code repetition in the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that would be better, it's somewhat messy as it is. It would be nice to clearly separate the test cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was going to do it. Updated

tests/foreman/cli/test_user.py Outdated
assert f'Successfully invalidated registration tokens for {admin_login}' in result

# Re-register the host with invalidated token
result = rhel_contenthost.execute(cmd.strip('\n'))
Copy link
Contributor

@jameerpathan111 jameerpathan111 Feb 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Don't we need to properly unregister/clean the host first so we can try re-registration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I added that.

tests/foreman/cli/test_user.py Outdated

# Invalidate JWTs for multiple users
result = module_target_sat.cli.User.with_user(admin_login, password).invalidate_multiple(
options={'search': f"id ^ ({admin_user['id']}, {non_admin_user['id']})"}
Copy link
Contributor

Choose a reason for hiding this comment

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

we already invalidate token foradmin_login so is this alright? I suppose it won't hurt but is this what you wanted to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is intentional.

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 10069
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_user.py -k test_positive_invalidating_users_tokens --external-logging
Test Result : =========== 25 deselected, 34 warnings, 1 error in 642.05s (0:10:42) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Feb 5, 2025
@shweta83 shweta83 force-pushed the jwt_invalidate_hammer branch from 7c0d2de to 70008b4 Compare February 5, 2025 13:49
@shweta83
Copy link
Contributor Author

shweta83 commented Feb 5, 2025

trigger: test-robottelo
pytest: tests/foreman/cli/test_user.py -k test_positive_invalidating_users_tokens

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 10079
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_user.py -k test_positive_invalidating_users_tokens --external-logging
Test Result : ========= 2 passed, 25 deselected, 134 warnings in 2358.64s (0:39:18) ==========

@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 Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@girijaasoni girijaasoni Awaiting requested review from girijaasoni

@amolpati30 amolpati30 Awaiting requested review from amolpati30

@Gauravtalreja1 Gauravtalreja1 Awaiting requested review from Gauravtalreja1

@jameerpathan111 jameerpathan111 Awaiting requested review from jameerpathan111

@lhellebr lhellebr Awaiting requested review from lhellebr

Assignees

@shweta83 shweta83

Labels
No-CherryPick PR doesnt need CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR Stream Introduced in or relating directly to Satellite Stream/Master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shweta83 @Satellite-QE @lhellebr @jameerpathan111 @Gauravtalreja1 @amolpati30