Skip to content

Commit e96f32d

Browse files
pasha-codefreshpasha-codefresh
authored
Merge commit from fork
* feat: verify rbac on each message and not just during handshake Signed-off-by: pashakostohrys <[email protected]> * feat: verify rbac on each message and not just during handshake Signed-off-by: pashakostohrys <[email protected]> * fix: linter and e2e tests Signed-off-by: pashakostohrys <[email protected]> * fix: linter and e2e tests Signed-off-by: pashakostohrys <[email protected]> --------- Signed-off-by: pashakostohrys <[email protected]>
1 parent 212d320 commit e96f32d

File tree

3 files changed

+163
-15
lines changed

3 files changed

+163
-15
lines changed

‎server/application/terminal.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func (s *terminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
225225

226226
fieldLog.Info("terminal session starting")
227227

228-
session, err := newTerminalSession(w, r, nil, s.sessionManager)
228+
session, err := newTerminalSession(ctx, w, r, nil, s.sessionManager, appRBACName, s.enf)
229229
if err != nil {
230230
http.Error(w, "Failed to start terminal session", http.StatusBadRequest)
231231
return

‎server/application/websocket.go

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
package application
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
6-
"github.com/argoproj/argo-cd/v2/common"
7-
httputil "github.com/argoproj/argo-cd/v2/util/http"
8-
util_session "github.com/argoproj/argo-cd/v2/util/session"
97
"net/http"
108
"sync"
119
"time"
1210

11+
"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
12+
"github.com/argoproj/argo-cd/v2/util/rbac"
13+
14+
"github.com/argoproj/argo-cd/v2/common"
15+
httputil "github.com/argoproj/argo-cd/v2/util/http"
16+
util_session "github.com/argoproj/argo-cd/v2/util/session"
17+
1318
"github.com/gorilla/websocket"
1419
log "github.com/sirupsen/logrus"
1520
"k8s.io/client-go/tools/remotecommand"
@@ -31,6 +36,7 @@ var upgrader = func() websocket.Upgrader {
3136

3237
// terminalSession implements PtyHandler
3338
type terminalSession struct {
39+
ctx context.Context
3440
wsConn *websocket.Conn
3541
sizeChan chan remotecommand.TerminalSize
3642
doneChan chan struct{}
@@ -39,6 +45,8 @@ type terminalSession struct {
3945
writeLock sync.Mutex
4046
sessionManager *util_session.SessionManager
4147
token *string
48+
appRBACName string
49+
enf *rbac.Enforcer
4250
}
4351

4452
// getToken get auth token from web socket request
@@ -48,7 +56,7 @@ func getToken(r *http.Request) (string, error) {
4856
}
4957

5058
// newTerminalSession create terminalSession
51-
func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager) (*terminalSession, error) {
59+
func newTerminalSession(ctx context.Context, w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager, appRBACName string, enf *rbac.Enforcer) (*terminalSession, error) {
5260
token, err := getToken(r)
5361
if err != nil {
5462
return nil, err
@@ -59,12 +67,15 @@ func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader h
5967
return nil, err
6068
}
6169
session := &terminalSession{
70+
ctx: ctx,
6271
wsConn: conn,
6372
tty: true,
6473
sizeChan: make(chan remotecommand.TerminalSize),
6574
doneChan: make(chan struct{}),
6675
sessionManager: sessionManager,
6776
token: &token,
77+
appRBACName: appRBACName,
78+
enf: enf,
6879
}
6980
return session, nil
7081
}
@@ -125,6 +136,29 @@ func (t *terminalSession) reconnect() (int, error) {
125136
return 0, nil
126137
}
127138

139+
func (t *terminalSession) validatePermissions(p []byte) (int, error) {
140+
permissionDeniedMessage, _ := json.Marshal(TerminalMessage{
141+
Operation: "stdout",
142+
Data: "Permission denied",
143+
})
144+
if err := t.enf.EnforceErr(t.ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, t.appRBACName); err != nil {
145+
err = t.wsConn.WriteMessage(websocket.TextMessage, permissionDeniedMessage)
146+
if err != nil {
147+
log.Errorf("permission denied message err: %v", err)
148+
}
149+
return copy(p, EndOfTransmission), permissionDeniedErr
150+
}
151+
152+
if err := t.enf.EnforceErr(t.ctx.Value("claims"), rbacpolicy.ResourceExec, rbacpolicy.ActionCreate, t.appRBACName); err != nil {
153+
err = t.wsConn.WriteMessage(websocket.TextMessage, permissionDeniedMessage)
154+
if err != nil {
155+
log.Errorf("permission denied message err: %v", err)
156+
}
157+
return copy(p, EndOfTransmission), permissionDeniedErr
158+
}
159+
return 0, nil
160+
}
161+
128162
// Read called in a loop from remotecommand as long as the process is running
129163
func (t *terminalSession) Read(p []byte) (int, error) {
130164
// check if token still valid
@@ -135,6 +169,12 @@ func (t *terminalSession) Read(p []byte) (int, error) {
135169
return t.reconnect()
136170
}
137171

172+
// validate permissions
173+
code, err := t.validatePermissions(p)
174+
if err != nil {
175+
return code, err
176+
}
177+
138178
t.readLock.Lock()
139179
_, message, err := t.wsConn.ReadMessage()
140180
t.readLock.Unlock()

‎server/application/websocket_test.go

Lines changed: 118 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,77 @@
11
package application
22

33
import (
4+
"context"
45
"encoding/json"
5-
"github.com/gorilla/websocket"
6-
"github.com/stretchr/testify/assert"
76
"net/http"
87
"net/http/httptest"
98
"strings"
109
"testing"
10+
11+
v1 "k8s.io/api/core/v1"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/client-go/kubernetes/fake"
14+
15+
"github.com/argoproj/argo-cd/v2/common"
16+
"github.com/argoproj/argo-cd/v2/util/assets"
17+
"github.com/argoproj/argo-cd/v2/util/rbac"
18+
19+
"github.com/golang-jwt/jwt/v4"
20+
"github.com/gorilla/websocket"
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
1123
)
1224

13-
func reconnect(w http.ResponseWriter, r *http.Request) {
14-
var upgrader = websocket.Upgrader{}
25+
func newTestTerminalSession(w http.ResponseWriter, r *http.Request) terminalSession {
26+
upgrader := websocket.Upgrader{}
1527
c, err := upgrader.Upgrade(w, r, nil)
1628
if err != nil {
17-
return
29+
return terminalSession{}
1830
}
1931

20-
ts := terminalSession{wsConn: c}
32+
return terminalSession{wsConn: c}
33+
}
34+
35+
func newEnforcer() *rbac.Enforcer {
36+
additionalConfig := make(map[string]string, 0)
37+
kubeclientset := fake.NewSimpleClientset(&v1.ConfigMap{
38+
ObjectMeta: metav1.ObjectMeta{
39+
Namespace: testNamespace,
40+
Name: "argocd-cm",
41+
Labels: map[string]string{
42+
"app.kubernetes.io/part-of": "argocd",
43+
},
44+
},
45+
Data: additionalConfig,
46+
}, &v1.Secret{
47+
ObjectMeta: metav1.ObjectMeta{
48+
Name: "argocd-secret",
49+
Namespace: testNamespace,
50+
},
51+
Data: map[string][]byte{
52+
"admin.password": []byte("test"),
53+
"server.secretkey": []byte("test"),
54+
},
55+
})
56+
57+
enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
58+
return enforcer
59+
}
60+
61+
func reconnect(w http.ResponseWriter, r *http.Request) {
62+
ts := newTestTerminalSession(w, r)
2163
_, _ = ts.reconnect()
2264
}
2365

2466
func TestReconnect(t *testing.T) {
25-
2667
s := httptest.NewServer(http.HandlerFunc(reconnect))
2768
defer s.Close()
2869

2970
u := "ws" + strings.TrimPrefix(s.URL, "http")
3071

3172
// Connect to the server
3273
ws, _, err := websocket.DefaultDialer.Dial(u, nil)
33-
assert.NoError(t, err)
74+
require.NoError(t, err)
3475

3576
defer ws.Close()
3677

@@ -40,7 +81,74 @@ func TestReconnect(t *testing.T) {
4081

4182
err = json.Unmarshal(p, &message)
4283

43-
assert.NoError(t, err)
44-
assert.Equal(t, message.Data, ReconnectMessage)
84+
require.NoError(t, err)
85+
assert.Equal(t, ReconnectMessage, message.Data)
86+
}
87+
88+
func TestValidateWithAdminPermissions(t *testing.T) {
89+
validate := func(w http.ResponseWriter, r *http.Request) {
90+
enf := newEnforcer()
91+
_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
92+
enf.SetDefaultRole("role:admin")
93+
enf.SetClaimsEnforcerFunc(func(claims jwt.Claims, rvals ...interface{}) bool {
94+
return true
95+
})
96+
ts := newTestTerminalSession(w, r)
97+
ts.enf = enf
98+
ts.appRBACName = "test"
99+
// nolint:staticcheck
100+
ts.ctx = context.WithValue(context.Background(), "claims", &jwt.MapClaims{"groups": []string{"admin"}})
101+
_, err := ts.validatePermissions([]byte{})
102+
require.NoError(t, err)
103+
}
104+
105+
s := httptest.NewServer(http.HandlerFunc(validate))
106+
defer s.Close()
107+
108+
u := "ws" + strings.TrimPrefix(s.URL, "http")
109+
110+
// Connect to the server
111+
ws, _, err := websocket.DefaultDialer.Dial(u, nil)
112+
require.NoError(t, err)
113+
114+
defer ws.Close()
115+
}
116+
117+
func TestValidateWithoutPermissions(t *testing.T) {
118+
validate := func(w http.ResponseWriter, r *http.Request) {
119+
enf := newEnforcer()
120+
_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
121+
enf.SetDefaultRole("role:test")
122+
enf.SetClaimsEnforcerFunc(func(claims jwt.Claims, rvals ...interface{}) bool {
123+
return false
124+
})
125+
ts := newTestTerminalSession(w, r)
126+
ts.enf = enf
127+
ts.appRBACName = "test"
128+
// nolint:staticcheck
129+
ts.ctx = context.WithValue(context.Background(), "claims", &jwt.MapClaims{"groups": []string{"test"}})
130+
_, err := ts.validatePermissions([]byte{})
131+
require.Error(t, err)
132+
assert.Equal(t, permissionDeniedErr.Error(), err.Error())
133+
}
134+
135+
s := httptest.NewServer(http.HandlerFunc(validate))
136+
defer s.Close()
137+
138+
u := "ws" + strings.TrimPrefix(s.URL, "http")
139+
140+
// Connect to the server
141+
ws, _, err := websocket.DefaultDialer.Dial(u, nil)
142+
require.NoError(t, err)
143+
144+
defer ws.Close()
145+
146+
_, p, _ := ws.ReadMessage()
147+
148+
var message TerminalMessage
149+
150+
err = json.Unmarshal(p, &message)
45151

152+
require.NoError(t, err)
153+
assert.Equal(t, "Permission denied", message.Data)
46154
}

0 commit comments

Comments
 (0)