Skip to content

Commit 4ed3287

Browse files
committed
Support revoke intercept
Signed-off-by: Phan Duc <phan.duc@moneyforward.co.jp>
1 parent 894c6ec commit 4ed3287

File tree

6 files changed

+770
-256
lines changed

6 files changed

+770
-256
lines changed
Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
package manager
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
"google.golang.org/grpc/codes"
9+
"google.golang.org/grpc/status"
10+
authv1 "k8s.io/api/authentication/v1"
11+
"k8s.io/apimachinery/pkg/runtime"
12+
"k8s.io/client-go/kubernetes/fake"
13+
k8stesting "k8s.io/client-go/testing"
14+
15+
"github.com/datawire/dlib/dgroup"
16+
"github.com/datawire/dlib/dlog"
17+
rpc "github.com/telepresenceio/telepresence/rpc/v2/manager"
18+
"github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager/state"
19+
"github.com/telepresenceio/telepresence/v2/pkg/k8sapi"
20+
)
21+
22+
func TestRevokeIntercept_Authentication(t *testing.T) {
23+
tests := []struct {
24+
name string
25+
token string
26+
authenticated bool
27+
username string
28+
groups []string
29+
wantCode codes.Code
30+
wantErr bool
31+
errMessage string
32+
}{
33+
{
34+
name: "authorized user with system:masters",
35+
token: "valid-masters-token",
36+
authenticated: true,
37+
username: "admin-user",
38+
groups: []string{"system:authenticated", "system:masters"},
39+
wantCode: codes.NotFound, // Will fail on intercept lookup, but auth passes
40+
wantErr: true,
41+
errMessage: "not found",
42+
},
43+
{
44+
name: "authorized user with telepresence:admin",
45+
token: "valid-admin-token",
46+
authenticated: true,
47+
username: "telepresence-admin",
48+
groups: []string{"system:authenticated", "telepresence:admin"},
49+
wantCode: codes.NotFound, // Will fail on intercept lookup, but auth passes
50+
wantErr: true,
51+
errMessage: "not found",
52+
},
53+
{
54+
name: "unauthorized user - not in allowed groups",
55+
token: "valid-user-token",
56+
authenticated: true,
57+
username: "regular-user",
58+
groups: []string{"system:authenticated", "developers"},
59+
wantCode: codes.PermissionDenied,
60+
wantErr: true,
61+
errMessage: "user must be a member of telepresence:admin or system:masters group",
62+
},
63+
{
64+
name: "unauthenticated token",
65+
token: "invalid-token",
66+
authenticated: false,
67+
username: "",
68+
groups: []string{},
69+
wantCode: codes.PermissionDenied,
70+
wantErr: true,
71+
errMessage: "authentication failed",
72+
},
73+
{
74+
name: "empty token",
75+
token: "",
76+
authenticated: false,
77+
username: "",
78+
groups: []string{},
79+
wantCode: codes.PermissionDenied,
80+
wantErr: true,
81+
errMessage: "authentication failed",
82+
},
83+
{
84+
name: "user with both groups",
85+
token: "super-admin-token",
86+
authenticated: true,
87+
username: "super-admin",
88+
groups: []string{"system:authenticated", "system:masters", "telepresence:admin"},
89+
wantCode: codes.NotFound, // Will fail on intercept lookup, but auth passes
90+
wantErr: true,
91+
errMessage: "not found",
92+
},
93+
}
94+
95+
for _, tt := range tests {
96+
t.Run(tt.name, func(t *testing.T) {
97+
ctx := dlog.NewTestContext(t, true)
98+
99+
// Create fake Kubernetes client with TokenReview reactor
100+
fakeClient := fake.NewClientset()
101+
fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
102+
createAction := action.(k8stesting.CreateAction)
103+
tr := createAction.GetObject().(*authv1.TokenReview)
104+
105+
// Simulate TokenReview API response based on the token
106+
if tr.Spec.Token == tt.token && tt.authenticated {
107+
tr.Status = authv1.TokenReviewStatus{
108+
Authenticated: true,
109+
User: authv1.UserInfo{
110+
Username: tt.username,
111+
Groups: tt.groups,
112+
},
113+
}
114+
} else {
115+
tr.Status = authv1.TokenReviewStatus{
116+
Authenticated: false,
117+
}
118+
}
119+
120+
return true, tr, nil
121+
})
122+
123+
// Set up context with fake K8s client
124+
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
125+
126+
// Create a minimal service instance
127+
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
128+
svc := &service{
129+
state: state.NewState(ctx, g),
130+
}
131+
132+
// Call RevokeIntercept
133+
req := &rpc.RevokeInterceptRequest{
134+
InterceptId: "test-session:test-intercept",
135+
Token: tt.token,
136+
}
137+
138+
_, err := svc.RevokeIntercept(ctx, req)
139+
140+
// Verify results
141+
if tt.wantErr {
142+
require.Error(t, err, "expected error but got none")
143+
st, ok := status.FromError(err)
144+
require.True(t, ok, "error should be a gRPC status error")
145+
assert.Equal(t, tt.wantCode, st.Code(), "unexpected error code")
146+
assert.Contains(t, st.Message(), tt.errMessage, "error message mismatch")
147+
} else {
148+
require.NoError(t, err, "expected no error")
149+
}
150+
})
151+
}
152+
}
153+
154+
func TestRevokeIntercept_SuccessfulRevocation(t *testing.T) {
155+
ctx := dlog.NewTestContext(t, true)
156+
require := require.New(t)
157+
158+
// Create fake Kubernetes client with TokenReview reactor
159+
fakeClient := fake.NewClientset()
160+
fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
161+
createAction := action.(k8stesting.CreateAction)
162+
tr := createAction.GetObject().(*authv1.TokenReview)
163+
164+
// Always return authenticated with system:masters
165+
tr.Status = authv1.TokenReviewStatus{
166+
Authenticated: true,
167+
User: authv1.UserInfo{
168+
Username: "admin",
169+
Groups: []string{"system:authenticated", "system:masters"},
170+
},
171+
}
172+
173+
return true, tr, nil
174+
})
175+
176+
// Set up context with fake K8s client
177+
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
178+
179+
// Create a service instance with state
180+
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
181+
svc := &service{
182+
state: state.NewState(ctx, g),
183+
}
184+
185+
// Create a test intercept in the state
186+
testInterceptID := "test-session:test-intercept"
187+
188+
// Create a mock client session
189+
clientInfo := &rpc.ClientInfo{
190+
Name: "test-client",
191+
InstallId: "test-install-id",
192+
}
193+
194+
sessionInfo := &rpc.SessionInfo{
195+
SessionId: "test-session",
196+
}
197+
198+
// Create intercept spec
199+
interceptSpec := &rpc.InterceptSpec{
200+
Name: "test-intercept",
201+
Namespace: "default",
202+
Client: sessionInfo.SessionId,
203+
}
204+
205+
// Manually add an intercept to the state for testing
206+
// Note: This requires accessing internal state methods
207+
// In a real scenario, you would use the proper CreateIntercept flow
208+
209+
// For now, we'll test that a non-existent intercept returns NotFound
210+
// which proves authentication worked (if auth failed, we'd get PermissionDenied)
211+
req := &rpc.RevokeInterceptRequest{
212+
InterceptId: testInterceptID,
213+
Token: "valid-token",
214+
}
215+
216+
_, err := svc.RevokeIntercept(ctx, req)
217+
218+
// Should get NotFound since we didn't actually create the intercept
219+
// This proves authentication passed (otherwise we'd get PermissionDenied)
220+
require.Error(err)
221+
st, ok := status.FromError(err)
222+
require.True(ok)
223+
require.Equal(codes.NotFound, st.Code())
224+
require.Contains(st.Message(), "not found")
225+
226+
// Log for debugging
227+
t.Logf("Successfully verified that authenticated user can attempt to revoke intercept")
228+
t.Logf("Client: %v, Session: %v, Spec: %v", clientInfo, sessionInfo, interceptSpec)
229+
}
230+
231+
func TestRevokeIntercept_OnlySystemMasters(t *testing.T) {
232+
// This test specifically verifies that ONLY system:masters (or telepresence:admin) can revoke
233+
unauthorizedGroups := [][]string{
234+
{"system:authenticated"},
235+
{"system:authenticated", "developers"},
236+
{"system:authenticated", "system:nodes"},
237+
{"admin"}, // Not system:masters
238+
{"telepresence:user"},
239+
{"system:serviceaccounts"},
240+
}
241+
242+
for i, groups := range unauthorizedGroups {
243+
t.Run(t.Name()+"_"+string(rune('A'+i)), func(t *testing.T) {
244+
ctx := dlog.NewTestContext(t, true)
245+
246+
fakeClient := fake.NewClientset()
247+
fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
248+
createAction := action.(k8stesting.CreateAction)
249+
tr := createAction.GetObject().(*authv1.TokenReview)
250+
251+
tr.Status = authv1.TokenReviewStatus{
252+
Authenticated: true,
253+
User: authv1.UserInfo{
254+
Username: "test-user",
255+
Groups: groups,
256+
},
257+
}
258+
259+
return true, tr, nil
260+
})
261+
262+
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
263+
264+
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
265+
svc := &service{
266+
state: state.NewState(ctx, g),
267+
}
268+
269+
req := &rpc.RevokeInterceptRequest{
270+
InterceptId: "test:intercept",
271+
Token: "token",
272+
}
273+
274+
_, err := svc.RevokeIntercept(ctx, req)
275+
276+
require.Error(t, err)
277+
st, ok := status.FromError(err)
278+
require.True(t, ok)
279+
assert.Equal(t, codes.PermissionDenied, st.Code())
280+
assert.Contains(t, st.Message(), "user must be a member of telepresence:admin or system:masters group")
281+
282+
t.Logf("Correctly denied access for groups: %v", groups)
283+
})
284+
}
285+
}

