gitea icon indicating copy to clipboard operation
gitea copied to clipboard

OIDC provider

Open sorenisanerd opened this issue 2 years ago • 10 comments

This adds support for a Github Actions compatible OIDC provider.

For more info, see: https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect

It depends on a few changes in act and act_runner: https://gitea.com/gitea/act_runner/pulls/272 https://gitea.com/gitea/act/pulls/73

sorenisanerd avatar Jul 04 '23 00:07 sorenisanerd

Still work-in-progress. I have a few more things I want to test before it's ready.

sorenisanerd avatar Jul 04 '23 00:07 sorenisanerd

I think this is ready for review, but obviously it's useless without the act and act_runner changes.

sorenisanerd avatar Jul 06 '23 17:07 sorenisanerd

@sorenisanerd Thank you for your work, I have to say you did a great job, good idea, good code.

However, this featrue has been blocked some trivial things.

  • Where to keep the code for parsing permissions? Gitea, the forked act, the upstream act, or using actionlint indead?
  • The permissions field is only only for OIDC, but the permissions granted to the GITHUB_TOKEN/GITEA_TOKEN (probably more often), personally, I think the latter should be supported first.
  • Is it possible for contributors to send a PR that updates the permissions field and runs a malicious script?

You know, once permissions are involved, we need to be checked more carefully. Please give us some time to discuss and decide.

wolfogre avatar Jul 07 '23 04:07 wolfogre

Thank you for your kind comments :)

I'm very open to porting this to gitea or actionlinter or wherever. I don't have any strong opinions on it.

I agree it would have made more sense to first use permissions to adjust the capabilities of the GitHub token. I happened to need this oidc-provider functionality, so that's what I focused on. I included methods for merging permissions as well as definitions matching GitHub's defaults so I or someone else could add that later.

Great point about how permissions are affected when permissions apply for pull requests. Is that supposed to be safeguarded by the need to approve workflow runs for new contributors? I honestly don't know what GitHub does.

sorenisanerd avatar Jul 08 '23 07:07 sorenisanerd

I've pushed changes to the act and act_runner PR's on gitea.com. The resulting diffs are very modest. The rest of the functionality has been pulled into Gitea here.

Is this approach more agreeable? I'll add some tests once we're happy with the overall approach.

sorenisanerd avatar Jul 10 '23 04:07 sorenisanerd

I rebased this and reworked it into a set of more reviewable commits. If you like, I can submit them as individual PR's, but each one depends on the previous ones, which makes it a little awkward to manage as separate PR's.

sorenisanerd avatar Aug 10 '23 22:08 sorenisanerd

Nothing new, just rebased and folded https://github.com/go-gitea/gitea/commit/ded82c30abd033695d7a6edae9a1295f792063a0 (the formatting changes based on the linting errors) into the relevant patches.

sorenisanerd avatar Sep 16 '23 01:09 sorenisanerd

I took the liberty of renaming my migration to just permissions.go. At this point, I must have had to rename it 5-7 times locally and it's getting kinda annoying. Once this gets closer to being approved, I'll try to pick a number for it.

sorenisanerd avatar Sep 16 '23 20:09 sorenisanerd

I'm also working on a more complete permissions implementation that should probably be merged before this one, so I'm setting this one back to WIP. I expect the permissions patch to be ready some time this week.

sorenisanerd avatar Sep 19 '23 23:09 sorenisanerd

Hello, are there any updates on this? is there something a contributor can help out with? Would love to see this move along

punmechanic avatar May 08 '24 01:05 punmechanic