mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-30 06:02:22 +00:00
feat: enable auth to git HTTP via authorized integrations (#12715)
Allow authentication to git HTTP & git LFS via an authorized integration.
This is the first step in getting rid of OAuth, basic auth, etc.'s usage of [`isGitRawOrAttachPath(req)`](26f18a94ee/services/auth/method/basic.go (L38-L40)). I don't want to follow that pattern of HTTP route matching in the authentication method, so I've broken the HTTP routes related to git functionality out to using a separate authentication middleware in the top-level `web.Routes` handler. As this approach is expanded to the other endpoints in order to add support to them for authorized integrations, eventually it will be possible to remove this URL matching completely and just rely on middleware installation.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests for Go changes
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12715
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
Mathieu Fenniak
Mathieu Fenniak
parent
af412159ce
commit
c8d24ff06a
5 changed files with 138 additions and 72 deletions
|
|
@ -83,7 +83,7 @@ func init() {
|
|||
// {
|
||||
// "rules": [{
|
||||
// "claim": "sub",
|
||||
// "comparison": "eq",
|
||||
// "compare": "eq",
|
||||
// "value": "repo:forgejo/website:pull_request"
|
||||
// }]
|
||||
// }
|
||||
|
|
|
|||
|
|
@ -164,8 +164,9 @@ func (r *Route) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
|||
}
|
||||
|
||||
// NotFound defines a handler to respond whenever a route could not be found.
|
||||
func (r *Route) NotFound(h http.HandlerFunc) {
|
||||
r.R.NotFound(h)
|
||||
func (r *Route) NotFound(h ...any) {
|
||||
middlewares, handlerFunc := r.wrapMiddlewareAndHandler(h)
|
||||
r.R.With(middlewares...).NotFound(handlerFunc)
|
||||
}
|
||||
|
||||
// Combo delegates requests to Combo
|
||||
|
|
|
|||
|
|
@ -101,12 +101,8 @@ func optionsCorsHandler() func(next http.Handler) http.Handler {
|
|||
}
|
||||
}
|
||||
|
||||
// The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored
|
||||
// in the session (if there is a user id stored in session other plugins might return the user
|
||||
// object for that id).
|
||||
//
|
||||
// The Session plugin is expected to be executed second, in order to skip authentication
|
||||
// for users that have already signed in.
|
||||
// Authentication methods that are applied to all the web URL routes. They are processed in the order defined; an
|
||||
// earlier authentication success would prevent later authentication methods from being attempted.
|
||||
func buildAuthGroup() *auth_method.Group {
|
||||
group := auth_method.NewGroup()
|
||||
group.Add(&auth_method.OAuth2{}) // FIXME: this should be removed and only applied in download and oauth related routers
|
||||
|
|
@ -126,6 +122,23 @@ func buildAuthGroup() *auth_method.Group {
|
|||
return group
|
||||
}
|
||||
|
||||
// Authentication methods that are applied to Git HTTP & Git LFS HTTP routes. They are processed in the order defined;
|
||||
// an earlier authentication success would prevent later authentication methods from being attempted.
|
||||
func buildGitAuthGroup() *auth_method.Group {
|
||||
group := auth_method.NewGroup()
|
||||
group.Add(&auth_method.OAuth2{})
|
||||
group.Add(&auth_method.Basic{})
|
||||
group.Add(&auth_method.AccessToken{})
|
||||
group.Add(&auth_method.ActionRuntimeToken{})
|
||||
group.Add(&auth_method.ActionTaskToken{})
|
||||
group.Add(&auth_method.AuthorizedIntegration{
|
||||
// "Authorization: Basic ..." is easier to use for git operations, and already supported for other tokens, so it
|
||||
// is enabled for Authorized Integrations as well:
|
||||
PermitBasic: true,
|
||||
})
|
||||
return group
|
||||
}
|
||||
|
||||
func webAuth(authMethod auth_service.Method) func(*context.Context) {
|
||||
return func(ctx *context.Context) {
|
||||
output := common.AuthShared(ctx.Base, ctx.Session, authMethod)
|
||||
|
|
@ -275,6 +288,8 @@ func ctxDataSet(args ...any) func(ctx *context.Context) {
|
|||
func Routes() *web.Route {
|
||||
routes := web.NewRoute()
|
||||
|
||||
routes.Use(chi_middleware.GetHead)
|
||||
|
||||
routes.Head("/", misc.DummyOK) // for health check - doesn't need to be passed through gzip handler
|
||||
routes.Methods("GET, HEAD, OPTIONS", "/assets/*", optionsCorsHandler(), public.FileHandlerFunc())
|
||||
routes.Methods("GET, HEAD", "/avatars/*", resizingHandler("avatars", storage.Avatars, avatar.AllowedResizedAvatarSizes))
|
||||
|
|
@ -285,54 +300,82 @@ func Routes() *web.Route {
|
|||
|
||||
_ = templates.HTMLRenderer()
|
||||
|
||||
var mid []any
|
||||
|
||||
var gzipMid any
|
||||
if setting.EnableGzip {
|
||||
wrapper, err := gzhttp.NewWrapper(gzhttp.RandomJitter(32, 0, false), gzhttp.MinSize(GzipMinSize))
|
||||
if err != nil {
|
||||
log.Fatal("gzhttp.NewWrapper failed: %v", err)
|
||||
}
|
||||
mid = append(mid, wrapper)
|
||||
gzipMid = wrapper
|
||||
}
|
||||
|
||||
if setting.Service.EnableCaptcha {
|
||||
// The captcha http.Handler should only fire on /captcha/* so we can just mount this on that url
|
||||
routes.Methods("GET,HEAD", "/captcha/*", append(mid, captcha.Server(captcha.StdWidth, captcha.StdHeight).ServeHTTP)...)
|
||||
routes.Methods("GET,HEAD", "/captcha/*", gzipMid, captcha.Server(captcha.StdWidth, captcha.StdHeight).ServeHTTP)
|
||||
}
|
||||
|
||||
if setting.Metrics.Enabled {
|
||||
prometheus.MustRegister(metrics.NewCollector())
|
||||
routes.Get("/metrics", append(mid, Metrics)...)
|
||||
routes.Get("/metrics", gzipMid, Metrics)
|
||||
}
|
||||
|
||||
routes.Methods("GET,HEAD", "/robots.txt", append(mid, misc.RobotsTxt)...)
|
||||
routes.Methods("GET,HEAD", "/manifest.json", append(mid, misc.ManifestJSON)...)
|
||||
routes.Methods("GET,HEAD", "/robots.txt", gzipMid, misc.RobotsTxt)
|
||||
routes.Methods("GET,HEAD", "/manifest.json", gzipMid, misc.ManifestJSON)
|
||||
routes.Get("/ssh_info", misc.SSHInfo)
|
||||
routes.Get("/api/healthz", healthcheck.Check)
|
||||
|
||||
mid = append(mid, common.Sessioner(), context.Contexter())
|
||||
|
||||
// Get user from session if logged in.
|
||||
mid = append(mid, webAuth(buildAuthGroup()))
|
||||
|
||||
// GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route
|
||||
mid = append(mid, chi_middleware.GetHead)
|
||||
|
||||
if setting.API.EnableSwagger {
|
||||
// Note: The route is here but no in API routes because it renders a web page
|
||||
routes.Get("/api/swagger", append(mid, misc.Swagger)...) // Render V1 by default
|
||||
routes.Get("/api/forgejo/swagger", append(mid, misc.SwaggerForgejo)...)
|
||||
routes.Group("", func() {
|
||||
// Note: The route is here but no in API routes because it renders a web page
|
||||
routes.Get("/api/swagger", misc.Swagger) // Render V1 by default
|
||||
routes.Get("/api/forgejo/swagger", misc.SwaggerForgejo)
|
||||
}, gzipMid, context.Contexter())
|
||||
}
|
||||
|
||||
// TODO: These really seem like things that could be folded into Contexter or as helper functions
|
||||
mid = append(mid, user.GetNotificationCount)
|
||||
mid = append(mid, repo.GetActiveStopwatch)
|
||||
mid = append(mid, goGet)
|
||||
routes.Group("",
|
||||
func() {
|
||||
registerRoutes(routes)
|
||||
},
|
||||
gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildAuthGroup()),
|
||||
// TODO: GetNotificationCount & GetActiveStopwatch really seem like things that could be folded into Contexter or as helper functions
|
||||
user.GetNotificationCount, repo.GetActiveStopwatch,
|
||||
goGet)
|
||||
|
||||
// /{username}/{repo}/info/lfs routes currently need to use buildAuthGroup(), not buildGitAuthGroup(), because:
|
||||
//
|
||||
// a) there are tests that use session auth to access the LFS endpoints (TestAPILFSLocksLogged), which may be a test
|
||||
// error, and
|
||||
//
|
||||
// b) if AuthorizedIntegration is included then JWTs generated by the LFS system with `setting.LFS.SigningKey` will
|
||||
// return AuthenticationAttemptedIncorrectCredential from AuthorizedIntegration, and cause the requests to 401
|
||||
// before reaching the LFS handlers.
|
||||
//
|
||||
// (a) is probably an error that can be fixed, and (b) should be fixed by changing LFS's JWT handling to be an
|
||||
// `auth_service.Method` implementation which would be incorporated into the auth group, so that LFS isn't doing
|
||||
// it's own "after the authentication" authentication. In the interm, it's split out from the `registerRoutes` call
|
||||
// above because at least fewer middlewares can be safely applied.
|
||||
routes.Group("",
|
||||
func() {
|
||||
registerGitLFSRoutes(routes)
|
||||
}, gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildAuthGroup()), goGet)
|
||||
routes.Group("",
|
||||
func() {
|
||||
registerGitRoutes(routes)
|
||||
}, gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildGitAuthGroup()), goGet)
|
||||
|
||||
routes.NotFound(
|
||||
gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildAuthGroup()),
|
||||
// TODO: GetNotificationCount & GetActiveStopwatch really seem like things that could be folded into Contexter or as helper functions
|
||||
user.GetNotificationCount, repo.GetActiveStopwatch,
|
||||
goGet,
|
||||
func(w http.ResponseWriter, req *http.Request) {
|
||||
ctx := context.GetWebContext(req)
|
||||
if ctx == nil {
|
||||
panic("missing middleware context.Contexter()")
|
||||
}
|
||||
ctx.NotFound("", nil)
|
||||
})
|
||||
|
||||
others := web.NewRoute()
|
||||
others.Use(mid...)
|
||||
registerRoutes(others)
|
||||
routes.Mount("", others)
|
||||
return routes
|
||||
}
|
||||
|
||||
|
|
@ -405,13 +448,6 @@ func registerRoutes(m *web.Route) {
|
|||
}
|
||||
}
|
||||
|
||||
lfsServerEnabled := func(ctx *context.Context) {
|
||||
if !setting.LFS.StartServer {
|
||||
ctx.Error(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
federationEnabled := func(ctx *context.Context) {
|
||||
if !setting.Federation.Enabled {
|
||||
ctx.Error(http.StatusNotFound)
|
||||
|
|
@ -1763,27 +1799,6 @@ func registerRoutes(m *web.Route) {
|
|||
m.Group("/{reponame}", func() {
|
||||
m.Get("", repo.SetEditorconfigIfExists, repo.Home)
|
||||
}, ignSignIn, context.RepoAssignment, context.RepoRef(), context.UnitTypes())
|
||||
|
||||
m.Group("/{reponame}", func() {
|
||||
m.Group("/info/lfs", func() {
|
||||
m.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler)
|
||||
m.Put("/objects/{oid}/{size}", lfs.UploadHandler)
|
||||
m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler)
|
||||
m.Get("/objects/{oid}", lfs.DownloadHandler)
|
||||
m.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler)
|
||||
m.Group("/locks", func() {
|
||||
m.Get("/", lfs.GetListLockHandler)
|
||||
m.Post("/", lfs.PostLockHandler)
|
||||
m.Post("/verify", lfs.VerifyLockHandler)
|
||||
m.Post("/{lid}/unlock", lfs.UnLockHandler)
|
||||
}, lfs.CheckAcceptMediaType)
|
||||
m.Any("/*", func(ctx *context.Context) {
|
||||
ctx.NotFound("", nil)
|
||||
})
|
||||
}, ignoreCSRF, lfsServerEnabled)
|
||||
|
||||
gitHTTPRouters(m)
|
||||
})
|
||||
})
|
||||
|
||||
if setting.Repository.EnableFlags {
|
||||
|
|
@ -1815,10 +1830,37 @@ func registerRoutes(m *web.Route) {
|
|||
m.Get("/demo/error/{errcode}", demo.ErrorPage)
|
||||
}, ignSignIn)
|
||||
}
|
||||
}
|
||||
|
||||
m.NotFound(func(w http.ResponseWriter, req *http.Request) {
|
||||
ctx := context.GetWebContext(req)
|
||||
ctx.NotFound("", nil)
|
||||
// Registers HTTP Git related routes, which have different top-level middleware than [registerRoutes].
|
||||
func registerGitLFSRoutes(m *web.Route) {
|
||||
lfsServerEnabled := func(ctx *context.Context) {
|
||||
if !setting.LFS.StartServer {
|
||||
ctx.Error(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
}
|
||||
m.Group("/{username}/{reponame}/info/lfs", func() {
|
||||
m.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler)
|
||||
m.Put("/objects/{oid}/{size}", lfs.UploadHandler)
|
||||
m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler)
|
||||
m.Get("/objects/{oid}", lfs.DownloadHandler)
|
||||
m.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler)
|
||||
m.Group("/locks", func() {
|
||||
m.Get("/", lfs.GetListLockHandler)
|
||||
m.Post("/", lfs.PostLockHandler)
|
||||
m.Post("/verify", lfs.VerifyLockHandler)
|
||||
m.Post("/{lid}/unlock", lfs.UnLockHandler)
|
||||
}, lfs.CheckAcceptMediaType)
|
||||
m.Any("/*", func(ctx *context.Context) {
|
||||
ctx.NotFound("", nil)
|
||||
})
|
||||
}, ignoreCSRF, lfsServerEnabled)
|
||||
}
|
||||
|
||||
func registerGitRoutes(m *web.Route) {
|
||||
m.Group("/{username}/{reponame}", func() {
|
||||
gitHTTPRouters(m)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -178,10 +178,6 @@ func TestGitHTTPSameStatusCodeForGetAndHeadRequests(t *testing.T) {
|
|||
for _, c := range cases {
|
||||
t.Run(caseToTestName(c), func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
session := emptyTestSession(t)
|
||||
if c.User != nil {
|
||||
session = loginUser(t, c.User.Name)
|
||||
}
|
||||
if c.IsCollaborator {
|
||||
testCtx := NewAPITestContext(t, owner.Name, repo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
doAPIAddCollaborator(testCtx, c.User.Name, perm.AccessModeRead)(t)
|
||||
|
|
@ -194,9 +190,15 @@ func TestGitHTTPSameStatusCodeForGetAndHeadRequests(t *testing.T) {
|
|||
// code for both GET and HEAD, which needs to equal the test cases expected
|
||||
// status code
|
||||
getReq := NewRequestf(t, "GET", "%s/%s", repo.Link(), c.Endpoint)
|
||||
getResp := session.MakeRequest(t, getReq, NoExpectedStatus)
|
||||
if c.User != nil {
|
||||
getReq.AddBasicAuth(c.User.Name)
|
||||
}
|
||||
getResp := MakeRequest(t, getReq, NoExpectedStatus)
|
||||
headReq := NewRequestf(t, "HEAD", "%s/%s", repo.Link(), c.Endpoint)
|
||||
headResp := session.MakeRequest(t, headReq, NoExpectedStatus)
|
||||
if c.User != nil {
|
||||
headReq.AddBasicAuth(c.User.Name)
|
||||
}
|
||||
headResp := MakeRequest(t, headReq, NoExpectedStatus)
|
||||
require.Equal(t, getResp.Result().StatusCode, headResp.Result().StatusCode)
|
||||
require.Equal(t, c.ExpectedStatusCode, headResp.Result().StatusCode)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1414,3 +1414,24 @@ func doFsckConsistencyChecks(dstPath string) func(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitAuthorizedIntegration(t *testing.T) {
|
||||
onApplicationRun(t, func(t *testing.T, u *url.URL) {
|
||||
ait := newAITester(t)
|
||||
defer ait.close()
|
||||
token := ait.signedJWT()
|
||||
u.User = url.UserPassword("token", token)
|
||||
|
||||
t.Run("clone private repo of user", func(t *testing.T) {
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
|
||||
u.Path = fmt.Sprintf("/%s/%s.git", repo.OwnerName, repo.Name)
|
||||
doGitClone(t.TempDir(), u)(t)
|
||||
})
|
||||
|
||||
t.Run("cannot clone private repo of another user", func(t *testing.T) {
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 52})
|
||||
u.Path = fmt.Sprintf("/%s/%s.git", repo.OwnerName, repo.Name)
|
||||
doGitCloneFail(u)(t)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue