Skip to content

Commit 5aba801

Browse files
committed
feat(backend): reject invalid HTTP path params in backend
Signed-off-by: Yael <fishel.yael@gmail.com>
1 parent e1da4ca commit 5aba801

File tree

6 files changed

+444
-14
lines changed

6 files changed

+444
-14
lines changed

workspaces/backend/api/errors.go

-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ func (a *App) LogError(r *http.Request, err error) {
4646
a.logger.Error(err.Error(), "method", method, "uri", uri)
4747
}
4848

49-
//nolint:unused
5049
func (a *App) badRequestResponse(w http.ResponseWriter, r *http.Request, err error) {
5150
httpError := &HTTPError{
5251
StatusCode: http.StatusBadRequest,

workspaces/backend/api/workspacekinds_handler.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ package api
1818

1919
import (
2020
"errors"
21-
"fmt"
2221
"net/http"
2322

2423
"github.com/julienschmidt/httprouter"
2524

25+
"github.com/kubeflow/notebooks/workspaces/backend/internal/helper"
2626
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/workspacekinds"
2727
repository "github.com/kubeflow/notebooks/workspaces/backend/internal/repositories/workspacekinds"
2828
)
@@ -34,8 +34,8 @@ type WorkspaceKindEnvelope Envelope[models.WorkspaceKind]
3434
func (a *App) GetWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
3535
name := ps.ByName("name")
3636

37-
if name == "" {
38-
a.serverErrorResponse(w, r, fmt.Errorf("workspace kind name is missing"))
37+
if err := helper.ValidateWorkspaceKind(name); err != nil {
38+
a.badRequestResponse(w, r, err)
3939
return
4040
}
4141

workspaces/backend/api/workspacekinds_handler_test.go

+109
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"encoding/json"
2121
"fmt"
2222
"io"
23+
"math/rand"
2324
"net/http"
2425
"net/http/httptest"
2526
"strings"
@@ -267,4 +268,112 @@ var _ = Describe("WorkspaceKinds Handler", func() {
267268
Expect(rs.StatusCode).To(Equal(http.StatusNotFound))
268269
})
269270
})
271+
272+
Context("with unsupported request parameters", Ordered, func() {
273+
274+
var (
275+
a App
276+
validResourceName string
277+
invalidResourceName string
278+
validMaxLengthName string
279+
invalidLengthName string
280+
)
281+
282+
// generateResourceName generates a random string of the specified length and allowed chars.
283+
generateResourceName := func(length int) string {
284+
const allowedChars = "abcdefghijklmnopqrstuvwxyz0123456789-"
285+
286+
var sb strings.Builder
287+
for i := 0; i < length; i++ {
288+
if i == 0 || i == length-1 {
289+
sb.WriteByte(allowedChars[rand.Intn(len(allowedChars)-1)])
290+
} else {
291+
sb.WriteByte(allowedChars[rand.Intn(len(allowedChars))])
292+
}
293+
}
294+
return sb.String()
295+
}
296+
297+
BeforeAll(func() {
298+
validResourceName = "test"
299+
invalidResourceName = validResourceName + string(rune(rand.Intn(0x10FFFF-128)+128))
300+
validMaxLengthName = generateResourceName(253)
301+
invalidLengthName = generateResourceName(254)
302+
303+
repos := repositories.NewRepositories(k8sClient)
304+
a = App{
305+
Config: config.EnvConfig{
306+
Port: 4000,
307+
},
308+
repositories: repos,
309+
}
310+
})
311+
312+
It("should return 400 status code for a invalid workspacekind name", func() {
313+
By("creating the HTTP request")
314+
path := strings.Replace(WorkspaceKindsByNamePath, ":"+WorkspaceNamePathParam, invalidResourceName, 1)
315+
req, err := http.NewRequest(http.MethodGet, path, http.NoBody)
316+
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")
317+
318+
By("executing GetWorkspaceKindHandler")
319+
ps := httprouter.Params{
320+
httprouter.Param{
321+
Key: WorkspaceNamePathParam,
322+
Value: invalidResourceName,
323+
},
324+
}
325+
rr := httptest.NewRecorder()
326+
a.GetWorkspaceKindHandler(rr, req, ps)
327+
rs := rr.Result()
328+
defer rs.Body.Close()
329+
330+
By("verifying the HTTP response status code")
331+
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), "Expected HTTP status 400 Bad Request")
332+
})
333+
334+
It("should return 400 status code for a workspace longer than 253", func() {
335+
By("creating the HTTP request")
336+
path := strings.Replace(WorkspaceKindsByNamePath, ":"+WorkspaceNamePathParam, invalidLengthName, 1)
337+
req, err := http.NewRequest(http.MethodGet, path, http.NoBody)
338+
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")
339+
340+
By("executing GetWorkspaceKindHandler")
341+
ps := httprouter.Params{
342+
httprouter.Param{
343+
Key: WorkspaceNamePathParam,
344+
Value: invalidLengthName,
345+
},
346+
}
347+
rr := httptest.NewRecorder()
348+
a.GetWorkspaceKindHandler(rr, req, ps)
349+
rs := rr.Result()
350+
defer rs.Body.Close()
351+
352+
By("verifying the HTTP response status code")
353+
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), "Expected HTTP status 400 Bad Request")
354+
355+
})
356+
357+
It("should return 200 status code for a workspace with a length of 253 characters", func() {
358+
By("creating the HTTP request")
359+
path := strings.Replace(WorkspaceKindsByNamePath, ":"+WorkspaceNamePathParam, validMaxLengthName, 1)
360+
req, err := http.NewRequest(http.MethodGet, path, http.NoBody)
361+
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")
362+
363+
By("executing GetWorkspaceKindHandler")
364+
ps := httprouter.Params{
365+
httprouter.Param{
366+
Key: WorkspaceNamePathParam,
367+
Value: validMaxLengthName,
368+
},
369+
}
370+
rr := httptest.NewRecorder()
371+
a.GetWorkspaceKindHandler(rr, req, ps)
372+
rs := rr.Result()
373+
defer rs.Body.Close()
374+
375+
By("verifying the HTTP response status code")
376+
Expect(rs.StatusCode).To(Equal(http.StatusNotFound), "Expected HTTP status 404 Not Found")
377+
})
378+
})
270379
})