cmd/traffic/cmd/manager/service.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,47 @@ func (s *service) RemoveIntercept(ctx context.Context, riReq *rpc.RemoveIntercep
882882
return &empty.Empty{}, nil
883883
}
884884

885+
// RevokeIntercept allows the manager to revoke any client's intercept by intercept ID.
886+
// This is an administrative operation that can revoke intercepts for any client.
887+
// Requires authentication via token and membership in system:telepresence-admin group.
888+
func (s *service) RevokeIntercept(ctx context.Context, riReq *rpc.RevokeInterceptRequest) (*empty.Empty, error) {
889+
// Verify the authentication token
890+
tokenResult := k8sapi.VerifyToken(ctx, riReq.Token)
891+
if tokenResult.Error != nil || !tokenResult.Authenticated {
892+
dlog.Warnf(ctx, "Authentication failed for RevokeIntercept: %v", tokenResult.Error)
893+
return nil, status.Errorf(codes.PermissionDenied, "authentication failed")
894+
}
895+
896+
// Check if user belongs to system:telepresence-admin group
897+
if !(k8sapi.IsMemberOfGroup(tokenResult.Groups, "telepresence:admin") || k8sapi.IsMemberOfGroup(tokenResult.Groups, "system:masters")) {
898+
dlog.Warnf(ctx, "User %s is not a member of telepresence:admin or system:masters group", tokenResult.Username)
899+
return nil, status.Errorf(codes.PermissionDenied, "user must be a member of telepresence:admin or system:masters group")
900+
}
901+
902+
dlog.Infof(ctx, "User %s authorized to revoke intercepts", tokenResult.Username)
903+
904+
interceptID := riReq.InterceptId
905+
dlog.Debugf(ctx, "Revoking intercept ID %s", interceptID)
906+
907+
// Get the intercept to verify it exists and to update metrics
908+
intercept, ok := s.state.GetIntercept(interceptID)
909+
if !ok {
910+
return nil, status.Errorf(codes.NotFound, "Intercept with ID %q not found", interceptID)
911+
}
912+
913+
// Get the client session from the intercept to update metrics
914+
clientSessionID := tunnel.SessionID(intercept.ClientSession.SessionId)
915+
if client := s.state.GetClient(clientSessionID); client != nil {
916+
interceptName := intercept.Spec.Name
917+
SetGauge(ctx, s.state.GetInterceptActiveStatus(), client.Name, client.InstallId, &interceptName, 0)
918+
}
919+
920+
// Remove the intercept
921+
s.state.RemoveIntercept(ctx, interceptID)
922+
dlog.Infof(ctx, "Successfully revoked intercept ID %s", interceptID)
923+
return &empty.Empty{}, nil
924+
}
925+
885926
// GetIntercept gets an intercept info from intercept name.
886927
func (s *service) GetIntercept(ctx context.Context, request *rpc.GetInterceptRequest) (*rpc.InterceptInfo, error) {
887928
interceptID, err := s.MakeInterceptID(ctx, request.GetSession().GetSessionId(), request.GetName())

0 commit comments

Comments
 (0)