mirror of https://github.com/coder/coder.git
fix: Strip session_token cookie from app proxy requests (#3528)
Fixes coder/security#1.
This commit is contained in:
parent
000e1a5ef2
commit
c3f946737c
|
@ -0,0 +1,31 @@
|
|||
package httpapi
|
||||
|
||||
import (
|
||||
"net/textproto"
|
||||
"strings"
|
||||
|
||||
"github.com/coder/coder/codersdk"
|
||||
)
|
||||
|
||||
// StripCoderCookies removes the session token from the cookie header provided.
|
||||
func StripCoderCookies(header string) string {
|
||||
header = textproto.TrimString(header)
|
||||
cookies := []string{}
|
||||
|
||||
var part string
|
||||
for len(header) > 0 { // continue since we have rest
|
||||
part, header, _ = strings.Cut(header, ";")
|
||||
part = textproto.TrimString(part)
|
||||
if part == "" {
|
||||
continue
|
||||
}
|
||||
name, _, _ := strings.Cut(part, "=")
|
||||
if name == codersdk.SessionTokenKey ||
|
||||
name == codersdk.OAuth2StateKey ||
|
||||
name == codersdk.OAuth2RedirectKey {
|
||||
continue
|
||||
}
|
||||
cookies = append(cookies, part)
|
||||
}
|
||||
return strings.Join(cookies, "; ")
|
||||
}
|
|
@ -0,0 +1,35 @@
|
|||
package httpapi_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/coderd/httpapi"
|
||||
)
|
||||
|
||||
func TestStripCoderCookies(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, tc := range []struct {
|
||||
Input string
|
||||
Output string
|
||||
}{{
|
||||
"testing=hello; wow=test",
|
||||
"testing=hello; wow=test",
|
||||
}, {
|
||||
"session_token=moo; wow=test",
|
||||
"wow=test",
|
||||
}, {
|
||||
"another_token=wow; session_token=ok",
|
||||
"another_token=wow",
|
||||
}, {
|
||||
"session_token=ok; oauth_state=wow; oauth_redirect=/",
|
||||
"",
|
||||
}} {
|
||||
tc := tc
|
||||
t.Run(tc.Input, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
require.Equal(t, tc.Output, httpapi.StripCoderCookies(tc.Input))
|
||||
})
|
||||
}
|
||||
}
|
|
@ -13,11 +13,6 @@ import (
|
|||
"github.com/coder/coder/cryptorand"
|
||||
)
|
||||
|
||||
const (
|
||||
oauth2StateCookieName = "oauth_state"
|
||||
oauth2RedirectCookieName = "oauth_redirect"
|
||||
)
|
||||
|
||||
type oauth2StateKey struct{}
|
||||
|
||||
type OAuth2State struct {
|
||||
|
@ -71,7 +66,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
|
|||
}
|
||||
|
||||
http.SetCookie(rw, &http.Cookie{
|
||||
Name: oauth2StateCookieName,
|
||||
Name: codersdk.OAuth2StateKey,
|
||||
Value: state,
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
|
@ -80,7 +75,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
|
|||
// Redirect must always be specified, otherwise
|
||||
// an old redirect could apply!
|
||||
http.SetCookie(rw, &http.Cookie{
|
||||
Name: oauth2RedirectCookieName,
|
||||
Name: codersdk.OAuth2RedirectKey,
|
||||
Value: r.URL.Query().Get("redirect"),
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
|
@ -98,10 +93,10 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
|
|||
return
|
||||
}
|
||||
|
||||
stateCookie, err := r.Cookie(oauth2StateCookieName)
|
||||
stateCookie, err := r.Cookie(codersdk.OAuth2StateKey)
|
||||
if err != nil {
|
||||
httpapi.Write(rw, http.StatusUnauthorized, codersdk.Response{
|
||||
Message: fmt.Sprintf("Cookie %q must be provided.", oauth2StateCookieName),
|
||||
Message: fmt.Sprintf("Cookie %q must be provided.", codersdk.OAuth2StateKey),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
@ -113,7 +108,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
|
|||
}
|
||||
|
||||
var redirect string
|
||||
stateRedirect, err := r.Cookie(oauth2RedirectCookieName)
|
||||
stateRedirect, err := r.Cookie(codersdk.OAuth2RedirectKey)
|
||||
if err == nil {
|
||||
redirect = stateRedirect.Value
|
||||
}
|
||||
|
|
|
@ -12,6 +12,7 @@ import (
|
|||
"golang.org/x/oauth2"
|
||||
|
||||
"github.com/coder/coder/coderd/httpmw"
|
||||
"github.com/coder/coder/codersdk"
|
||||
)
|
||||
|
||||
type testOAuth2Provider struct {
|
||||
|
@ -71,7 +72,7 @@ func TestOAuth2(t *testing.T) {
|
|||
t.Parallel()
|
||||
req := httptest.NewRequest("GET", "/?code=something&state=test", nil)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: "oauth_state",
|
||||
Name: codersdk.OAuth2StateKey,
|
||||
Value: "mismatch",
|
||||
})
|
||||
res := httptest.NewRecorder()
|
||||
|
@ -82,7 +83,7 @@ func TestOAuth2(t *testing.T) {
|
|||
t.Parallel()
|
||||
req := httptest.NewRequest("GET", "/?code=test&state=something", nil)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: "oauth_state",
|
||||
Name: codersdk.OAuth2StateKey,
|
||||
Value: "something",
|
||||
})
|
||||
req.AddCookie(&http.Cookie{
|
||||
|
|
|
@ -447,7 +447,7 @@ func oauth2Callback(t *testing.T, client *codersdk.Client) *http.Response {
|
|||
req, err := http.NewRequest("GET", oauthURL.String(), nil)
|
||||
require.NoError(t, err)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: "oauth_state",
|
||||
Name: codersdk.OAuth2StateKey,
|
||||
Value: state,
|
||||
})
|
||||
res, err := client.HTTPClient.Do(req)
|
||||
|
@ -469,7 +469,7 @@ func oidcCallback(t *testing.T, client *codersdk.Client) *http.Response {
|
|||
req, err := http.NewRequest("GET", oauthURL.String(), nil)
|
||||
require.NoError(t, err)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: "oauth_state",
|
||||
Name: codersdk.OAuth2StateKey,
|
||||
Value: state,
|
||||
})
|
||||
res, err := client.HTTPClient.Do(req)
|
||||
|
|
|
@ -170,6 +170,12 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
|
|||
}
|
||||
defer release()
|
||||
|
||||
// This strips the session token from a workspace app request.
|
||||
cookieHeaders := r.Header.Values("Cookie")[:]
|
||||
r.Header.Del("Cookie")
|
||||
for _, cookieHeader := range cookieHeaders {
|
||||
r.Header.Add("Cookie", httpapi.StripCoderCookies(cookieHeader))
|
||||
}
|
||||
proxy.Transport = conn.HTTPTransport()
|
||||
proxy.ServeHTTP(rw, r)
|
||||
}
|
||||
|
|
|
@ -9,6 +9,7 @@ import (
|
|||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"cdr.dev/slog/sloggers/slogtest"
|
||||
|
@ -27,6 +28,8 @@ func TestWorkspaceAppsProxyPath(t *testing.T) {
|
|||
require.NoError(t, err)
|
||||
server := http.Server{
|
||||
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_, err := r.Cookie(codersdk.SessionTokenKey)
|
||||
assert.ErrorIs(t, err, http.ErrNoCookie)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}),
|
||||
}
|
||||
|
|
|
@ -15,8 +15,15 @@ import (
|
|||
"nhooyr.io/websocket"
|
||||
)
|
||||
|
||||
// SessionTokenKey represents the name of the cookie or query parameter the API key is stored in.
|
||||
const SessionTokenKey = "session_token"
|
||||
// These cookies are Coder-specific. If a new one is added or changed, the name
|
||||
// shouldn't be likely to conflict with any user-application set cookies.
|
||||
// Be sure to strip additional cookies in httpapi.StripCoder Cookies!
|
||||
const (
|
||||
// SessionTokenKey represents the name of the cookie or query parameter the API key is stored in.
|
||||
SessionTokenKey = "session_token"
|
||||
OAuth2StateKey = "oauth_state"
|
||||
OAuth2RedirectKey = "oauth_redirect"
|
||||
)
|
||||
|
||||
// New creates a Coder client for the provided URL.
|
||||
func New(serverURL *url.URL) *Client {
|
||||
|
|
Loading…
Reference in New Issue