Skip to content
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

OIDC provider #33945

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

OIDC provider #33945

wants to merge 16 commits into from

Conversation

scubbo
Copy link

@scubbo scubbo commented Mar 20, 2025

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


The above is the summary of the original PR that this sprang from. It looks like that one (which has been open for nearly 2 years) has lost steam after the original contributor got frustrated with delays, so I'm hoping that opening a fresh PR will revitalize the effort.

To be extremely clear:

  • All the content in this PR at the time it was opened was written by someone else, not me
  • I have a reasonable understanding of both OIDC (as a user rather than an implementer/maintainer) and of GoLang (using both in my day job), but am by no means an expert in either. I'm taking this on because I'm very keen to see it move along (so that I can authenticate to Vault on a per-repo basis), not because I'm necessarily the right person for the job. I'll happily take guidance and learn as I go, but you should probably assume that I'll have to do some research before understanding any comments.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 20, 2025
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 20, 2025
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/migrations modifies/dependencies labels Mar 20, 2025
@scubbo scubbo mentioned this pull request Mar 20, 2025
@@ -0,0 +1,221 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this file is actually needed - in the original PR there was just a small change to its logic, but it looks like the file has been entirely deleted in the main branch of the repo. I think it can be deleted entirely, but I'll keep it around until either a) I can get more confidence in that position, or b) someone more knowledgeable about the codebase can confirm that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been moved in a recent PR. Maybe you could try to find when it is deleted. https://stackoverflow.com/questions/6839398/find-when-a-file-was-deleted-in-git

@scubbo
Copy link
Author

scubbo commented Mar 20, 2025

Hmm - I'm getting error:

no required module provides package code.gitea.io/gitea/modules/context; to add it:
	go get code.gitea.io/gitea/modules/context

but when I run that command, I get

go: code.gitea.io/gitea/modules/context: no matching versions for query "upgrade"

I'm guessing that some names changed between the time this code was originally written and now. I'll keep poking around!

@wxiaoguang
Copy link
Contributor

It is code.gitea.io/gitea/services/context now. services

This will not build until [this
PR](https://gitea.com/gitea/act/pulls/73) (which adds
`SingleWorkflow.RawPermissions`) is merged.
@scubbo
Copy link
Author

scubbo commented Mar 20, 2025

Note that building will still fail until this PR is merged, since it adds a RawPermissions struct-field that this depends on. Operating on the assumption that OP won't mind if I take that one over (and resolve conflicts), I'll do so now - since that one was already approved, hopefully it shouldn't take long to get the second version of it approved and merged!

EDIT: re-opened here.

@scubbo
Copy link
Author

scubbo commented Mar 20, 2025

I re-opened the PRs on act and act_runner which are dependencies of this one. Hopefully 🤞🏻 those should be smooth to merge (since it looks like the originals were already approved), at which point I can iron out the rest of the build/lint failures. Thanks for the assistance/attention so far! 🙏🏻

@scubbo scubbo requested a review from lunny March 20, 2025 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/dependencies modifies/go Pull requests that update Go code modifies/migrations size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants