Skip to content

Conversation

chris1984
Copy link
Member

@chris1984 chris1984 commented Sep 12, 2024

What are the changes introduced in this pull request?

  • When trying to query smart proxies with curl or hammer on a user that is not an admin, the view_smart_proxies permission is needed from Foreman, we did not have this before and would only list Katello permissions. We hard code view_smart_proxies here when getting the proxies from Foreman
    @smart_proxies = SmartProxy.with_content.authorized(:view_smart_proxies).includes(:features).
{"message":"Access denied","details":"Missing one of the required permissions: manage_capsule_content","missing_permissions":["manage_capsule_content"]}
}

Considerations taken when implementing this change?

  • Make sure you can still see Smart Proxies

What are the testing steps for this pull request?

  • Create a user with a role that only has view_capsule_content and manage_capsule_content
  • Try to curl and see the error message curl -k -u unauth:unauth https://localhost/katello/api/capsules/
  • Apply PR
  • Try the curl again and see if it shows the missing view_smart_proxies permission
  • Apply the permission and see if curl completes without an error

Copy link
Contributor

@vsedmik vsedmik left a comment

Choose a reason for hiding this comment

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

Works good:

$ curl -k -u unauth:unauth https://satellite.redhat.com/katello/api/capsules/
{
  "error": {"message":"Access denied","details":"Missing one of the required permissions: view_smart_proxy, manage_capsule_content, view_capsule_content","missing_permissions":["view_smart_proxy","manage_capsule_content","view_capsule_content"]}
}
$ curl -k -u unauth:unauth https://satellite.redhat.com/katello/api/capsules/2
{
  "error": {"message":"Access denied","details":"Missing one of the required permissions: view_smart_proxy, manage_capsule_content, view_capsule_content","missing_permissions":["view_smart_proxy","manage_capsule_content","view_capsule_content"]}
}

(unauth is an user with insufficient permissions)

Copy link
Contributor

@vsedmik vsedmik left a comment

Choose a reason for hiding this comment

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

Should we go plural?

chris1984

This comment was marked as outdated.

Copy link
Contributor

@vsedmik vsedmik left a comment

Choose a reason for hiding this comment

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

Looks we are good now. Thanks for the patch!

Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

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

Ack.. 🎉

@chris1984 chris1984 merged commit 640ef71 into Katello:master Sep 16, 2024
27 checks passed
@chris1984 chris1984 deleted the fix-capsuleperms branch September 16, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants