Skip to content

Commit

Permalink
feat(backend): reject invalid HTTP path params in backend
Browse files Browse the repository at this point in the history
Signed-off-by: Yael <[email protected]>
  • Loading branch information
Yael-F committed Feb 5, 2025
1 parent e1da4ca commit ba90176
Show file tree
Hide file tree
Showing 6 changed files with 444 additions and 14 deletions.
1 change: 0 additions & 1 deletion workspaces/backend/api/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func (a *App) LogError(r *http.Request, err error) {
a.logger.Error(err.Error(), "method", method, "uri", uri)
}

//nolint:unused
func (a *App) badRequestResponse(w http.ResponseWriter, r *http.Request, err error) {
httpError := &HTTPError{
StatusCode: http.StatusBadRequest,
Expand Down
6 changes: 3 additions & 3 deletions workspaces/backend/api/workspacekinds_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ package api

import (
"errors"
"fmt"
"net/http"

"github.com/julienschmidt/httprouter"

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

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

Expand Down
109 changes: 109 additions & 0 deletions workspaces/backend/api/workspacekinds_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"fmt"
"io"
"math/rand"
"net/http"
"net/http/httptest"
"strings"
Expand Down Expand Up @@ -267,4 +268,112 @@ var _ = Describe("WorkspaceKinds Handler", func() {
Expect(rs.StatusCode).To(Equal(http.StatusNotFound))
})
})

Context("with unsupported request parameters", Ordered, func() {

var (
a App
validResourceName string
invalidResourceName string
validMaxLengthName string
invalidLengthName string
)

// generateResourceName generates a random string of the specified length and allowed chars.
generateResourceName := func(length int) string {
const allowedChars = "abcdefghijklmnopqrstuvwxyz0123456789-"

var sb strings.Builder
for i := 0; i < length; i++ {
if i == 0 || i == length-1 {
sb.WriteByte(allowedChars[rand.Intn(len(allowedChars)-1)])
} else {
sb.WriteByte(allowedChars[rand.Intn(len(allowedChars))])
}
}
return sb.String()
}

BeforeAll(func() {
validResourceName = "test"
invalidResourceName = validResourceName + string(rune(rand.Intn(0x10FFFF-128)+128))
validMaxLengthName = generateResourceName(253)
invalidLengthName = generateResourceName(254)

repos := repositories.NewRepositories(k8sClient)
a = App{
Config: config.EnvConfig{
Port: 4000,
},
repositories: repos,
}
})

It("should return 400 status code for a invalid workspacekind name", func() {
By("creating the HTTP request")
path := strings.Replace(WorkspaceKindsByNamePath, ":"+WorkspaceNamePathParam, invalidResourceName, 1)
req, err := http.NewRequest(http.MethodGet, path, http.NoBody)
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")

By("executing GetWorkspaceKindHandler")
ps := httprouter.Params{
httprouter.Param{
Key: WorkspaceNamePathParam,
Value: invalidResourceName,
},
}
rr := httptest.NewRecorder()
a.GetWorkspaceKindHandler(rr, req, ps)
rs := rr.Result()
defer rs.Body.Close()

By("verifying the HTTP response status code")
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), "Expected HTTP status 400 Bad Request")
})

It("should return 400 status code for a workspace longer than 253", func() {
By("creating the HTTP request")
path := strings.Replace(WorkspaceKindsByNamePath, ":"+WorkspaceNamePathParam, invalidLengthName, 1)
req, err := http.NewRequest(http.MethodGet, path, http.NoBody)
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")

By("executing GetWorkspaceKindHandler")
ps := httprouter.Params{
httprouter.Param{
Key: WorkspaceNamePathParam,
Value: invalidLengthName,
},
}
rr := httptest.NewRecorder()
a.GetWorkspaceKindHandler(rr, req, ps)
rs := rr.Result()
defer rs.Body.Close()

By("verifying the HTTP response status code")
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), "Expected HTTP status 400 Bad Request")

})

It("should return 200 status code for a workspace with a length of 253 characters", func() {
By("creating the HTTP request")
path := strings.Replace(WorkspaceKindsByNamePath, ":"+WorkspaceNamePathParam, validMaxLengthName, 1)
req, err := http.NewRequest(http.MethodGet, path, http.NoBody)
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")

By("executing GetWorkspaceKindHandler")
ps := httprouter.Params{
httprouter.Param{
Key: WorkspaceNamePathParam,
Value: validMaxLengthName,
},
}
rr := httptest.NewRecorder()
a.GetWorkspaceKindHandler(rr, req, ps)
rs := rr.Result()
defer rs.Body.Close()

By("verifying the HTTP response status code")
Expect(rs.StatusCode).To(Equal(http.StatusNotFound), "Expected HTTP status 404 Not Found")
})
})
})
28 changes: 18 additions & 10 deletions workspaces/backend/api/workspaces_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/julienschmidt/httprouter"

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

var workspace models.Workspace
var err error
if namespace == "" {
a.serverErrorResponse(w, r, fmt.Errorf("namespace is nil"))
return
}
if workspaceName == "" {
a.serverErrorResponse(w, r, fmt.Errorf("workspaceName is nil"))

if err := helper.ValidateWorkspace(namespace, workspaceName); err != nil {
a.badRequestResponse(w, r, err)
return
}

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

if err := helper.ValidateNamespace(namespace, false); err != nil {
a.badRequestResponse(w, r, err)
return
}

var workspaces []models.Workspace
var err error
if namespace == "" {
Expand All @@ -96,8 +99,8 @@ func (a *App) GetWorkspacesHandler(w http.ResponseWriter, r *http.Request, ps ht
func (a *App) CreateWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
namespace := ps.ByName("namespace")

if namespace == "" {
a.serverErrorResponse(w, r, fmt.Errorf("namespace is missing"))
if err := helper.ValidateNamespace(namespace, true); err != nil {
a.badRequestResponse(w, r, err)
return
}

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

if err := helper.ValidateWorkspace(workspaceModel.Namespace, workspaceModel.Name); err != nil {
a.badRequestResponse(w, r, err)
return
}

workspaceModel.Namespace = namespace

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

if namespace == "" {
a.serverErrorResponse(w, r, fmt.Errorf("namespace is missing"))
a.badRequestResponse(w, r, fmt.Errorf("namespace is missing"))
return
}

if workspaceName == "" {
a.serverErrorResponse(w, r, fmt.Errorf("workspace name is missing"))
a.badRequestResponse(w, r, fmt.Errorf("workspace name is missing"))
return
}

Expand Down
Loading

0 comments on commit ba90176

Please sign in to comment.