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
This commit is contained in:
zeripath 2018-11-04 15:42:15 +00:00 committed by techknowlogick
parent f95c966770
commit c0bbbdd30b
5 changed files with 32 additions and 10 deletions

View file

@ -39,8 +39,8 @@ func TestAPIAdminCreateAndDeleteSSHKey(t *testing.T) {
OwnerID: keyOwner.ID, OwnerID: keyOwner.ID,
}) })
req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token="+token, req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token=%s",
keyOwner.Name, newPublicKey.ID) keyOwner.Name, newPublicKey.ID, token)
session.MakeRequest(t, req, http.StatusNoContent) session.MakeRequest(t, req, http.StatusNoContent)
models.AssertNotExistsBean(t, &models.PublicKey{ID: newPublicKey.ID}) models.AssertNotExistsBean(t, &models.PublicKey{ID: newPublicKey.ID})
} }
@ -51,7 +51,7 @@ func TestAPIAdminDeleteMissingSSHKey(t *testing.T) {
session := loginUser(t, "user1") session := loginUser(t, "user1")
token := getTokenForLoggedInUser(t, session) 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) session.MakeRequest(t, req, http.StatusNotFound)
} }
@ -73,8 +73,8 @@ func TestAPIAdminDeleteUnauthorizedKey(t *testing.T) {
session = loginUser(t, normalUsername) session = loginUser(t, normalUsername)
token = getTokenForLoggedInUser(t, session) token = getTokenForLoggedInUser(t, session)
req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token="+token, req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token=%s",
adminUsername, newPublicKey.ID) adminUsername, newPublicKey.ID, token)
session.MakeRequest(t, req, http.StatusForbidden) session.MakeRequest(t, req, http.StatusForbidden)
} }

View file

@ -143,7 +143,8 @@ func TestGit(t *testing.T) {
session := loginUser(t, "user1") session := loginUser(t, "user1")
keyOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User) 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") dataPubKey, err := ioutil.ReadFile(keyFile + ".pub")
assert.NoError(t, err) assert.NoError(t, err)

View file

@ -8,6 +8,8 @@ import (
"fmt" "fmt"
"strings" "strings"
"github.com/go-macaron/csrf"
"code.gitea.io/git" "code.gitea.io/git"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/base" "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 // APIContexter returns apicontext as macaron middleware
func APIContexter() macaron.Handler { func APIContexter() macaron.Handler {
return func(c *Context) { return func(c *Context) {

View file

@ -2590,6 +2590,10 @@ function updateDeadline(deadlineString) {
data: JSON.stringify({ data: JSON.stringify({
'due_date': realDeadline, 'due_date': realDeadline,
}), }),
headers: {
'X-Csrf-Token': csrf,
'X-Remote': true,
},
contentType: 'application/json', contentType: 'application/json',
type: 'POST', type: 'POST',
success: function () { success: function () {

View file

@ -174,11 +174,15 @@ func repoAssignment() macaron.Handler {
// Contexter middleware already checks token for user sign in process. // Contexter middleware already checks token for user sign in process.
func reqToken() macaron.Handler { func reqToken() macaron.Handler {
return func(ctx *context.Context) { return func(ctx *context.APIContext) {
if true != ctx.Data["IsApiToken"] { if true == ctx.Data["IsApiToken"] {
ctx.Error(401)
return 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) m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo)
}) })
}) })
}, reqAdmin()) }, reqToken(), reqAdmin())
m.Group("/topics", func() { m.Group("/topics", func() {
m.Get("/search", repo.TopicSearch) m.Get("/search", repo.TopicSearch)