synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-13 21:36:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: Implement (but not enforce) CSRF for FE requests (#3786)

Browse Source 
Future work is to enforce CSRF

Co-authored-by: Presley Pizzo <presley@coder.com>
This commit is contained in:
Steven Masley
2022-09-13 15:26:46 -04:00
committed by GitHub
parent 9ab437d6e2
commit 9b5ee8f267
22 changed files with 211 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
coderd/coderd.go
View File

@ -187,6 +187,7 @@ func New(options *Options) *API {
next.ServeHTTP(w, r)
})
},
httpmw.CSRF(options.SecureAuthCookie),
)
apps := func(r chi.Router) {

6
coderd/httpapi/cookie_test.go
View File

@ -17,13 +17,13 @@ func TestStripCoderCookies(t *testing.T) {
"testing=hello; wow=test",
"testing=hello; wow=test",
}, {
"session_token=moo; wow=test",
"coder_session_token=moo; wow=test",
"wow=test",
}, {
"another_token=wow; session_token=ok",
"another_token=wow; coder_session_token=ok",
"another_token=wow",
}, {
"session_token=ok; oauth_state=wow; oauth_redirect=/",
"coder_session_token=ok; oauth_state=wow; oauth_redirect=/",
"",
}} {
tc := tc

41
coderd/httpmw/apikey.go
View File

@ -123,13 +123,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
httpapi.Write(rw, code, response)
}
var cookieValue string
cookie, err := r.Cookie(codersdk.SessionTokenKey)
if err != nil {
cookieValue = r.URL.Query().Get(codersdk.SessionTokenKey)
} else {
cookieValue = cookie.Value
}
cookieValue := apiTokenFromRequest(r)
if cookieValue == "" {
write(http.StatusUnauthorized, codersdk.Response{
Message: signedOutErrorMessage,
@ -335,3 +329,36 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
})
}
}
// apiTokenFromRequest returns the api token from the request.
// Find the session token from:
// 1: The cookie
// 2: The old cookie
// 3. The coder_session_token query parameter
// 4. The custom auth header
func apiTokenFromRequest(r *http.Request) string {
cookie, err := r.Cookie(codersdk.SessionTokenKey)
if err == nil && cookie.Value != "" {
return cookie.Value
}
// TODO: @emyrk in October 2022, remove this oldCookie check.
// This is just to support the old cli for 1 release. Then everyone
// must update.
oldCookie, err := r.Cookie("session_token")
if err == nil && oldCookie.Value != "" {
return oldCookie.Value
}
urlValue := r.URL.Query().Get(codersdk.SessionTokenKey)
if urlValue != "" {
return urlValue
}
headerValue := r.Header.Get(codersdk.SessionCustomHeader)
if headerValue != "" {
return headerValue
}
return ""
}

60
coderd/httpmw/apikey_test.go
View File

@ -74,10 +74,7 @@ func TestAPIKey(t *testing.T) {
r = httptest.NewRequest("GET", "/", nil)
rw = httptest.NewRecorder()
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: "test-wow-hello",
})
r.Header.Set(codersdk.SessionCustomHeader, "test-wow-hello")
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
res := rw.Result()
@ -92,10 +89,7 @@ func TestAPIKey(t *testing.T) {
r = httptest.NewRequest("GET", "/", nil)
rw = httptest.NewRecorder()
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: "test-wow",
})
r.Header.Set(codersdk.SessionCustomHeader, "test-wow")
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
res := rw.Result()
@ -110,10 +104,7 @@ func TestAPIKey(t *testing.T) {
r = httptest.NewRequest("GET", "/", nil)
rw = httptest.NewRecorder()
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: "testtestid-wow",
})
r.Header.Set(codersdk.SessionCustomHeader, "testtestid-wow")
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
res := rw.Result()
@ -129,10 +120,7 @@ func TestAPIKey(t *testing.T) {
r = httptest.NewRequest("GET", "/", nil)
rw = httptest.NewRecorder()
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
httpmw.ExtractAPIKey(db, nil, false)(successHandler).ServeHTTP(rw, r)
res := rw.Result()
@ -149,10 +137,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
// Use a different secret so they don't match!
hashed := sha256.Sum256([]byte("differentsecret"))
@ -178,10 +163,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
_, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,
@ -206,10 +188,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,
@ -280,10 +259,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,
@ -316,10 +292,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,
@ -352,10 +325,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,
@ -395,10 +365,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
sentAPIKey, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,
@ -449,10 +416,7 @@ func TestAPIKey(t *testing.T) {
user = createUser(r.Context(), t, db)
)
r.RemoteAddr = "1.1.1.1:3555"
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
_, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
ID: id,

5
coderd/httpmw/authorize_test.go
View File

@ -93,10 +93,7 @@ func TestExtractUserRoles(t *testing.T) {
})
req := httptest.NewRequest("GET", "/", nil)
req.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: token,
})
req.Header.Set(codersdk.SessionCustomHeader, token)
rtr.ServeHTTP(rw, req)
resp := rw.Result()

