From 3151c8fe81a3fe97935319371e0abdf60d7c144d Mon Sep 17 00:00:00 2001 From: Michael Jerger Date: Fri, 8 Dec 2023 19:41:22 +0100 Subject: [PATCH] make validate more compact --- models/activitypub/actor.go | 72 ++++++++++++++++++++------------ models/activitypub/actor_test.go | 18 ++++++-- modules/validation/helpers.go | 15 +++++-- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/models/activitypub/actor.go b/models/activitypub/actor.go index 88d295e91b..b794adc247 100644 --- a/models/activitypub/actor.go +++ b/models/activitypub/actor.go @@ -19,12 +19,13 @@ type Validatable interface { // ToDo: What is the right package for this interfa } type ActorId struct { - userId string - source string - schema string - path string - host string - port string // optional + userId string + source string + schema string + path string + host string + port string + unvalidatedInput string } func validate_is_not_empty(str string) error { @@ -37,30 +38,24 @@ func validate_is_not_empty(str string) error { /* Validate collects error strings in a slice and returns this */ -func (a ActorId) Validate() []string { +func (value ActorId) Validate() []string { + var result = []string{} + result = append(result, validation.ValidateNotEmpty(value.userId, "userId")...) + result = append(result, validation.ValidateNotEmpty(value.source, "source")...) + result = append(result, validation.ValidateNotEmpty(value.schema, "schema")...) + result = append(result, validation.ValidateNotEmpty(value.path, "path")...) + result = append(result, validation.ValidateNotEmpty(value.host, "host")...) + result = append(result, validation.ValidateNotEmpty(value.unvalidatedInput, "unvalidatedInput")...) - var err = []string{} - - if res := validation.ValidateNotEmpty(a.schema, "schema"); res != nil { - err = append(err, res.Error()) - } - - if res := validate_is_not_empty(a.host); res != nil { - err = append(err, strings.Join([]string{res.Error(), "for host field"}, " ")) - } - - switch a.source { + result = append(result, validation.ValidateOneOf(value.source, []string{"forgejo", "gitea"})...) + switch value.source { case "forgejo", "gitea": - if !strings.Contains(a.path, "api/v1/activitypub/user-id") && - !strings.Contains(a.path, "api/v1/activitypub/repository-id") { - err = append(err, fmt.Errorf("the Path to the API was invalid: ---%v---", a.path).Error()) + if !strings.Contains(value.path, "api/v1/activitypub/user-id") { + result = append(result, fmt.Sprintf("path has to be a api path")) } - default: - err = append(err, fmt.Errorf("currently only forgeo and gitea sources are allowed from actor id").Error()) } - return err - + return result } /* @@ -173,5 +168,30 @@ func NewActorId(uri string, source string) (ActorId, error) { if !validation.IsValidExternalURL(uri) { return ActorId{}, fmt.Errorf("uri %s is not a valid external url", uri) } - return ActorId{}, nil + + validatedUri, _ := url.Parse(uri) + pathWithUserID := strings.Split(validatedUri.Path, "/") + + if containsEmptyString(pathWithUserID) { + pathWithUserID = removeEmptyStrings(pathWithUserID) + } + + length := len(pathWithUserID) + pathWithoutUserID := strings.Join(pathWithUserID[0:length-1], "/") + userId := pathWithUserID[length-1] + + actorId := ActorId{ + userId: userId, + source: source, + schema: validatedUri.Scheme, + host: validatedUri.Hostname(), + path: pathWithoutUserID, + port: validatedUri.Port(), + unvalidatedInput: uri, + } + if valid, err := actorId.IsValid(); !valid { + return ActorId{}, err + } + + return actorId, nil } diff --git a/models/activitypub/actor_test.go b/models/activitypub/actor_test.go index 0d6123f359..4ac241e5d2 100644 --- a/models/activitypub/actor_test.go +++ b/models/activitypub/actor_test.go @@ -130,11 +130,23 @@ func TestShouldThrowErrorOnInvalidInput(t *testing.T) { } _, err = NewActorId("./api/v1/something", "forgejo") if err == nil { - t.Errorf("relative uris are not alowed.") + t.Errorf("relative uris are not alowed") + } + _, err = NewActorId("http://1.2.3.4/api/v1/something", "forgejo") + if err == nil { + t.Errorf("uri may not be ip-4 based") + } + _, err = NewActorId("http:///[fe80::1ff:fe23:4567:890a%25eth0]/api/v1/something", "forgejo") + if err == nil { + t.Errorf("uri may not be ip-6 based") + } + _, err = NewActorId("https://codeberg.org/api/v1/activitypub/../activitypub/user-id/12345", "forgejo") + if err == nil { + t.Errorf("uri may not contain relative path elements") } - _, err = NewActorId("https://an.other.host/api/v1/activitypub/person-id/1", "forgejo") + _, err = NewActorId("https://an.other.host/api/v1/activitypub/user-id/1", "forgejo") if err != nil { - t.Errorf("this uri should be valid") + t.Errorf("this uri should be valid but was: %v", err) } } diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index 7ea02dab02..d22520bc33 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -136,9 +136,18 @@ func IsValidUsername(name string) bool { return validUsernamePatternWithoutDots.MatchString(name) && !invalidUsernamePattern.MatchString(name) } -func ValidateNotEmpty(value string, fieldName string) error { +func ValidateNotEmpty(value string, fieldName string) []string { if value == "" { - return fmt.Errorf("Field %v may not be empty.", fieldName) + return []string{fmt.Sprintf("Field %v may not be empty.", fieldName)} } - return nil + return []string{} +} + +func ValidateOneOf(value string, allowed []string) []string { + for _, allowedElem := range allowed { + if value == allowedElem { + return []string{} + } + } + return []string{fmt.Sprintf("Value %v is not contained in allowed values [%v]", value, allowed)} }