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
This commit is contained in:
		
							parent
							
								
									57a8440db3
								
							
						
					
					
						commit
						7096085f2b
					
				
					 5 changed files with 32 additions and 10 deletions
				
			
		|  | @ -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) | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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) { | ||||
|  |  | |||
|  | @ -2595,6 +2595,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 () { | ||||
|  |  | |||
|  | @ -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) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
|  | @ -635,7 +639,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) | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue