PR#1730: allow tag or target permissions as appropriate -

koji

#1730 allow tag or target permissions as appropriate

Closed Oct 29, 2019 by tkopecek. Opened Oct 25, 2019 by cobrien.

cobrien/koji issue_1729 into koji-1.18-updates

cobrien commented Oct 25, 2019

I went the simple route here and set the check to be either tag/target or admin and updated the error message to match.

It may be more correct to just completely remove the permission check and allow the hub to take care of failing. But since the check exists, I figured this was the minimal path.

Fixes #1729

cobrien commented Oct 25, 2019

If you would like me to instead just rip out the client-side permission checks (in order for the hub to be the one to make the entire decision) just let me know and I'll update.

cobrien commented Oct 25, 2019

I note that this does break some unit tests that are expecting a particular error string when a permission isn't met. I can look at those as well if you'd like and you're happy with the double-check change. Just let me know.

tkopecek commented Oct 25, 2019

Yes, this is the behaviour we wanted. I've wrongly assumed, that hasPerm works more as assertPerm, so that's the reason. If tests are fixed, I'm +1 for merge.

1 new commit added

test for appropriate hasPerm calls and error messages

Oct 25, 2019

cobrien commented Oct 25, 2019

Tests pass now.

tkopecek commented Oct 29, 2019

:thumbsup:

tkopecek commented Oct 29, 2019

Closing in favour of #1733

Pull-Request has been closed by tkopecek

Oct 29, 2019

Metadata

Assignee

None

Tags

No Tags

koji

#1730 allow tag or target permissions as appropriate Closed Oct 29, 2019 by tkopecek. Opened Oct 25, 2019 by cobrien. cobrien/koji issue_1729 into koji-1.18-updates

Metadata

#1730 allow tag or target permissions as appropriate

Closed Oct 29, 2019 by tkopecek. Opened Oct 25, 2019 by cobrien.

cobrien/koji issue_1729 into koji-1.18-updates