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 support for invalidating users JWTs #17465

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

Conversation

shweta83
Copy link
Contributor

Problem Statement

Add support for Invalidate JWT feature : SAT-27385

Solution

Added support for it.

Related Issues

SatelliteQE/airgun#1721

@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 January 30, 2025 11:18
@shweta83 shweta83 requested a review from a team as a code owner January 30, 2025 11:18
:CaseImportance: High
"""

org = module_org
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you create a separate variable instead of directly using module_org.name?

location=[module_location],
organization=[org],
password=password,
login=admin_username,
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a new variable, you can add it gen_string('alpha') here directly. And later, you can invoke them using admin_user.login.

location=[module_location],
organization=[org],
password=password,
login=non_admin_username,
Copy link
Contributor

Choose a reason for hiding this comment

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

same here gen_string('alpha')

admin_username = gen_string('alpha')
non_admin_username = gen_string('alpha')
password = gen_string('alpha')
roles = [module_target_sat.api.Role().create()]
Copy link
Contributor

Choose a reason for hiding this comment

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

You can take it directly instead of using a list.
roles = module_target_sat.api.Role().create()

@@ -307,6 +308,88 @@ def test_positive_create_product_with_limited_user_permission(
assert newsession.product.search(product_name)[0]['Name'] == product_name


@pytest.mark.rhel_ver_match('8')
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing tier decorator

:id: be328fd7-b640-4080-9373-25f96ba2aef6

:steps:
1. Create an admin user and an non-admin user with "edit_users" and "view_users" permissions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there also be a test for a user without edit_users and view_users permissions invalidating their own JWT token?

'password': password,
}
role = module_target_sat.cli.Role.info({'name': 'Register hosts'})
module_target_sat.cli.User.add_role({'id': non_admin_user.id, 'role-id': role['id']})
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the role while creating the user?

result = session.login.logout()

session.login.login(login_details)
session.user.invalidate_jwt(admin_user.login)
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't assert anything after invalidating admin's token.

activation_keys=[ak.name],
force=True,
)
assert result.status == 1, f'Failed to register host: {result.stderr}'
Copy link
Contributor

Choose a reason for hiding this comment

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

There is probably a wrong error message when this fails. Also, it may be worth it checking more than just status.

session.organization.select(org.name)
session.location.select(module_location.name)
user_cfg = user_nailgun_config(non_admin_user, password)
result = rhel_contenthost.api_register(
Copy link
Contributor

Choose a reason for hiding this comment

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

I may be missing something here. Where do you generate the JWT? Does api_register do it? How come it doesn't do the same, i.e. generate a new JWT, when you use it the second time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants