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
Fixes #31836 - Unclear error message for cvv promotion with bad perrms #9153
Merged
ianballou
merged 1 commit into
Katello:master
from
ianballou:31836-fix-promote-bad-perms-error
Feb 17, 2021
Merged
Fixes #31836 - Unclear error message for cvv promotion with bad perrms #9153
ianballou
merged 1 commit into
Katello:master
from
ianballou:31836-fix-promote-bad-perms-error
Feb 17, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issues: #31836 |
ianballou
force-pushed
the
31836-fix-promote-bad-perms-error
branch
from
February 10, 2021 16:29
7435577
to
399f9cd
Compare
ianballou
force-pushed
the
31836-fix-promote-bad-perms-error
branch
from
February 10, 2021 21:17
399f9cd
to
0ade011
Compare
ianballou
force-pushed
the
31836-fix-promote-bad-perms-error
branch
from
February 15, 2021 19:37
0ade011
to
85839d0
Compare
chris1984
approved these changes
Feb 17, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK, works good:
Before:
[vagrant@hammer ~]$ hammer -u test -p changeme content-view version promote --content-view-id 2 --organization-id 1 --to-lifecycle-environment-id 1
Could not promote the content view:
Could not find content_view_version resource with id 2
With patch:
[vagrant@hammer ~]$ hammer -u test -p changeme content-view version promote --content-view-id 2 --organization-id 1 --to-lifecycle-environment-id 1
Could not promote the content view:
Could not find content_view_version resource with id 2. Potential missing permissions: promote_or_remove_content_views, promote_or_remove_content_views_to_environments
chris1984
added
Ready for merge
and removed
Needs testing
Not yet reviewed
Waiting on contributor
labels
Feb 17, 2021
jturel
pushed a commit
to jturel/katello
that referenced
this pull request
Mar 4, 2021
Katello#9153) (cherry picked from commit 1bba15f)
jjeffers
pushed a commit
to jjeffers/katello
that referenced
this pull request
Mar 15, 2021
jjeffers
pushed a commit
that referenced
this pull request
Mar 16, 2021
For future reference, the weird permissions relationship between lifecycle environments and content views is rooted here: https://github.com/Katello/katello/blob/master/app/models/katello/authorization/content_view.rb#L24 |
jturel
pushed a commit
to jturel/katello
that referenced
this pull request
Apr 30, 2021
…ote a content view with a user that has insufficient permissions Fixes #31836 - Unclear error message for cvv promotion with bad perrms (Katello#9153) (cherry picked from commit 1bba15f)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings back the error message that reports which permissions could be missing that went away with #8956
The root of the issue here is that
promote_or_remove_content_views_to_environments
has a special relationship topromote_or_remove_content_views
. The whole permissions model seems to expect permissions to be separate and not have relationships, which isn't always the case. The PR here does seem like a bit of a workaround, but it also seems to be the least complicated fix at least.A better long-term solution may be to formally declare these special relationships somehow. I'm not sure if it's worth the time to do that here.
I'm curious to hear if there are ideas about better ways to solve the issue. I'm not sure how this change will interact with other permissions so I'm marking it a draft for now.
To test: