Skip to content

Commit 7096085

Browse files
zeripathtechknowlogick
authored andcommitted
Fix #5226 by adding CSRF checking to api reqToken and add CSRF to the POST header for deadline (#5250)
* 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
1 parent 57a8440 commit 7096085

File tree

5 files changed

+32
-10
lines changed

5 files changed

+32
-10
lines changed

integrations/api_admin_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ func TestAPIAdminCreateAndDeleteSSHKey(t *testing.T) {
3939
OwnerID: keyOwner.ID,
4040
})
4141

42-
req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token="+token,
43-
keyOwner.Name, newPublicKey.ID)
42+
req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token=%s",
43+
keyOwner.Name, newPublicKey.ID, token)
4444
session.MakeRequest(t, req, http.StatusNoContent)
4545
models.AssertNotExistsBean(t, &models.PublicKey{ID: newPublicKey.ID})
4646
}
@@ -51,7 +51,7 @@ func TestAPIAdminDeleteMissingSSHKey(t *testing.T) {
5151
session := loginUser(t, "user1")
5252

5353
token := getTokenForLoggedInUser(t, session)
54-
req := NewRequestf(t, "DELETE", "/api/v1/admin/users/user1/keys/%d?token="+token, models.NonexistentID)
54+
req := NewRequestf(t, "DELETE", "/api/v1/admin/users/user1/keys/%d?token=%s", models.NonexistentID, token)
5555
session.MakeRequest(t, req, http.StatusNotFound)
5656
}
5757

@@ -73,8 +73,8 @@ func TestAPIAdminDeleteUnauthorizedKey(t *testing.T) {
7373

7474
session = loginUser(t, normalUsername)
7575
token = getTokenForLoggedInUser(t, session)
76-
req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token="+token,
77-
adminUsername, newPublicKey.ID)
76+
req = NewRequestf(t, "DELETE", "/api/v1/admin/users/%s/keys/%d?token=%s",
77+
adminUsername, newPublicKey.ID, token)
7878
session.MakeRequest(t, req, http.StatusForbidden)
7979
}
8080

integrations/git_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ func TestGit(t *testing.T) {
143143

144144
session := loginUser(t, "user1")
145145
keyOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User)
146-
urlStr := fmt.Sprintf("/api/v1/admin/users/%s/keys", keyOwner.Name)
146+
token := getTokenForLoggedInUser(t, session)
147+
urlStr := fmt.Sprintf("/api/v1/admin/users/%s/keys?token=%s", keyOwner.Name, token)
147148

148149
dataPubKey, err := ioutil.ReadFile(keyFile + ".pub")
149150
assert.NoError(t, err)

modules/context/api.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"fmt"
99
"strings"
1010

11+
"github.com/go-macaron/csrf"
12+
1113
"code.gitea.io/git"
1214
"code.gitea.io/gitea/models"
1315
"code.gitea.io/gitea/modules/base"
@@ -97,6 +99,17 @@ func (ctx *APIContext) SetLinkHeader(total, pageSize int) {
9799
}
98100
}
99101

102+
// RequireCSRF requires a validated a CSRF token
103+
func (ctx *APIContext) RequireCSRF() {
104+
headerToken := ctx.Req.Header.Get(ctx.csrf.GetHeaderName())
105+
formValueToken := ctx.Req.FormValue(ctx.csrf.GetFormName())
106+
if len(headerToken) > 0 || len(formValueToken) > 0 {
107+
csrf.Validate(ctx.Context.Context, ctx.csrf)
108+
} else {
109+
ctx.Context.Error(401)
110+
}
111+
}
112+
100113
// APIContexter returns apicontext as macaron middleware
101114
func APIContexter() macaron.Handler {
102115
return func(c *Context) {

public/js/index.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2595,6 +2595,10 @@ function updateDeadline(deadlineString) {
25952595
data: JSON.stringify({
25962596
'due_date': realDeadline,
25972597
}),
2598+
headers: {
2599+
'X-Csrf-Token': csrf,
2600+
'X-Remote': true,
2601+
},
25982602
contentType: 'application/json',
25992603
type: 'POST',
26002604
success: function () {

routers/api/v1/api.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,15 @@ func repoAssignment() macaron.Handler {
174174

175175
// Contexter middleware already checks token for user sign in process.
176176
func reqToken() macaron.Handler {
177-
return func(ctx *context.Context) {
178-
if true != ctx.Data["IsApiToken"] {
179-
ctx.Error(401)
177+
return func(ctx *context.APIContext) {
178+
if true == ctx.Data["IsApiToken"] {
179+
return
180+
}
181+
if ctx.IsSigned {
182+
ctx.RequireCSRF()
180183
return
181184
}
185+
ctx.Context.Error(401)
182186
}
183187
}
184188

@@ -635,7 +639,7 @@ func RegisterRoutes(m *macaron.Macaron) {
635639
m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo)
636640
})
637641
})
638-
}, reqAdmin())
642+
}, reqToken(), reqAdmin())
639643

640644
m.Group("/topics", func() {
641645
m.Get("/search", repo.TopicSearch)

0 commit comments

Comments
 (0)