:thumbsup:

Can you split this out into its own commit?

Can you split this into its own commit?

@ngompa done

Did I get the access control correct? My assumption here is that access control is enforced by Fedora Messaging. If this assumption is erroneous, then this code contains a security vulnerability.

I don't think Fedora Messaging enforces any access control at the moment, besides being allowed to connect to the bus or not. Any publisher inside the infra can publish to any topic (we want to change that but it hasn't been done).

@abompard Does that mean that any fedmsg publisher can cause any package to be signed, or is there additional authentication? I could add a check that the metadata signing request was somehow signed by koji.

I think so. This issue is mitigated by the fact that only Fedora Infra applications can publish to the bus (you need a certificate that we provide).

I planned to improve this situation by adding topic authorization, but the ticket has been blocked for a while. I'll try to get it going, but doing this before the end of the year would be very unadvisable as I'm sure we don't have the exhaustive list of what topics apps are publishing to.

In that case I think this PR is good to go. It has a security check to ensure that it cannot be abused to sign packages.

Can you please rebase the PR so we can get it merged?

done

@abompard Can you please merge this?

Now that everything looks good, it should be good to merge. :thumbsup: