Skip to content

Commit

Permalink
validate space names
Browse files Browse the repository at this point in the history
Signed-off-by: jkoberg <jkoberg@owncloud.com>
  • Loading branch information
kobergj committed Nov 2, 2022
1 parent ed42a02 commit 7df292e
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 1 deletion.
9 changes: 9 additions & 0 deletions changelog/unreleased/validate-space-names.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Enhancement: Validate space names

We now return BAD REQUEST when space names are not
- too long (max 255 characters)
- containing evil characters (/, \, ., \\, :, ?, *, ", >, <, |)

Additionally leading and trailing spaces will be removed silently.

https://github.com/owncloud/ocis/pull/4954
33 changes: 32 additions & 1 deletion services/graph/pkg/service/v0/drives.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ import (
merrors "go-micro.dev/v4/errors"
)

var (
_invalidSpaceNameCharacters = []string{`/`, `\`, `.`, `\\`, `:`, `?`, `*`, `"`, `>`, `<`, `|`}
_maxSpaceNameLength = 255

// ErrNameTooLong is thrown when the spacename is too long
ErrNameTooLong = fmt.Errorf("spacename must be smaller than %d", _maxSpaceNameLength)

// ErrForbiddenCharacter is thrown when the spacename contains an invalid character
ErrForbiddenCharacter = fmt.Errorf("spacenames must not contain %v", _invalidSpaceNameCharacters)
)

// GetDrives lists all drives the current user has access to
func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) {
g.getDrives(w, r, false)
Expand Down Expand Up @@ -229,13 +240,19 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) {
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid body schema definition")
return
}
spaceName := *drive.Name
spaceName := strings.TrimSpace(*drive.Name)
if spaceName == "" {
logger.Debug().Str("name", spaceName).Msg("could not create drive: invalid name")
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid name")
return
}

if err := validateSpaceName(spaceName); err != nil {
logger.Debug().Str("name", spaceName).Err(err).Msg("could not create drive: name validation failed")
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid spacename: %s", err.Error()))
return
}

var driveType string
if drive.DriveType != nil {
driveType = *drive.DriveType
Expand Down Expand Up @@ -901,3 +918,17 @@ func sortSpaces(req *godata.GoDataRequest, spaces []*libregraph.Drive) ([]*libre
sort.Sort(sorter)
return spaces, nil
}

func validateSpaceName(name string) error {
if len(name) > _maxSpaceNameLength {
return ErrNameTooLong
}

for _, c := range _invalidSpaceNameCharacters {
if strings.Contains(name, c) {
return ErrForbiddenCharacter
}
}

return nil
}
32 changes: 32 additions & 0 deletions services/graph/pkg/service/v0/drives_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/CiscoM31/godata"
libregraph "github.com/owncloud/libre-graph-api-go"
"github.com/stretchr/testify/assert"
"github.com/test-go/testify/require"
)

type sortTest struct {
Expand Down Expand Up @@ -121,3 +122,34 @@ func TestSort(t *testing.T) {
assert.Equal(t, test.DrivesSorted, sorted)
}
}

func TestSpaceNameValidation(t *testing.T) {
// set max length
_maxSpaceNameLength = 10

testCases := []struct {
Alias string
SpaceName string
ExpectedError error
}{
{"Happy Path", "Space", nil},
{"Just not too Long", "abcdefghij", nil},
{"Too Long", "abcdefghijk", ErrNameTooLong},
{"Contains /", "Space/", ErrForbiddenCharacter},
{`Contains \`, `Space\`, ErrForbiddenCharacter},
{`Contains \\`, `Space\\`, ErrForbiddenCharacter},
{"Contains .", "Space.", ErrForbiddenCharacter},
{"Contains :", "Space:", ErrForbiddenCharacter},
{"Contains ?", "Sp?ace", ErrForbiddenCharacter},
{"Contains *", "Spa*ce", ErrForbiddenCharacter},
{`Contains "`, `"Space"`, ErrForbiddenCharacter},
{`Contains >`, `Sp>ce`, ErrForbiddenCharacter},
{`Contains <`, `S<pce`, ErrForbiddenCharacter},
{`Contains |`, `S|p|e`, ErrForbiddenCharacter},
}

for _, tc := range testCases {
err := validateSpaceName(tc.SpaceName)
require.Equal(t, tc.ExpectedError, err, tc.Alias)
}
}

0 comments on commit 7df292e

Please sign in to comment.