Backport #5250 on v1.6: Fix Issue 5249 and protect /api/v1/admin routes with CSRF...

* 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

diff --git a/integrations/api_admin_test.go b/integrations/api_admin_test.go
index 690edad757a640a878ff56eb739071bfbea58ccf..b8dded9c116a4bd80153d6295a7ca254144e4d7d 100644 (file)
--- 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 7ac375dd029d673cb5a43cfe00301c9fb8f258a9..96d39e0519e7dfeadc66413854d7d3c7a4f0c2c3 100644 (file)
--- 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 0bf4307726e8e3b75ea0cadfe03fe75664744580..6a9c792370f4fd23c08c84bbcd0f7d72c573e355 100644 (file)
--- 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 fad531cc4991083060785c49324412694496528f..9bde52f97d1ee0254e7c4717bce0c92a5cb9ec1b 100644 (file)
--- 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 23a85759c2f5f945fa6b61590ad7d06ad2baddc5..fe54aa2a320b6ab5d3489c490f08ee2b12cb3b45 100644 (file)
--- 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)