From 620940ec3ffb7a7aa3d9316c4650e77f4e5da1f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Mon, 3 Apr 2023 11:11:09 +0200 Subject: [PATCH] graph: Add strict json decoding to return errors when receiving unknown parameters --- services/graph/pkg/service/v0/approleassignments.go | 3 +-- services/graph/pkg/service/v0/drives.go | 4 ++-- services/graph/pkg/service/v0/educationclasses.go | 9 ++++----- services/graph/pkg/service/v0/educationschools.go | 9 ++++----- services/graph/pkg/service/v0/educationuser.go | 5 ++--- services/graph/pkg/service/v0/groups.go | 7 +++---- services/graph/pkg/service/v0/password.go | 3 +-- services/graph/pkg/service/v0/tags.go | 5 ++--- services/graph/pkg/service/v0/users.go | 5 ++--- services/graph/pkg/service/v0/utils.go | 13 +++++++++++++ .../expected-failures-localAPI-on-OCIS-storage.md | 3 --- 11 files changed, 34 insertions(+), 32 deletions(-) create mode 100644 services/graph/pkg/service/v0/utils.go diff --git a/services/graph/pkg/service/v0/approleassignments.go b/services/graph/pkg/service/v0/approleassignments.go index 849193cd61..2340b77a86 100644 --- a/services/graph/pkg/service/v0/approleassignments.go +++ b/services/graph/pkg/service/v0/approleassignments.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "fmt" "net/http" @@ -46,7 +45,7 @@ func (g Graph) CreateAppRoleAssignment(w http.ResponseWriter, r *http.Request) { logger.Info().Interface("query", r.URL.Query()).Msg("calling create appRoleAssignment") appRoleAssignment := libregraph.NewAppRoleAssignmentWithDefaults() - err := json.NewDecoder(r.Body).Decode(appRoleAssignment) + err := StrictJSONUnmarshal(r.Body, appRoleAssignment) if err != nil { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %v", err.Error())) return diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 4e29f14038..4e80fb5cdc 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -246,7 +246,7 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) { client := g.GetGatewayClient() drive := libregraph.Drive{} - if err := json.NewDecoder(r.Body).Decode(&drive); err != nil { + if err := StrictJSONUnmarshal(r.Body, &drive); err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not create drive: invalid body schema definition") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid body schema definition") return @@ -342,7 +342,7 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) { } drive := libregraph.Drive{} - if err = json.NewDecoder(r.Body).Decode(&drive); err != nil { + if err = StrictJSONUnmarshal(r.Body, &drive); err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not update drive, invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: error: %v", err.Error())) return diff --git a/services/graph/pkg/service/v0/educationclasses.go b/services/graph/pkg/service/v0/educationclasses.go index 86aeb29b69..aa0fb98fa6 100644 --- a/services/graph/pkg/service/v0/educationclasses.go +++ b/services/graph/pkg/service/v0/educationclasses.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "errors" "fmt" "net/http" @@ -59,7 +58,7 @@ func (g Graph) PostEducationClass(w http.ResponseWriter, r *http.Request) { logger := g.logger.SubloggerWithRequestID(r.Context()) logger.Info().Msg("calling post EducationClass") class := libregraph.NewEducationClassWithDefaults() - err := json.NewDecoder(r.Body).Decode(class) + err := StrictJSONUnmarshal(r.Body, class) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not create education class: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) @@ -120,7 +119,7 @@ func (g Graph) PatchEducationClass(w http.ResponseWriter, r *http.Request) { return } changes := libregraph.NewEducationClassWithDefaults() - err = json.NewDecoder(r.Body).Decode(changes) + err = StrictJSONUnmarshal(r.Body, changes) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not change class: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) @@ -344,7 +343,7 @@ func (g Graph) PostEducationClassMember(w http.ResponseWriter, r *http.Request) return } memberRef := libregraph.NewMemberReference() - err = json.NewDecoder(r.Body).Decode(memberRef) + err = StrictJSONUnmarshal(r.Body, memberRef) if err != nil { logger.Debug(). Err(err). @@ -505,7 +504,7 @@ func (g Graph) PostEducationClassTeacher(w http.ResponseWriter, r *http.Request) return } memberRef := libregraph.NewMemberReference() - err = json.NewDecoder(r.Body).Decode(memberRef) + err = StrictJSONUnmarshal(r.Body, memberRef) if err != nil { logger.Debug(). Err(err). diff --git a/services/graph/pkg/service/v0/educationschools.go b/services/graph/pkg/service/v0/educationschools.go index 9bafb8ac1c..8215927572 100644 --- a/services/graph/pkg/service/v0/educationschools.go +++ b/services/graph/pkg/service/v0/educationschools.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "errors" "fmt" "net/http" @@ -57,7 +56,7 @@ func (g Graph) PostEducationSchool(w http.ResponseWriter, r *http.Request) { logger := g.logger.SubloggerWithRequestID(r.Context()) logger.Info().Msg("calling post school") school := libregraph.NewEducationSchool() - err := json.NewDecoder(r.Body).Decode(school) + err := StrictJSONUnmarshal(r.Body, school) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not create school: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) @@ -129,7 +128,7 @@ func (g Graph) PatchEducationSchool(w http.ResponseWriter, r *http.Request) { } school := libregraph.NewEducationSchool() - err = json.NewDecoder(r.Body).Decode(school) + err = StrictJSONUnmarshal(r.Body, school) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not update school: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) @@ -315,7 +314,7 @@ func (g Graph) PostEducationSchoolUser(w http.ResponseWriter, r *http.Request) { return } memberRef := libregraph.NewMemberReference() - err = json.NewDecoder(r.Body).Decode(memberRef) + err = StrictJSONUnmarshal(r.Body, memberRef) if err != nil { logger.Debug(). Err(err). @@ -485,7 +484,7 @@ func (g Graph) PostEducationSchoolClass(w http.ResponseWriter, r *http.Request) return } memberRef := libregraph.NewMemberReference() - err = json.NewDecoder(r.Body).Decode(memberRef) + err = StrictJSONUnmarshal(r.Body, memberRef) if err != nil { logger.Debug(). Err(err). diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index 0094362937..9ba3c4052e 100644 --- a/services/graph/pkg/service/v0/educationuser.go +++ b/services/graph/pkg/service/v0/educationuser.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "errors" "fmt" "net/http" @@ -60,7 +59,7 @@ func (g Graph) PostEducationUser(w http.ResponseWriter, r *http.Request) { logger := g.logger.SubloggerWithRequestID(r.Context()) logger.Info().Interface("body", r.Body).Msg("calling create education user") u := libregraph.NewEducationUser() - err := json.NewDecoder(r.Body).Decode(u) + err := StrictJSONUnmarshal(r.Body, u) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not create education user: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %v", err.Error())) @@ -334,7 +333,7 @@ func (g Graph) PatchEducationUser(w http.ResponseWriter, r *http.Request) { return } changes := libregraph.NewEducationUser() - err = json.NewDecoder(r.Body).Decode(changes) + err = StrictJSONUnmarshal(r.Body, changes) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not update education user: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go index 5fa9210486..0e2dbaa68c 100644 --- a/services/graph/pkg/service/v0/groups.go +++ b/services/graph/pkg/service/v0/groups.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "errors" "fmt" "net/http" @@ -60,7 +59,7 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) { logger := g.logger.SubloggerWithRequestID(r.Context()) logger.Info().Msg("calling post group") grp := libregraph.NewGroup() - err := json.NewDecoder(r.Body).Decode(grp) + err := StrictJSONUnmarshal(r.Body, grp) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not create group: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) @@ -119,7 +118,7 @@ func (g Graph) PatchGroup(w http.ResponseWriter, r *http.Request) { return } changes := libregraph.NewGroup() - err = json.NewDecoder(r.Body).Decode(changes) + err = StrictJSONUnmarshal(r.Body, changes) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not change group: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) @@ -325,7 +324,7 @@ func (g Graph) PostGroupMember(w http.ResponseWriter, r *http.Request) { return } memberRef := libregraph.NewMemberReference() - err = json.NewDecoder(r.Body).Decode(memberRef) + err = StrictJSONUnmarshal(r.Body, memberRef) if err != nil { logger.Debug(). Err(err). diff --git a/services/graph/pkg/service/v0/password.go b/services/graph/pkg/service/v0/password.go index 66c82ffccd..551c0e29ff 100644 --- a/services/graph/pkg/service/v0/password.go +++ b/services/graph/pkg/service/v0/password.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "net/http" "strings" @@ -34,7 +33,7 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { return } cpw := libregraph.NewPasswordChangeWithDefaults() - err = json.NewDecoder(r.Body).Decode(cpw) + err = StrictJSONUnmarshal(r.Body, cpw) if err != nil { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return diff --git a/services/graph/pkg/service/v0/tags.go b/services/graph/pkg/service/v0/tags.go index 57bedfa564..b3e95ef5e7 100644 --- a/services/graph/pkg/service/v0/tags.go +++ b/services/graph/pkg/service/v0/tags.go @@ -1,7 +1,6 @@ package svc import ( - "encoding/json" "net/http" "strings" @@ -54,7 +53,7 @@ func (g Graph) AssignTags(w http.ResponseWriter, r *http.Request) { ctx = r.Context() ) - if err := json.NewDecoder(r.Body).Decode(&assignment); err != nil { + if err := StrictJSONUnmarshal(r.Body, &assignment); err != nil { g.logger.Debug().Err(err).Interface("body", r.Body).Msg("could not decode tag assignment request") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid body schema definition") return @@ -143,7 +142,7 @@ func (g Graph) UnassignTags(w http.ResponseWriter, r *http.Request) { ctx = r.Context() ) - if err := json.NewDecoder(r.Body).Decode(&unassignment); err != nil { + if err := StrictJSONUnmarshal(r.Body, &unassignment); err != nil { g.logger.Debug().Err(err).Interface("body", r.Body).Msg("could not decode tag assignment request") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid body schema definition") return diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index d27e4b37e8..0a3e5cc143 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -2,7 +2,6 @@ package svc import ( "context" - "encoding/json" "errors" "fmt" "net/http" @@ -264,7 +263,7 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) { logger := g.logger.SubloggerWithRequestID(r.Context()) logger.Info().Interface("body", r.Body).Msg("calling create user") u := libregraph.NewUser() - err := json.NewDecoder(r.Body).Decode(u) + err := StrictJSONUnmarshal(r.Body, u) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not create user: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %v", err.Error())) @@ -639,7 +638,7 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { return } changes := libregraph.NewUser() - err = json.NewDecoder(r.Body).Decode(changes) + err = StrictJSONUnmarshal(r.Body, changes) if err != nil { logger.Debug().Err(err).Interface("body", r.Body).Msg("could not update user: invalid request body") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go new file mode 100644 index 0000000000..ce8c42da73 --- /dev/null +++ b/services/graph/pkg/service/v0/utils.go @@ -0,0 +1,13 @@ +package svc + +import ( + "encoding/json" + "io" +) + +// StrictJSONUnmarshal is a wrapper around json.Unmarshal that returns an error if the json contains unknown fields. +func StrictJSONUnmarshal(r io.Reader, v interface{}) error { + dec := json.NewDecoder(r) + dec.DisallowUnknownFields() + return dec.Decode(v) +} diff --git a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md index ea9e022e36..8938aacdf5 100644 --- a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md @@ -127,9 +127,6 @@ The expected failures in this file are from features in the owncloud/ocis repo. #### [Same users can be added in a group multiple time](https://github.com/owncloud/ocis/issues/5702) - [apiGraph/addUserToGroup.feature:246](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/addUserToGroup.feature#L246) -#### [Try to add group to a group return 204](https://github.com/owncloud/ocis/issues/5793) -- [apiGraph/addUserToGroup.feature:268](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/addUserToGroup.feature#L268) - #### [API requests from an unauthorized user should return 403](https://github.com/owncloud/ocis/issues/5938) - [apiGraph/addUserToGroup.feature:131](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/addUserToGroup.feature#L131) - [apiGraph/addUserToGroup.feature:132](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/addUserToGroup.feature#L132)