:+1: seems reasonable to me.

Doesn't this mean we'll have to create double (or more) mappings for all permissions? One for the user who should be able to apply the waiver, and another for each service that might act as a proxy for that user (Bodhi, Errata Tool, CVP, CPaaS, etc). This seems like a lot of extra maintenance for little extra security.

I agree with Mike. Additionally if we merge this, "superusers" then should have a rule that says that they can waive everything, or it gets pretty hard to maintain the rules for the superusers too. Like in the case of bodhi, bodhi should have permission for everything, because potentially users can request for so many different waivers... that becomes basically every waiver.
And if we put such rule this code is pretty useless.
I see how that can become not maintainable once greenwave, and consequently waiverdb, gets more popular with time.

This PR is my reaction to: https://gitlab.cee.redhat.com/devops/factory2-openshift-templates/merge_requests/273

The user in SUPERUSERS list cannot waive anything if it hasn't permission to do so in PERMISSION_MAPPING.

Maybe I misunderstood something.

Allowing web sites to submit any waiver for anything in the permission mapping does not seem like a good idea to me. It makes the mapping bit redundant.

This PR is my reaction to: https://gitlab.cee.redhat.com/devops/factory2-openshift-templates/merge_requests/273

The user in SUPERUSERS list cannot waive anything if it hasn't permission to do so in PERMISSION_MAPPING.

Maybe I misunderstood something.

I replied earlier today on IRC about that and in the other PR. I guess I didn't explain myself.

Allowing web sites to submit any waiver for anything in the permission mapping does not seem like a good idea to me. It makes the mapping bit redundant.

I don't understand why it's such a big deal now that we have no control at all. This feature it's there since waiverdb exists. We are increasing the security here, not decreasing it.

I replied earlier today on IRC about that and in the other PR. I guess I didn't explain myself.

Do you mean following?

I mean that the superuser cannot waive anything for him/herself if he/she doesn't have permission.

Superuser can just look into permission mapping and pick other user who can waive the test case.

I don't understand why it's such a big deal now that we have no control at all. This feature it's there since waiverdb exists. We are increasing the security here, not decreasing it.

This was only used by Bodhi and when there was no permission mapping. If I can exploit web site which can do this I might be also able to waive anything. This can be mitigated by using the permission mapping for superusers.

But I'm OK with adding the new superuser if it's temporary workaround for bad CORS headers. The we can drop this PR ... SUPERUSER should be used only sparingly.

I understand the desire to not give a service access to do anything, but that's kind of what the idea of a SUPERUSER is. Assuming we do merge this, could we create a meaningful policy that would improve security? For example, Bodhi would essentially need to be able to waive anything, right? Are there other SUPERUSERS that should only be able to waive a subset of test cases? Could we prevent SUPERUSERs from waiving security-related tests?

Assuming we do merge this, could we create a meaningful policy that would improve security?

https://gitlab.cee.redhat.com/devops/factory2-openshift-templates/merge_requests/273#note_663604

For example, Bodhi would essentially need to be able to waive anything, right?

Not sure if this is happening but if it's really needed, then it could be:

PERMISSION_MAPPING = {
    r'':  {'users': ['bodhi']},
}

I replied earlier today on IRC about that and in the other PR. I guess I didn't explain myself.

Do you mean following?

I mean that the superuser cannot waive anything for him/herself if he/she doesn't have permission.

What I meant with that sentence is that if you run this as a superuser (let's say bodhi) and the superuser doesn't have an entry in PERMISSION_MAPPING, but user "pippo" does have permission in PERMiSSION_MAPPING then:
waiverdb-cli --username=pippo
this works.

But if you run:
waiverdb
it doesn't work.

Instead it seems to me what you understood from my sentence is that permission mapping gets checked for both "bodhi" and "pippo".
I'm just explaining the sentence here, not trying to say this is right or wrong.

Superuser can just look into permission mapping and pick other user who can waive the test case.

I assume we trust "superuser" if it is in that list, otherwise we wouldn't put it/he/she there.
The same can happen if my (or your) own account gets compromised, there's no difference.

This was only used by Bodhi and when there was no permission mapping. If I can exploit web site which can do this I might be also able to waive anything. This can be mitigated by using the permission mapping for superusers.

But the point here is that if they should be able to act as "any" user, they should have a rule that says they have permission on everything. And that makes this code useless.

Closing. Probably not needed since SUPERUSERS solution is temporary.