72
coderd/httpmw/csrf.go Normal file
View File

@ -0,0 +1,72 @@
package httpmw
import (
"net/http"
"regexp"
"github.com/justinas/nosurf"
"golang.org/x/xerrors"
"github.com/coder/coder/codersdk"
)
// CSRF is a middleware that verifies that a CSRF token is present in the request
// for non-GET requests.
func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
mw := nosurf.New(next)
mw.SetBaseCookie(http.Cookie{Path: "/", HttpOnly: true, SameSite: http.SameSiteLaxMode, Secure: secureCookie})
mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Something is wrong with your CSRF token. Please refresh the page. If this error persists, try clearing your cookies.", http.StatusBadRequest)
}))
// Exempt all requests that do not require CSRF protection.
// All GET requests are exempt by default.
mw.ExemptPath("/api/v2/csp/reports")
// Top level agent routes.
mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/[^/]*$"))
// Agent authenticated routes
mw.ExemptRegexp(regexp.MustCompile("api/v2/workspaceagents/me/*"))
// Derp routes
mw.ExemptRegexp(regexp.MustCompile("derp/*"))
mw.ExemptFunc(func(r *http.Request) bool {
// Enable CSRF in November 2022 by deleting this "return true" line.
// CSRF is not enforced to ensure backwards compatibility with older
// cli versions.
//nolint:revive
return true
// CSRF only affects requests that automatically attach credentials via a cookie.
// If no cookie is present, then there is no risk of CSRF.
//nolint:govet
sessCookie, err := r.Cookie(codersdk.SessionTokenKey)
if xerrors.Is(err, http.ErrNoCookie) {
return true
}
if token := r.Header.Get(codersdk.SessionCustomHeader); token == sessCookie.Value {
// If the cookie and header match, we can assume this is the same as just using the
// custom header auth. Custom header auth can bypass CSRF, as CSRF attacks
// cannot add custom headers.
return true
}
if token := r.URL.Query().Get(codersdk.SessionTokenKey); token == sessCookie.Value {
// If the auth is set in a url param and matches the cookie, it
// is the same as just using the url param.
return true
}
// If the X-CSRF-TOKEN header is set, we can exempt the func if it's valid.
// This is the CSRF check.
sent := r.Header.Get("X-CSRF-TOKEN")
if sent != "" {
return nosurf.VerifyToken(nosurf.Token(r), sent)
}
return false
})
return mw
}
}

5
coderd/httpmw/organizationparam_test.go
View File

@ -29,10 +29,7 @@ func TestOrganizationParam(t *testing.T) {
r = httptest.NewRequest("GET", "/", nil)
hashed = sha256.Sum256([]byte(secret))
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)

5
coderd/httpmw/templateparam_test.go
View File

@ -29,10 +29,7 @@ func TestTemplateParam(t *testing.T) {
hashed = sha256.Sum256([]byte(secret))
)
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)

5
coderd/httpmw/templateversionparam_test.go
View File

@ -29,10 +29,7 @@ func TestTemplateVersionParam(t *testing.T) {
hashed = sha256.Sum256([]byte(secret))
)
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)

5
coderd/httpmw/userparam_test.go
View File

