Redirect on bad CSRF instead of presenting bad page (#14937)
The current CSRF handler is a bit harsh with bad CSRF tokens on webpages I think we can be a little kinder and redirect to base page with a flash error Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
		
							parent
							
								
									fc1607b368
								
							
						
					
					
						commit
						d06f9ce274
					
				
					 3 changed files with 31 additions and 5 deletions
				
			
		|  | @ -11,6 +11,7 @@ import ( | ||||||
| 	"strings" | 	"strings" | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
|  | 	"code.gitea.io/gitea/modules/setting" | ||||||
| 	"code.gitea.io/gitea/modules/test" | 	"code.gitea.io/gitea/modules/test" | ||||||
| 
 | 
 | ||||||
| 	"github.com/stretchr/testify/assert" | 	"github.com/stretchr/testify/assert" | ||||||
|  | @ -134,5 +135,13 @@ func TestCreateBranchInvalidCSRF(t *testing.T) { | ||||||
| 		"_csrf":           "fake_csrf", | 		"_csrf":           "fake_csrf", | ||||||
| 		"new_branch_name": "test", | 		"new_branch_name": "test", | ||||||
| 	}) | 	}) | ||||||
| 	session.MakeRequest(t, req, http.StatusBadRequest) | 	resp := session.MakeRequest(t, req, http.StatusFound) | ||||||
|  | 	loc := resp.Header().Get("Location") | ||||||
|  | 	assert.Equal(t, setting.AppSubURL+"/", loc) | ||||||
|  | 	resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK) | ||||||
|  | 	htmlDoc := NewHTMLParser(t, resp.Body) | ||||||
|  | 	assert.Equal(t, | ||||||
|  | 		"Bad Request: Invalid CSRF token", | ||||||
|  | 		strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()), | ||||||
|  | 	) | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -22,6 +22,7 @@ import ( | ||||||
| 	"net/http" | 	"net/http" | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
|  | 	"code.gitea.io/gitea/modules/setting" | ||||||
| 	"code.gitea.io/gitea/modules/web/middleware" | 	"code.gitea.io/gitea/modules/web/middleware" | ||||||
| 
 | 
 | ||||||
| 	"github.com/unknwon/com" | 	"github.com/unknwon/com" | ||||||
|  | @ -266,7 +267,12 @@ func Validate(ctx *Context, x CSRF) { | ||||||
| 				-1, | 				-1, | ||||||
| 				x.GetCookiePath(), | 				x.GetCookiePath(), | ||||||
| 				x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
 | 				x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
 | ||||||
|  | 			if middleware.IsAPIPath(ctx.Req) { | ||||||
| 				x.Error(ctx.Resp) | 				x.Error(ctx.Resp) | ||||||
|  | 				return | ||||||
|  | 			} | ||||||
|  | 			ctx.Flash.Error(ctx.Tr("error.invalid_csrf")) | ||||||
|  | 			ctx.Redirect(setting.AppSubURL + "/") | ||||||
| 		} | 		} | ||||||
| 		return | 		return | ||||||
| 	} | 	} | ||||||
|  | @ -277,10 +283,19 @@ func Validate(ctx *Context, x CSRF) { | ||||||
| 				-1, | 				-1, | ||||||
| 				x.GetCookiePath(), | 				x.GetCookiePath(), | ||||||
| 				x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
 | 				x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
 | ||||||
|  | 			if middleware.IsAPIPath(ctx.Req) { | ||||||
| 				x.Error(ctx.Resp) | 				x.Error(ctx.Resp) | ||||||
|  | 				return | ||||||
|  | 			} | ||||||
|  | 			ctx.Flash.Error(ctx.Tr("error.invalid_csrf")) | ||||||
|  | 			ctx.Redirect(setting.AppSubURL + "/") | ||||||
| 		} | 		} | ||||||
| 		return | 		return | ||||||
| 	} | 	} | ||||||
| 
 | 	if middleware.IsAPIPath(ctx.Req) { | ||||||
| 		http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest) | 		http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest) | ||||||
|  | 		return | ||||||
|  | 	} | ||||||
|  | 	ctx.Flash.Error(ctx.Tr("error.missing_csrf")) | ||||||
|  | 	ctx.Redirect(setting.AppSubURL + "/") | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -100,6 +100,8 @@ never = Never | ||||||
| [error] | [error] | ||||||
| occurred = An error has occurred | occurred = An error has occurred | ||||||
| report_message = If you are sure this is a Gitea bug, please search for issue on <a href="https://github.com/go-gitea/gitea/issues">GitHub</a> and open new issue if necessary. | report_message = If you are sure this is a Gitea bug, please search for issue on <a href="https://github.com/go-gitea/gitea/issues">GitHub</a> and open new issue if necessary. | ||||||
|  | missing_csrf = Bad Request: no CSRF token present | ||||||
|  | invalid_csrf = Bad Request: Invalid CSRF token | ||||||
| 
 | 
 | ||||||
| [startpage] | [startpage] | ||||||
| app_desc = A painless, self-hosted Git service | app_desc = A painless, self-hosted Git service | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue