Skip to content

Commit 05edb2a

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]> * feat: verify rbac on each message and not just during handshake Signed-off-by: pashakostohrys <[email protected]> --------- Signed-off-by: pashakostohrys <[email protected]>
1 parent 089247d commit 05edb2a

File tree

3 files changed

+159
-10
lines changed

3 files changed

+159
-10
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: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
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/common"
12+
"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
13+
httputil "github.com/argoproj/argo-cd/v2/util/http"
14+
"github.com/argoproj/argo-cd/v2/util/rbac"
15+
util_session "github.com/argoproj/argo-cd/v2/util/session"
16+
1317
"github.com/gorilla/websocket"
1418
log "github.com/sirupsen/logrus"
1519
"k8s.io/client-go/tools/remotecommand"
@@ -31,6 +35,7 @@ var upgrader = func() websocket.Upgrader {
3135

3236
// terminalSession implements PtyHandler
3337
type terminalSession struct {
38+
ctx context.Context
3439
wsConn *websocket.Conn
3540
sizeChan chan remotecommand.TerminalSize
3641
doneChan chan struct{}
@@ -39,6 +44,8 @@ type terminalSession struct {
3944
writeLock sync.Mutex
4045
sessionManager *util_session.SessionManager
4146
token *string
47+
appRBACName string
48+
enf *rbac.Enforcer
4249
}
4350

4451
// getToken get auth token from web socket request
@@ -48,7 +55,7 @@ func getToken(r *http.Request) (string, error) {
4855
}
4956

5057
// newTerminalSession create terminalSession
51-
func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager) (*terminalSession, error) {
58+
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) {
5259
token, err := getToken(r)
5360
if err != nil {
5461
return nil, err
@@ -59,12 +66,15 @@ func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader h
5966
return nil, err
6067
}
6168
session := &terminalSession{
69+
ctx: ctx,
6270
wsConn: conn,
6371
tty: true,
6472
sizeChan: make(chan remotecommand.TerminalSize),
6573
doneChan: make(chan struct{}),
6674
sessionManager: sessionManager,
6775
token: &token,
76+
appRBACName: appRBACName,
77+
enf: enf,
6878
}
6979
return session, nil
7080
}
@@ -125,6 +135,29 @@ func (t *terminalSession) reconnect() (int, error) {
125135
return 0, nil
126136
}
127137

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

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

‎server/application/websocket_test.go

Lines changed: 115 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,65 @@
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) {
25+
func newTestTerminalSession(w http.ResponseWriter, r *http.Request) terminalSession {
1426
var 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

@@ -44,3 +86,71 @@ func TestReconnect(t *testing.T) {
4486
assert.Equal(t, message.Data, ReconnectMessage)
4587

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

0 commit comments

Comments
 (0)