workspaces/backend/api/workspaces_handler.go

+18-10
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/julienschmidt/httprouter"
2626

27+
"github.com/kubeflow/notebooks/workspaces/backend/internal/helper"
2728
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/workspaces"
2829
repository "github.com/kubeflow/notebooks/workspaces/backend/internal/repositories/workspaces"
2930
)
@@ -38,12 +39,9 @@ func (a *App) GetWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps htt
3839

3940
var workspace models.Workspace
4041
var err error
41-
if namespace == "" {
42-
a.serverErrorResponse(w, r, fmt.Errorf("namespace is nil"))
43-
return
44-
}
45-
if workspaceName == "" {
46-
a.serverErrorResponse(w, r, fmt.Errorf("workspaceName is nil"))
42+
43+
if err := helper.ValidateWorkspace(namespace, workspaceName); err != nil {
44+
a.badRequestResponse(w, r, err)
4745
return
4846
}
4947

@@ -71,6 +69,11 @@ func (a *App) GetWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps htt
7169
func (a *App) GetWorkspacesHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
7270
namespace := ps.ByName(NamespacePathParam)
7371

72+
if err := helper.ValidateNamespace(namespace, false); err != nil {
73+
a.badRequestResponse(w, r, err)
74+
return
75+
}
76+
7477
var workspaces []models.Workspace
7578
var err error
7679
if namespace == "" {
@@ -96,8 +99,8 @@ func (a *App) GetWorkspacesHandler(w http.ResponseWriter, r *http.Request, ps ht
9699
func (a *App) CreateWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
97100
namespace := ps.ByName("namespace")
98101

99-
if namespace == "" {
100-
a.serverErrorResponse(w, r, fmt.Errorf("namespace is missing"))
102+
if err := helper.ValidateNamespace(namespace, true); err != nil {
103+
a.badRequestResponse(w, r, err)
101104
return
102105
}
103106

@@ -107,6 +110,11 @@ func (a *App) CreateWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps
107110
return
108111
}
109112

113+
if err := helper.ValidateWorkspace(workspaceModel.Namespace, workspaceModel.Name); err != nil {
114+
a.badRequestResponse(w, r, err)
115+
return
116+
}
117+
110118
workspaceModel.Namespace = namespace
111119

112120
createdWorkspace, err := a.repositories.Workspace.CreateWorkspace(r.Context(), workspaceModel)
@@ -132,12 +140,12 @@ func (a *App) DeleteWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps
132140
workspaceName := ps.ByName("name")
133141

134142
if namespace == "" {
135-
a.serverErrorResponse(w, r, fmt.Errorf("namespace is missing"))
143+
a.badRequestResponse(w, r, fmt.Errorf("namespace is missing"))
136144
return
137145
}
138146

139147
if workspaceName == "" {
140-
a.serverErrorResponse(w, r, fmt.Errorf("workspace name is missing"))
148+
a.badRequestResponse(w, r, fmt.Errorf("workspace name is missing"))
141149
return
142150
}
143151

0 commit comments

Comments
 (0)