Skip to content

Commit 6b34333

Browse files
mjudeikis-botOpenClaw Botmjudeikis
authored
fix(security): route bare-path OIDC requests to user workspace, not kcp root (#65) (#87)
serveOIDC lacked the else-branch that serveStaticToken already had: bare paths (/api/..., /apis/...) were forwarded to kcp root with an empty kcpPath instead of the user's tenant workspace. Add the same else-branch to scope all bare-path requests to the user's default cluster. Refactor: extract resolveKCPPath() as a shared, tested helper used by both serveOIDC and serveStaticToken, eliminating the duplicated path- routing logic between the two handlers. Co-authored-by: OpenClaw Bot <bot@openclaw.ai> Co-authored-by: Mangirdas Judeikis <mangirdas@judeikis.lt>
1 parent 18fc175 commit 6b34333

2 files changed

Lines changed: 177 additions & 51 deletions

File tree

‎pkg/server/proxy/proxy.go‎

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -178,31 +178,17 @@ func (p *KCPProxy) serveOIDC(w http.ResponseWriter, r *http.Request, token strin
178178
return
179179
}
180180

181-
// TODO(#65): bare paths (/api/..., /apis/...) are not handled here — kcpPath stays
182-
// empty and the request is forwarded to the kcp root URL instead of the user's
183-
// workspace. Add the same else-branch as serveStaticToken:
184-
// kcpPath = "/clusters/" + user.Spec.DefaultCluster + r.URL.Path
185-
// https://github.com/faroshq/kedge/issues/65
186-
187-
// Determine kcp path based on incoming request format.
188-
var kcpPath string
189-
if strings.HasPrefix(r.URL.Path, "/clusters/") {
190-
// kcp-syntax: /clusters/{logicalClusterName}/api/...
191-
rest := strings.TrimPrefix(r.URL.Path, "/clusters/")
192-
slashIdx := strings.Index(rest, "/")
193-
clusterID := rest
194-
if slashIdx >= 0 {
195-
clusterID = rest[:slashIdx]
196-
}
197-
// Allow exact match or mount access ({clusterName}:{mountName}).
198-
if clusterID != user.Spec.DefaultCluster && !strings.HasPrefix(clusterID, user.Spec.DefaultCluster+":") {
199-
p.logger.Info("cluster access denied", "user", user.Name, "requested", clusterID, "allowed", user.Spec.DefaultCluster)
200-
w.Header().Set("Content-Type", "application/json")
201-
w.WriteHeader(http.StatusForbidden)
202-
_, _ = fmt.Fprint(w, `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"cluster access denied","reason":"Forbidden","code":403}`)
203-
return
181+
kcpPath, errStatus, errBody := resolveKCPPath(r.URL.Path, user.Spec.DefaultCluster)
182+
if errStatus != 0 {
183+
if strings.HasPrefix(r.URL.Path, "/clusters/") {
184+
p.logger.Info("cluster access denied", "user", user.Name, "path", r.URL.Path, "allowed", user.Spec.DefaultCluster)
185+
} else {
186+
p.logger.Error(nil, "user has no default cluster", "user", user.Name)
204187
}
205-
kcpPath = r.URL.Path // already in /clusters/{id}/... format
188+
w.Header().Set("Content-Type", "application/json")
189+
w.WriteHeader(errStatus)
190+
_, _ = fmt.Fprint(w, errBody)
191+
return
206192
}
207193

208194
target := *p.kcpTarget
@@ -253,36 +239,17 @@ func (p *KCPProxy) serveStaticToken(w http.ResponseWriter, r *http.Request, toke
253239
return
254240
}
255241

256-
// Determine kcp path based on incoming request format.
257-
var kcpPath string
258-
if strings.HasPrefix(r.URL.Path, "/clusters/") {
259-
// kcp-syntax: /clusters/{logicalClusterName}/api/...
260-
rest := strings.TrimPrefix(r.URL.Path, "/clusters/")
261-
slashIdx := strings.Index(rest, "/")
262-
clusterID := rest
263-
if slashIdx >= 0 {
264-
clusterID = rest[:slashIdx]
265-
}
266-
// Allow exact match or mount access ({clusterName}:{mountName}).
267-
if clusterID != user.Spec.DefaultCluster && !strings.HasPrefix(clusterID, user.Spec.DefaultCluster+":") {
268-
p.logger.Info("cluster access denied", "user", user.Name, "requested", clusterID, "allowed", user.Spec.DefaultCluster)
269-
w.Header().Set("Content-Type", "application/json")
270-
w.WriteHeader(http.StatusForbidden)
271-
_, _ = fmt.Fprint(w, `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"cluster access denied","reason":"Forbidden","code":403}`)
272-
return
273-
}
274-
kcpPath = r.URL.Path // already in /clusters/{id}/... format
275-
} else {
276-
// Bare path: construct workspace path from user's default cluster.
277-
if user.Spec.DefaultCluster != "" {
278-
kcpPath = "/clusters/" + user.Spec.DefaultCluster + r.URL.Path
242+
kcpPath, errStatus, errBody := resolveKCPPath(r.URL.Path, user.Spec.DefaultCluster)
243+
if errStatus != 0 {
244+
if strings.HasPrefix(r.URL.Path, "/clusters/") {
245+
p.logger.Info("cluster access denied", "user", user.Name, "path", r.URL.Path, "allowed", user.Spec.DefaultCluster)
279246
} else {
280247
p.logger.Error(nil, "user has no default cluster", "user", user.Name)
281-
w.Header().Set("Content-Type", "application/json")
282-
w.WriteHeader(http.StatusForbidden)
283-
_, _ = fmt.Fprint(w, `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"user workspace not configured","reason":"Forbidden","code":403}`)
284-
return
285248
}
249+
w.Header().Set("Content-Type", "application/json")
250+
w.WriteHeader(errStatus)
251+
_, _ = fmt.Fprint(w, errBody)
252+
return
286253
}
287254

288255
target := *p.kcpTarget
@@ -524,6 +491,39 @@ func writeUnauthorized(w http.ResponseWriter) {
524491
_, _ = fmt.Fprint(w, `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Unauthorized","reason":"Unauthorized","code":401}`)
525492
}
526493

494+
// resolveKCPPath computes the target kcp path for the given request URL path.
495+
//
496+
// Two formats are accepted:
497+
// - /clusters/{logicalClusterName}/... — validated against defaultCluster;
498+
// returns the original path unchanged on success.
499+
// - /api/... or /apis/... (bare paths) — prepended with
500+
// /clusters/{defaultCluster}; returns 403 if defaultCluster is empty.
501+
//
502+
// Returns (kcpPath, 0, "") on success, or ("", httpStatus, jsonBody) on error.
503+
// The caller is responsible for logging context and writing the HTTP response.
504+
func resolveKCPPath(urlPath, defaultCluster string) (string, int, string) {
505+
if strings.HasPrefix(urlPath, "/clusters/") {
506+
// kcp-syntax: validate that the requested cluster matches the user's workspace.
507+
rest := strings.TrimPrefix(urlPath, "/clusters/")
508+
slashIdx := strings.Index(rest, "/")
509+
clusterID := rest
510+
if slashIdx >= 0 {
511+
clusterID = rest[:slashIdx]
512+
}
513+
// Allow exact match or mount access ({clusterName}:{mountName}).
514+
if clusterID != defaultCluster && !strings.HasPrefix(clusterID, defaultCluster+":") {
515+
return "", http.StatusForbidden, `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"cluster access denied","reason":"Forbidden","code":403}`
516+
}
517+
return urlPath, 0, ""
518+
}
519+
520+
// Bare path: scope to the user's default cluster.
521+
if defaultCluster != "" {
522+
return "/clusters/" + defaultCluster + urlPath, 0, ""
523+
}
524+
return "", http.StatusForbidden, `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"user has no default cluster","reason":"Forbidden","code":403}`
525+
}
526+
527527
// resolveUser looks up the User CRD by OIDC issuer+sub hash and returns the full User object.
528528
func (p *KCPProxy) resolveUser(ctx context.Context, issuer, sub string) (*tenancyv1alpha1.User, error) {
529529
hash := sha256.Sum256([]byte(issuer + "/" + sub))

‎pkg/server/proxy/proxy_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
Copyright 2026 The Faros Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package proxy
18+
19+
import (
20+
"encoding/json"
21+
"net/http"
22+
"testing"
23+
)
24+
25+
// TestResolveKCPPath tests the path-routing logic used by serveOIDC and
26+
// serveStaticToken to scope incoming requests to the correct tenant workspace.
27+
//
28+
// This covers the fix for issue #65: bare paths (/api/..., /apis/...) must be
29+
// routed to the user's default cluster, not forwarded to kcp root.
30+
func TestResolveKCPPath(t *testing.T) {
31+
const defaultCluster = "root:tenant-abc"
32+
33+
tests := []struct {
34+
name string
35+
urlPath string
36+
defaultCluster string
37+
wantPath string
38+
wantStatus int // 0 means success
39+
wantMsgSubstr string
40+
}{
41+
// ── issue #65 regression: bare paths must route to the user workspace ───
42+
{
43+
name: "bare /api path routes to user workspace",
44+
urlPath: "/api/v1/pods",
45+
defaultCluster: defaultCluster,
46+
wantPath: "/clusters/" + defaultCluster + "/api/v1/pods",
47+
wantStatus: 0,
48+
},
49+
{
50+
name: "bare /apis path routes to user workspace",
51+
urlPath: "/apis/apps/v1/deployments",
52+
defaultCluster: defaultCluster,
53+
wantPath: "/clusters/" + defaultCluster + "/apis/apps/v1/deployments",
54+
wantStatus: 0,
55+
},
56+
{
57+
name: "bare path with empty defaultCluster returns 403",
58+
urlPath: "/api/v1/pods",
59+
defaultCluster: "",
60+
wantStatus: http.StatusForbidden,
61+
wantMsgSubstr: "user has no default cluster",
62+
},
63+
// ── /clusters/{id}/... paths ─────────────────────────────────────────────
64+
{
65+
name: "cluster-syntax path with matching cluster passes through",
66+
urlPath: "/clusters/" + defaultCluster + "/api/v1/pods",
67+
defaultCluster: defaultCluster,
68+
wantPath: "/clusters/" + defaultCluster + "/api/v1/pods",
69+
wantStatus: 0,
70+
},
71+
{
72+
name: "cluster-syntax path with mount suffix passes through",
73+
urlPath: "/clusters/" + defaultCluster + ":mount1/api/v1/pods",
74+
defaultCluster: defaultCluster,
75+
wantPath: "/clusters/" + defaultCluster + ":mount1/api/v1/pods",
76+
wantStatus: 0,
77+
},
78+
{
79+
name: "cluster-syntax path with wrong cluster returns 403",
80+
urlPath: "/clusters/root:other-tenant/api/v1/pods",
81+
defaultCluster: defaultCluster,
82+
wantStatus: http.StatusForbidden,
83+
wantMsgSubstr: "cluster access denied",
84+
},
85+
{
86+
name: "cluster-syntax path with no trailing slash (bare cluster) passes",
87+
urlPath: "/clusters/" + defaultCluster,
88+
defaultCluster: defaultCluster,
89+
wantPath: "/clusters/" + defaultCluster,
90+
wantStatus: 0,
91+
},
92+
}
93+
94+
for _, tc := range tests {
95+
t.Run(tc.name, func(t *testing.T) {
96+
gotPath, gotStatus, gotBody := resolveKCPPath(tc.urlPath, tc.defaultCluster)
97+
98+
if tc.wantStatus == 0 {
99+
// Expect success.
100+
if gotStatus != 0 {
101+
t.Fatalf("expected success (status 0), got status %d body %q", gotStatus, gotBody)
102+
}
103+
if gotPath != tc.wantPath {
104+
t.Errorf("kcpPath: got %q, want %q", gotPath, tc.wantPath)
105+
}
106+
} else {
107+
// Expect error.
108+
if gotStatus != tc.wantStatus {
109+
t.Errorf("status: got %d, want %d", gotStatus, tc.wantStatus)
110+
}
111+
if gotPath != "" {
112+
t.Errorf("expected empty kcpPath on error, got %q", gotPath)
113+
}
114+
// Verify the response body is valid JSON and contains the expected message.
115+
var status map[string]interface{}
116+
if err := json.Unmarshal([]byte(gotBody), &status); err != nil {
117+
t.Fatalf("response body is not valid JSON: %v — body: %q", err, gotBody)
118+
}
119+
msg, _ := status["message"].(string)
120+
if tc.wantMsgSubstr != "" && msg != tc.wantMsgSubstr {
121+
t.Errorf("message: got %q, want %q", msg, tc.wantMsgSubstr)
122+
}
123+
}
124+
})
125+
}
126+
}

0 commit comments

Comments
 (0)