Skip to content

Commit 497f945

Browse files
authored
Merge pull request #1003 from gotify/csrf
Csrf
2 parents 50b917a + 97c425c commit 497f945

4 files changed

Lines changed: 85 additions & 4 deletions

File tree

auth/authentication.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package auth
22

33
import (
44
"errors"
5+
"net/http"
56
"strings"
67
"time"
78

@@ -40,6 +41,7 @@ type Database interface {
4041
type Auth struct {
4142
DB Database
4243
SecureCookie bool
44+
CrossOrigin *http.CrossOriginProtection
4345
}
4446

4547
// RequireAdmin requires an elevated client token or basic auth, the user must be an admin.
@@ -87,6 +89,9 @@ func (a *Auth) Optional(ctx *gin.Context) {
8789
}
8890

8991
func (a *Auth) evaluate(ctx *gin.Context, funcs ...func(ctx *gin.Context) (authState, error)) bool {
92+
if a.rejectForeignOrigin(ctx) {
93+
return true
94+
}
9095
for _, fn := range funcs {
9196
state, err := fn(ctx)
9297
if err != nil {
@@ -128,6 +133,17 @@ func (a *Auth) abort403(ctx *gin.Context) {
128133
ctx.AbortWithError(403, errors.New("you are not allowed to access this api"))
129134
}
130135

136+
func (a *Auth) rejectForeignOrigin(ctx *gin.Context) bool {
137+
if _, isCookie := a.readTokenFromRequest(ctx); !isCookie {
138+
return false
139+
}
140+
if err := a.CrossOrigin.Check(ctx.Request); err != nil {
141+
ctx.AbortWithError(403, err)
142+
return true
143+
}
144+
return false
145+
}
146+
131147
func (a *Auth) handleUser(checks ...func(*model.User) (authState, error)) func(ctx *gin.Context) (authState, error) {
132148
return func(ctx *gin.Context) (authState, error) {
133149
if name, pass, ok := ctx.Request.BasicAuth(); ok {

auth/authentication_test.go

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type AuthenticationSuite struct {
2929
func (s *AuthenticationSuite) SetupSuite() {
3030
mode.Set(mode.TestDev)
3131
s.DB = testdb.NewDB(s.T())
32-
s.auth = &Auth{DB: s.DB}
32+
s.auth = &Auth{DB: s.DB, CrossOrigin: http.NewCrossOriginProtection()}
3333

3434
now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)
3535
timeNow = func() time.Time { return now }
@@ -354,4 +354,65 @@ func (s *AuthenticationSuite) assertHeaderRequest(key, value string, f fMiddlewa
354354
return ctx
355355
}
356356

357+
func (s *AuthenticationSuite) TestCookieCrossOriginProtection() {
358+
// httptest sets the request host to example.com.
359+
s.assertCsrfRequest(map[string]string{"Origin": "http://example.com"}, "clienttoken", s.auth.RequireClient, 200)
360+
s.assertCsrfRequest(map[string]string{"Origin": "https://example.com"}, "clienttoken", s.auth.RequireClient, 200)
361+
362+
s.assertCsrfRequest(map[string]string{"Origin": "http://evil.com"}, "clienttoken", s.auth.RequireClient, 403)
363+
s.assertCsrfRequest(map[string]string{"Origin": "https://example.com.evil.com"}, "clienttoken", s.auth.RequireClient, 403)
364+
s.assertCsrfRequest(map[string]string{"Origin": "null"}, "clienttoken", s.auth.RequireClient, 403)
365+
366+
s.assertCsrfRequest(nil, "clienttoken", s.auth.RequireClient, 200)
367+
368+
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-origin"}, "clienttoken", s.auth.RequireClient, 200)
369+
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "none"}, "clienttoken", s.auth.RequireClient, 200)
370+
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "cross-site"}, "clienttoken", s.auth.RequireClient, 403)
371+
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-site"}, "clienttoken", s.auth.RequireClient, 403)
372+
373+
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "cross-site"}, "clienttoken_admin_elevated", s.auth.RequireElevatedClient, 403)
374+
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-origin"}, "clienttoken_admin_elevated", s.auth.RequireElevatedClient, 200)
375+
}
376+
377+
func (s *AuthenticationSuite) TestCrossOriginProtectionIgnoredForTokenAuth() {
378+
recorder := httptest.NewRecorder()
379+
ctx, _ := gin.CreateTestContext(recorder)
380+
ctx.Request = httptest.NewRequest("POST", "/", nil)
381+
ctx.Request.Header.Set("X-Gotify-Key", "clienttoken")
382+
ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site")
383+
s.auth.RequireClient(ctx)
384+
assert.Equal(s.T(), 200, recorder.Code)
385+
386+
recorder = httptest.NewRecorder()
387+
ctx, _ = gin.CreateTestContext(recorder)
388+
ctx.Request = httptest.NewRequest("POST", "/?token=clienttoken", nil)
389+
ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site")
390+
s.auth.RequireClient(ctx)
391+
assert.Equal(s.T(), 200, recorder.Code)
392+
}
393+
394+
func (s *AuthenticationSuite) TestCrossOriginProtectionAllowsSafeMethods() {
395+
recorder := httptest.NewRecorder()
396+
ctx, _ := gin.CreateTestContext(recorder)
397+
ctx.Request = httptest.NewRequest("GET", "/", nil)
398+
ctx.Request.AddCookie(&http.Cookie{Name: cookieName, Value: "clienttoken"})
399+
ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site")
400+
s.auth.RequireClient(ctx)
401+
assert.Equal(s.T(), 200, recorder.Code)
402+
}
403+
404+
func (s *AuthenticationSuite) assertCsrfRequest(headers map[string]string, cookie string, f fMiddleware, code int) {
405+
recorder := httptest.NewRecorder()
406+
ctx, _ := gin.CreateTestContext(recorder)
407+
ctx.Request = httptest.NewRequest("POST", "/", nil)
408+
if cookie != "" {
409+
ctx.Request.AddCookie(&http.Cookie{Name: cookieName, Value: cookie})
410+
}
411+
for k, v := range headers {
412+
ctx.Request.Header.Set(k, v)
413+
}
414+
f(ctx)
415+
assert.Equal(s.T(), code, recorder.Code)
416+
}
417+
357418
type fMiddleware gin.HandlerFunc

model/message.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type MessageExternal struct {
3232
// read only: true
3333
// required: true
3434
// example: 5
35-
ApplicationID uint `json:"appid"`
35+
ApplicationID uint `form:"appid" query:"appid" json:"appid"`
3636
// The message. Markdown (excluding html) is allowed.
3737
//
3838
// required: true
@@ -74,7 +74,7 @@ type CreateMessage struct {
7474
// The application id that send this message. Always set when returned via the API.
7575
//
7676
// example: 5
77-
ApplicationID uint `json:"appid"`
77+
ApplicationID uint `form:"appid" query:"appid" json:"appid"`
7878
// The message. Markdown (excluding html) is allowed.
7979
//
8080
// required: true

router/router.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,11 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co
8484
}
8585
}
8686
}()
87-
authentication := auth.Auth{DB: db, SecureCookie: conf.Server.SecureCookie}
87+
authentication := auth.Auth{
88+
DB: db,
89+
SecureCookie: conf.Server.SecureCookie,
90+
CrossOrigin: http.NewCrossOriginProtection(),
91+
}
8892
messageHandler := api.MessageAPI{Notifier: streamHandler, DB: db}
8993
healthHandler := api.HealthAPI{DB: db}
9094
clientHandler := api.ClientAPI{

0 commit comments

Comments
 (0)