Skip to content

Commit 9e0f6e1

Browse files
[ws-proxy] close connection if workspace or port stopped share (#20248)
* remove useless InfoProvider * close connection if visibility change * nit: fix typo * fn name typo fix --------- Co-authored-by: Filip Troníček <[email protected]>
1 parent f1b040f commit 9e0f6e1

File tree

6 files changed

+152
-79
lines changed

6 files changed

+152
-79
lines changed

components/ws-proxy/cmd/run.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,13 @@ var runCmd = &cobra.Command{
7878
log.WithError(err).Fatal(err, "unable to start manager")
7979
}
8080

81-
var infoprov proxy.CompositeInfoProvider
82-
crdInfoProv, err := proxy.NewCRDWorkspaceInfoProvider(mgr.GetClient(), mgr.GetScheme())
81+
infoprov, err := proxy.NewCRDWorkspaceInfoProvider(mgr.GetClient(), mgr.GetScheme())
8382
if err != nil {
8483
log.WithError(err).Fatal("cannot create CRD-based info provider")
8584
}
86-
if err = crdInfoProv.SetupWithManager(mgr); err != nil {
85+
if err = infoprov.SetupWithManager(mgr); err != nil {
8786
log.WithError(err).Fatal(err, "unable to create CRD-based info provider", "controller", "Workspace")
8887
}
89-
infoprov = append(infoprov, crdInfoProv)
9088

9189
if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
9290
log.WithError(err).Fatal("unable to set up health check")

components/ws-proxy/pkg/common/infoprovider.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package common
66

77
import (
8+
"context"
89
"time"
910

1011
"github.com/gitpod-io/gitpod/ws-manager/api"
@@ -43,6 +44,9 @@ type WorkspaceCoords struct {
4344
type WorkspaceInfoProvider interface {
4445
// WorkspaceInfo returns the workspace information of a workspace using it's workspace ID
4546
WorkspaceInfo(workspaceID string) *WorkspaceInfo
47+
48+
AcquireContext(ctx context.Context, workspaceID, port string) (context.Context, string, error)
49+
ReleaseContext(id string)
4650
}
4751

4852
// WorkspaceInfo is all the infos ws-proxy needs to know about a workspace.

components/ws-proxy/pkg/proxy/auth.go

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package proxy
66

77
import (
8+
"errors"
89
"fmt"
910
"net/http"
1011
"net/url"
@@ -17,6 +18,12 @@ import (
1718
"github.com/gitpod-io/gitpod/ws-proxy/pkg/common"
1819
)
1920

21+
var (
22+
ErrTokenNotFound = fmt.Errorf("no owner cookie present")
23+
ErrTokenMismatch = fmt.Errorf("owner token mismatch")
24+
ErrTokenDecode = fmt.Errorf("cannot decode owner token")
25+
)
26+
2027
// WorkspaceAuthHandler rejects requests which are not authenticated or authorized to access a workspace.
2128
func WorkspaceAuthHandler(domain string, info common.WorkspaceInfoProvider) mux.MiddlewareFunc {
2229
return func(h http.Handler) http.Handler {
@@ -47,68 +54,76 @@ func WorkspaceAuthHandler(domain string, info common.WorkspaceInfoProvider) mux.
4754
return
4855
}
4956

57+
isPublic := false
5058
if ws.Auth != nil && ws.Auth.Admission == api.AdmissionLevel_ADMIT_EVERYONE {
51-
// workspace is free for all - no tokens or cookies matter
52-
h.ServeHTTP(resp, req)
53-
54-
return
55-
}
56-
57-
if port != "" {
58-
// this is a workspace port request and ports can be public or private.
59-
// For public ports no tokens or cookies matter, private ports are subject
60-
// to the same access policies as the workspace itself is.
61-
var isPublic bool
62-
59+
isPublic = true
60+
} else if port != "" {
6361
prt, err := strconv.ParseUint(port, 10, 16)
6462
if err != nil {
6563
log.WithField("port", port).WithError(err).Error("cannot convert port to int")
6664
} else {
6765
for _, p := range ws.Ports {
6866
if p.Port == uint32(prt) {
6967
isPublic = p.Visibility == api.PortVisibility_PORT_VISIBILITY_PUBLIC
70-
7168
break
7269
}
7370
}
7471
}
75-
76-
if isPublic {
77-
// workspace port is free for all - no tokens or cookies matter
78-
h.ServeHTTP(resp, req)
79-
80-
return
81-
}
82-
83-
// port seems to be private - subject it to the same access policy as the workspace itself
8472
}
8573

86-
tkn := req.Header.Get("x-gitpod-owner-token")
87-
if tkn == "" {
88-
cn := fmt.Sprintf("%s%s_owner_", cookiePrefix, ws.InstanceID)
89-
c, err := req.Cookie(cn)
74+
authenticate := func() (bool, error) {
75+
tkn := req.Header.Get("x-gitpod-owner-token")
76+
if tkn == "" {
77+
cn := fmt.Sprintf("%s%s_owner_", cookiePrefix, ws.InstanceID)
78+
c, err := req.Cookie(cn)
79+
if err != nil {
80+
return false, ErrTokenNotFound
81+
}
82+
tkn = c.Value
83+
}
84+
tkn, err := url.QueryUnescape(tkn)
9085
if err != nil {
91-
log.WithField("cookieName", cn).Debug("no owner cookie present")
92-
resp.WriteHeader(http.StatusUnauthorized)
93-
94-
return
86+
return false, ErrTokenDecode
9587
}
9688

97-
tkn = c.Value
89+
if tkn != ws.Auth.OwnerToken {
90+
return false, ErrTokenMismatch
91+
}
92+
return true, nil
9893
}
99-
tkn, err := url.QueryUnescape(tkn)
100-
if err != nil {
101-
log.WithError(err).Warn("cannot decode owner token")
102-
resp.WriteHeader(http.StatusBadRequest)
10394

95+
authenticated, err := authenticate()
96+
if !authenticated && !isPublic {
97+
if err != nil {
98+
if errors.Is(err, ErrTokenNotFound) {
99+
resp.WriteHeader(http.StatusUnauthorized)
100+
return
101+
}
102+
if errors.Is(err, ErrTokenMismatch) {
103+
log.Warn("owner token mismatch")
104+
resp.WriteHeader(http.StatusForbidden)
105+
return
106+
}
107+
if errors.Is(err, ErrTokenDecode) {
108+
log.Warn("cannot decode owner token")
109+
resp.WriteHeader(http.StatusBadRequest)
110+
return
111+
}
112+
}
113+
log.WithError(err).Error("cannot authenticate")
114+
resp.WriteHeader(http.StatusInternalServerError)
104115
return
105116
}
106117

107-
if tkn != ws.Auth.OwnerToken {
108-
log.Warn("owner token mismatch")
109-
resp.WriteHeader(http.StatusForbidden)
110-
111-
return
118+
if !authenticated && isPublic {
119+
ctx, id, err := info.AcquireContext(req.Context(), wsID, port)
120+
if err != nil {
121+
log.WithError(err).Error("cannot acquire context")
122+
resp.WriteHeader(http.StatusInternalServerError)
123+
return
124+
}
125+
defer info.ReleaseContext(id)
126+
req = req.WithContext(ctx)
112127
}
113128

114129
h.ServeHTTP(resp, req)

components/ws-proxy/pkg/proxy/auth_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ func TestWorkspaceAuthHandler(t *testing.T) {
3636
testPort = 8080
3737
)
3838
var (
39-
ownerOnlyInfos = map[string]*common.WorkspaceInfo{
40-
workspaceID: {
39+
ownerOnlyInfos = []common.WorkspaceInfo{
40+
{
4141
WorkspaceID: workspaceID,
4242
InstanceID: instanceID,
4343
Auth: &api.WorkspaceAuthentication{
@@ -47,8 +47,8 @@ func TestWorkspaceAuthHandler(t *testing.T) {
4747
Ports: []*api.PortSpec{{Port: testPort, Visibility: api.PortVisibility_PORT_VISIBILITY_PRIVATE}},
4848
},
4949
}
50-
publicPortInfos = map[string]*common.WorkspaceInfo{
51-
workspaceID: {
50+
publicPortInfos = []common.WorkspaceInfo{
51+
{
5252
WorkspaceID: workspaceID,
5353
InstanceID: instanceID,
5454
Auth: &api.WorkspaceAuthentication{
@@ -58,8 +58,8 @@ func TestWorkspaceAuthHandler(t *testing.T) {
5858
Ports: []*api.PortSpec{{Port: testPort, Visibility: api.PortVisibility_PORT_VISIBILITY_PUBLIC}},
5959
},
6060
}
61-
admitEveryoneInfos = map[string]*common.WorkspaceInfo{
62-
workspaceID: {
61+
admitEveryoneInfos = []common.WorkspaceInfo{
62+
{
6363
WorkspaceID: workspaceID,
6464
InstanceID: instanceID,
6565
Auth: &api.WorkspaceAuthentication{Admission: api.AdmissionLevel_ADMIT_EVERYONE},
@@ -68,7 +68,7 @@ func TestWorkspaceAuthHandler(t *testing.T) {
6868
)
6969
tests := []struct {
7070
Name string
71-
Infos map[string]*common.WorkspaceInfo
71+
Infos []common.WorkspaceInfo
7272
OwnerCookie string
7373
WorkspaceID string
7474
Port string
@@ -225,7 +225,7 @@ func TestWorkspaceAuthHandler(t *testing.T) {
225225
for _, test := range tests {
226226
t.Run(test.Name, func(t *testing.T) {
227227
var res testResult
228-
handler := WorkspaceAuthHandler(domain, &fixedInfoProvider{Infos: test.Infos})(http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
228+
handler := WorkspaceAuthHandler(domain, &fakeWsInfoProvider{infos: test.Infos})(http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
229229
res.HandlerCalled = true
230230
resp.WriteHeader(http.StatusOK)
231231
}))

components/ws-proxy/pkg/proxy/infoprovider.go

Lines changed: 77 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ import (
88
"context"
99
"net/url"
1010
"sort"
11+
"strconv"
1112

1213
"golang.org/x/xerrors"
1314
"k8s.io/apimachinery/pkg/api/errors"
1415
"k8s.io/apimachinery/pkg/runtime"
16+
"k8s.io/apimachinery/pkg/util/uuid"
1517
"k8s.io/client-go/tools/cache"
1618
ctrl "sigs.k8s.io/controller-runtime"
1719
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -20,6 +22,7 @@ import (
2022

2123
wsk8s "github.com/gitpod-io/gitpod/common-go/kubernetes"
2224
"github.com/gitpod-io/gitpod/common-go/log"
25+
"github.com/gitpod-io/gitpod/ws-manager/api"
2326
wsapi "github.com/gitpod-io/gitpod/ws-manager/api"
2427
workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
2528
"github.com/gitpod-io/gitpod/ws-proxy/pkg/common"
@@ -48,11 +51,19 @@ func getPortStr(urlStr string) string {
4851
return portURL.Port()
4952
}
5053

54+
type ConnectionContext struct {
55+
WorkspaceID string
56+
Port string
57+
UUID string
58+
CancelFunc context.CancelCauseFunc
59+
}
60+
5161
type CRDWorkspaceInfoProvider struct {
5262
client.Client
5363
Scheme *runtime.Scheme
5464

55-
store cache.ThreadSafeStore
65+
store cache.ThreadSafeStore
66+
contextStore cache.ThreadSafeStore
5667
}
5768

5869
// NewCRDWorkspaceInfoProvider creates a fresh WorkspaceInfoProvider.
@@ -67,12 +78,21 @@ func NewCRDWorkspaceInfoProvider(client client.Client, scheme *runtime.Scheme) (
6778
return nil, xerrors.Errorf("object is not a WorkspaceInfo")
6879
},
6980
}
81+
contextIndexers := cache.Indexers{
82+
workspaceIndex: func(obj interface{}) ([]string, error) {
83+
if connCtx, ok := obj.(*ConnectionContext); ok {
84+
return []string{connCtx.WorkspaceID}, nil
85+
}
86+
return nil, xerrors.Errorf("object is not a ConnectionContext")
87+
},
88+
}
7089

7190
return &CRDWorkspaceInfoProvider{
7291
Client: client,
7392
Scheme: scheme,
7493

75-
store: cache.NewThreadSafeStore(indexers, cache.Indices{}),
94+
store: cache.NewThreadSafeStore(indexers, cache.Indices{}),
95+
contextStore: cache.NewThreadSafeStore(contextIndexers, cache.Indices{}),
7696
}, nil
7797
}
7898

@@ -101,6 +121,28 @@ func (r *CRDWorkspaceInfoProvider) WorkspaceInfo(workspaceID string) *common.Wor
101121
return nil
102122
}
103123

124+
func (r *CRDWorkspaceInfoProvider) AcquireContext(ctx context.Context, workspaceID string, port string) (context.Context, string, error) {
125+
ws := r.WorkspaceInfo(workspaceID)
126+
if ws == nil {
127+
return ctx, "", xerrors.Errorf("workspace %s not found", workspaceID)
128+
}
129+
id := string(uuid.NewUUID())
130+
ctx, cancel := context.WithCancelCause(ctx)
131+
connCtx := &ConnectionContext{
132+
WorkspaceID: workspaceID,
133+
Port: port,
134+
CancelFunc: cancel,
135+
UUID: id,
136+
}
137+
138+
r.contextStore.Add(id, connCtx)
139+
return ctx, id, nil
140+
}
141+
142+
func (r *CRDWorkspaceInfoProvider) ReleaseContext(id string) {
143+
r.contextStore.Delete(id)
144+
}
145+
104146
func (r *CRDWorkspaceInfoProvider) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
105147
var ws workspacev1.Workspace
106148
err := r.Client.Get(context.Background(), req.NamespacedName, &ws)
@@ -162,11 +204,44 @@ func (r *CRDWorkspaceInfoProvider) Reconcile(ctx context.Context, req ctrl.Reque
162204
}
163205

164206
r.store.Update(req.Name, wsinfo)
207+
r.invalidateConnectionContext(wsinfo)
165208
log.WithField("workspace", req.Name).WithField("details", wsinfo).Debug("adding/updating workspace details")
166209

167210
return ctrl.Result{}, nil
168211
}
169212

213+
func (r *CRDWorkspaceInfoProvider) invalidateConnectionContext(ws *common.WorkspaceInfo) {
214+
connCtxs, err := r.contextStore.ByIndex(workspaceIndex, ws.WorkspaceID)
215+
if err != nil {
216+
return
217+
}
218+
if len(connCtxs) == 0 {
219+
return
220+
}
221+
222+
if ws.Auth != nil && ws.Auth.Admission == wsapi.AdmissionLevel_ADMIT_EVERYONE {
223+
return
224+
}
225+
publicPorts := make(map[string]struct{})
226+
for _, p := range ws.Ports {
227+
if p.Visibility == api.PortVisibility_PORT_VISIBILITY_PUBLIC {
228+
publicPorts[strconv.FormatUint(uint64(p.Port), 10)] = struct{}{}
229+
}
230+
}
231+
232+
for _, _connCtx := range connCtxs {
233+
connCtx, ok := _connCtx.(*ConnectionContext)
234+
if !ok {
235+
continue
236+
}
237+
if _, ok := publicPorts[connCtx.Port]; ok {
238+
continue
239+
}
240+
connCtx.CancelFunc(xerrors.Errorf("workspace %s is no longer public", ws.WorkspaceID))
241+
r.contextStore.Delete(connCtx.UUID)
242+
}
243+
}
244+
170245
// SetupWithManager sets up the controller with the Manager.
171246
func (r *CRDWorkspaceInfoProvider) SetupWithManager(mgr ctrl.Manager) error {
172247
return ctrl.NewControllerManagedBy(mgr).
@@ -177,28 +252,3 @@ func (r *CRDWorkspaceInfoProvider) SetupWithManager(mgr ctrl.Manager) error {
177252
).
178253
Complete(r)
179254
}
180-
181-
// CompositeInfoProvider checks each of its info providers and returns the first info found.
182-
type CompositeInfoProvider []common.WorkspaceInfoProvider
183-
184-
func (c CompositeInfoProvider) WorkspaceInfo(workspaceID string) *common.WorkspaceInfo {
185-
for _, ip := range c {
186-
res := ip.WorkspaceInfo(workspaceID)
187-
if res != nil {
188-
return res
189-
}
190-
}
191-
return nil
192-
}
193-
194-
type fixedInfoProvider struct {
195-
Infos map[string]*common.WorkspaceInfo
196-
}
197-
198-
// WorkspaceInfo returns the workspace information of a workspace using it's workspace ID.
199-
func (fp *fixedInfoProvider) WorkspaceInfo(workspaceID string) *common.WorkspaceInfo {
200-
if fp.Infos == nil {
201-
return nil
202-
}
203-
return fp.Infos[workspaceID]
204-
}

0 commit comments

Comments
 (0)