diff options
author | zeripath <art27@cantab.net> | 2018-11-04 15:42:15 +0000 |
---|---|---|
committer | techknowlogick <hello@techknowlogick.com> | 2018-11-04 10:42:15 -0500 |
commit | c0bbbdd30b8161e34d34b9aaf398fd050a16f254 (patch) | |
tree | 59a762a2b41ef4e4dda54f28210684023a5db4b6 | |
parent | f95c9667707376626f810abbb2e738e766003185 (diff) | |
download | gitea-c0bbbdd30b8161e34d34b9aaf398fd050a16f254.tar.gz gitea-c0bbbdd30b8161e34d34b9aaf398fd050a16f254.zip |
Backport #5250 on v1.6: Fix Issue 5249 and protect /api/v1/admin routes with CSRF token (#5272)
* Add CSRF checking to reqToken and place CSRF in the post for deadline creation
Fixes #5226, #5249
* /api/v1/admin/users routes should have reqToken middleware
-rw-r--r-- | integrations/api_admin_test.go | 10 | ||||
-rw-r--r-- | integrations/git_test.go | 3 | ||||
-rw-r--r-- | modules/context/api.go | 13 | ||||
-rw-r--r-- | public/js/index.js | 4 | ||||
-rw-r--r-- | routers/api/v1/api.go | 12 |
5 files changed, 32 insertions, 10 deletions
diff --git a/integrations/api_admin_test.go b/integrations/api_admin_test.go index 690edad757..b8dded9c11 100644 --- a/integrations/api_admin_test.go +++ b/integrations/api_admin_test.go @@ -39,8 +39,8 @@ func TestAPIAdminCreateAndDeleteSSHKey(t *testing.T) { OwnerID: keyOwner.ID, }) - req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token="+token, - keyOwner.Name, newPublicKey.ID) + req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token=%s", + keyOwner.Name, newPublicKey.ID, token) session.MakeRequest(t, req, http.StatusNoContent) models.AssertNotExistsBean(t, &models.PublicKey{ID: newPublicKey.ID}) } @@ -51,7 +51,7 @@ func TestAPIAdminDeleteMissingSSHKey(t *testing.T) { session := loginUser(t, "user1") token := getTokenForLoggedInUser(t, session) - req := NewRequestf(t, "DELETE", "/api/v1/admin/users/user1/keys/%d?token="+token, models.NonexistentID) + req := NewRequestf(t, "DELETE", "/api/v1/admin/users/user1/keys/%d?token=%s", models.NonexistentID, token) session.MakeRequest(t, req, http.StatusNotFound) } @@ -73,8 +73,8 @@ func TestAPIAdminDeleteUnauthorizedKey(t *testing.T) { session = loginUser(t, normalUsername) token = getTokenForLoggedInUser(t, session) - req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token="+token, - adminUsername, newPublicKey.ID) + req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token=%s", + adminUsername, newPublicKey.ID, token) session.MakeRequest(t, req, http.StatusForbidden) } diff --git a/integrations/git_test.go b/integrations/git_test.go index 7ac375dd02..96d39e0519 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -143,7 +143,8 @@ func TestGit(t *testing.T) { session := loginUser(t, "user1") keyOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User) - urlStr := fmt.Sprintf("/api/v1/admin/users/%s/keys", keyOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/admin/users/%s/keys?token=%s", keyOwner.Name, token) dataPubKey, err := ioutil.ReadFile(keyFile + ".pub") assert.NoError(t, err) diff --git a/modules/context/api.go b/modules/context/api.go index 0bf4307726..6a9c792370 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -8,6 +8,8 @@ import ( "fmt" "strings" + "github.com/go-macaron/csrf" + "code.gitea.io/git" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" @@ -97,6 +99,17 @@ func (ctx *APIContext) SetLinkHeader(total, pageSize int) { } } +// RequireCSRF requires a validated a CSRF token +func (ctx *APIContext) RequireCSRF() { + headerToken := ctx.Req.Header.Get(ctx.csrf.GetHeaderName()) + formValueToken := ctx.Req.FormValue(ctx.csrf.GetFormName()) + if len(headerToken) > 0 || len(formValueToken) > 0 { + csrf.Validate(ctx.Context.Context, ctx.csrf) + } else { + ctx.Context.Error(401) + } +} + // APIContexter returns apicontext as macaron middleware func APIContexter() macaron.Handler { return func(c *Context) { diff --git a/public/js/index.js b/public/js/index.js index fad531cc49..9bde52f97d 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -2590,6 +2590,10 @@ function updateDeadline(deadlineString) { data: JSON.stringify({ 'due_date': realDeadline, }), + headers: { + 'X-Csrf-Token': csrf, + 'X-Remote': true, + }, contentType: 'application/json', type: 'POST', success: function () { diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 23a85759c2..fe54aa2a32 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -174,11 +174,15 @@ func repoAssignment() macaron.Handler { // Contexter middleware already checks token for user sign in process. func reqToken() macaron.Handler { - return func(ctx *context.Context) { - if true != ctx.Data["IsApiToken"] { - ctx.Error(401) + return func(ctx *context.APIContext) { + if true == ctx.Data["IsApiToken"] { + return + } + if ctx.IsSigned { + ctx.RequireCSRF() return } + ctx.Context.Error(401) } } @@ -627,7 +631,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo) }) }) - }, reqAdmin()) + }, reqToken(), reqAdmin()) m.Group("/topics", func() { m.Get("/search", repo.TopicSearch) |