@ -29,10 +29,7 @@ func TestUserParam(t *testing.T) {
r = httptest.NewRequest("GET", "/", nil)
rw = httptest.NewRecorder()
)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
user, err := db.InsertUser(r.Context(), database.InsertUserParams{
ID: uuid.New(),

6
coderd/httpmw/workspaceagent.go
View File

@ -29,14 +29,14 @@ func WorkspaceAgent(r *http.Request) database.WorkspaceAgent {
func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
cookie, err := r.Cookie(codersdk.SessionTokenKey)
if err != nil {
cookieValue := apiTokenFromRequest(r)
if cookieValue == "" {
httpapi.Write(rw, http.StatusUnauthorized, codersdk.Response{
Message: fmt.Sprintf("Cookie %q must be provided.", codersdk.SessionTokenKey),
})
return
}
token, err := uuid.Parse(cookie.Value)
token, err := uuid.Parse(cookieValue)
if err != nil {
httpapi.Write(rw, http.StatusUnauthorized, codersdk.Response{
Message: "Agent token is invalid.",

5
coderd/httpmw/workspaceagent_test.go
View File

@ -22,10 +22,7 @@ func TestWorkspaceAgent(t *testing.T) {
setup := func(db database.Store) (*http.Request, uuid.UUID) {
token := uuid.New()
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: token.String(),
})
r.Header.Set(codersdk.SessionCustomHeader, token.String())
return r, token
}

5
coderd/httpmw/workspaceagentparam_test.go
View File

@ -29,10 +29,7 @@ func TestWorkspaceAgentParam(t *testing.T) {
hashed = sha256.Sum256([]byte(secret))
)
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)

5
coderd/httpmw/workspacebuildparam_test.go
View File

@ -29,10 +29,7 @@ func TestWorkspaceBuildParam(t *testing.T) {
hashed = sha256.Sum256([]byte(secret))
)
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)

10
coderd/httpmw/workspaceparam_test.go
View File

@ -32,10 +32,7 @@ func TestWorkspaceParam(t *testing.T) {
hashed = sha256.Sum256([]byte(secret))
)
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)
@ -340,10 +337,7 @@ func setupWorkspaceWithAgents(t testing.TB, cfg setupConfig) (database.Store, *h
hashed = sha256.Sum256([]byte(secret))
)
r := httptest.NewRequest("GET", "/", nil)
r.AddCookie(&http.Cookie{
Name: codersdk.SessionTokenKey,
Value: fmt.Sprintf("%s-%s", id, secret),
})
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))
userID := uuid.New()
username, err := cryptorand.String(8)

15
coderd/userauth_test.go
View File

@ -245,7 +245,7 @@ func TestUserOAuth2Github(t *testing.T) {
resp := oauth2Callback(t, client)
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
client.SessionToken = resp.Cookies()[0].Value
client.SessionToken = authCookieValue(resp.Cookies())
user, err := client.User(context.Background(), "me")
require.NoError(t, err)
require.Equal(t, "kyle@coder.com", user.Email)
@ -398,14 +398,14 @@ func TestUserOIDC(t *testing.T) {
defer cancel()
if tc.Username != "" {
client.SessionToken = resp.Cookies()[0].Value
client.SessionToken = authCookieValue(resp.Cookies())
user, err := client.User(ctx, "me")
require.NoError(t, err)
require.Equal(t, tc.Username, user.Username)
}
if tc.AvatarURL != "" {
client.SessionToken = resp.Cookies()[0].Value
client.SessionToken = authCookieValue(resp.Cookies())
user, err := client.User(ctx, "me")
require.NoError(t, err)
require.Equal(t, tc.AvatarURL, user.AvatarURL)
@ -534,3 +534,12 @@ func oidcCallback(t *testing.T, client *codersdk.Client) *http.Response {
func i64ptr(i int64) *int64 {
return &i
}
func authCookieValue(cookies []*http.Cookie) string {
for _, cookie := range cookies {
if cookie.Name == codersdk.SessionTokenKey {
return cookie.Value
}
}
return ""
}

13
coderd/users_test.go
View File

@ -330,11 +330,16 @@ func TestPostLogout(t *testing.T) {
require.Equal(t, http.StatusOK, res.StatusCode)
cookies := res.Cookies()
require.Len(t, cookies, 2, "Exactly two cookies should be returned")
require.Equal(t, codersdk.SessionTokenKey, cookies[0].Name, "Cookie should be the auth & app cookie")
require.Equal(t, codersdk.SessionTokenKey, cookies[1].Name, "Cookie should be the auth & app cookie")
require.Equal(t, -1, cookies[0].MaxAge, "Cookie should be set to delete")
var found bool
for _, cookie := range cookies {
if cookie.Name == codersdk.SessionTokenKey {
require.Equal(t, codersdk.SessionTokenKey, cookie.Name, "Cookie should be the auth cookie")
require.Equal(t, -1, cookie.MaxAge, "Cookie should be set to delete")
found = true
}
}
require.True(t, found, "auth cookie should be returned")
_, err = client.GetAPIKey(ctx, admin.UserID.String(), keyID)
sdkErr := &codersdk.Error{}