From e847cbadfa0c5140ccdde17251eb100f0ce6d619 Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:35:34 +0200 Subject: [PATCH] Check permissions on repo lookup (#2357) There was no permission check when looking up repos so you were able to get basic repo information even if you're not allowed to. This uses `session.MustPull` (and set repo/perms before) to fix this. --- server/api/repo.go | 17 +---------------- server/router/api.go | 2 +- server/router/middleware/session/repo.go | 18 +++++++----------- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/server/api/repo.go b/server/api/repo.go index 6ca9b6436..f92d9d2a3 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -21,7 +21,6 @@ import ( "fmt" "net/http" "strconv" - "strings" "time" "github.com/gin-gonic/gin" @@ -277,21 +276,7 @@ func ChownRepo(c *gin.Context) { // @Param Authorization header string true "Insert your personal access token" default(Bearer ) // @Param repo_full_name path string true "the repository full-name / slug" func LookupRepo(c *gin.Context) { - _store := store.FromContext(c) - repoFullName := strings.TrimLeft(c.Param("repo_full_name"), "/") - - repo, err := _store.GetRepoName(repoFullName) - if err != nil { - if errors.Is(err, types.RecordNotExist) { - c.AbortWithStatus(http.StatusNotFound) - return - } - - _ = c.AbortWithError(http.StatusInternalServerError, err) - return - } - - c.JSON(http.StatusOK, repo) + c.JSON(http.StatusOK, session.Repo(c)) } // GetRepo diff --git a/server/router/api.go b/server/router/api.go index 53025bde1..43ac2243f 100644 --- a/server/router/api.go +++ b/server/router/api.go @@ -68,7 +68,7 @@ func apiRoutes(e *gin.RouterGroup) { } } - apiBase.GET("/repos/lookup/*repo_full_name", api.LookupRepo) // TODO: check if this public route is a security issue + apiBase.GET("/repos/lookup/*repo_full_name", session.SetRepo(), session.SetPerm(), session.MustPull, api.LookupRepo) apiBase.POST("/repos", session.MustUser(), api.PostRepo) repoBase := apiBase.Group("/repos/:repo_id") { diff --git a/server/router/middleware/session/repo.go b/server/router/middleware/session/repo.go index 3868ab859..58b7a1c48 100644 --- a/server/router/middleware/session/repo.go +++ b/server/router/middleware/session/repo.go @@ -18,6 +18,7 @@ import ( "errors" "net/http" "strconv" + "strings" "time" "github.com/gin-gonic/gin" @@ -45,11 +46,10 @@ func Repo(c *gin.Context) *model.Repo { func SetRepo() gin.HandlerFunc { return func(c *gin.Context) { var ( - _store = store.FromContext(c) - owner = c.Param("owner") - name = c.Param("name") - _repoID = c.Param("repo_id") - user = User(c) + _store = store.FromContext(c) + fullName = strings.TrimLeft(c.Param("repo_full_name"), "/") + _repoID = c.Param("repo_id") + user = User(c) ) var repo *model.Repo @@ -63,7 +63,7 @@ func SetRepo() gin.HandlerFunc { } repo, err = _store.GetRepo(repoID) } else { - repo, err = _store.GetRepoName(owner + "/" + name) + repo, err = _store.GetRepoName(fullName) } if repo != nil { @@ -73,11 +73,7 @@ func SetRepo() gin.HandlerFunc { } // debugging - log.Debug().Msgf("Cannot find repository %s/%s. %s", - owner, - name, - err.Error(), - ) + log.Debug().Err(err).Msgf("Cannot find repository %s.", fullName) if user == nil { c.AbortWithStatus(http.StatusUnauthorized)