diff --git a/system/scim/group_handler.go b/system/scim/group_handler.go index ce1d84e5a..8974d19f9 100644 --- a/system/scim/group_handler.go +++ b/system/scim/group_handler.go @@ -6,8 +6,8 @@ import ( "github.com/cortezaproject/corteza-server/pkg/errors" "github.com/cortezaproject/corteza-server/system/service" "github.com/cortezaproject/corteza-server/system/types" + "github.com/davecgh/go-spew/spew" "github.com/go-chi/chi" - "io" "net/http" "regexp" "strconv" @@ -18,8 +18,9 @@ type ( externalIdAsPrimary bool externalIdValidator *regexp.Regexp - svc service.RoleService - sec getSecurityContextFn + svc service.RoleService + passSvc passwordSetter + sec getSecurityContextFn } ) @@ -39,46 +40,91 @@ func (h groupsHandler) create(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() var ( - ctx = h.sec(r) + ctx = h.sec(r) + svc = h.svc.With(ctx) + payload = &groupResourceRequest{} + err error + existing *types.Role + code = http.StatusBadRequest ) - if u, code, err := h.createFromJSON(ctx, r.Body); err != nil { + if err = payload.decodeJSON(r.Body); err != nil { sendError(w, newErrorResonse(code, err)) - } else { - send(w, http.StatusCreated, newGroupResourceResponse(u)) + return } + + { + // do we need to upsert? + if payload.ExternalId != nil { + existing, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) + if err != nil && code != http.StatusNotFound { + sendError(w, newErrorResonse(code, err)) + return + } + } else if *payload.Name != "" { + existing, err = svc.FindByName(*payload.Name) + if err != nil && !errors.Is(err, service.RoleErrNotFound()) { + sendError(w, newErrorResonse(http.StatusInternalServerError, err)) + return + } + } + } + + res, code, err := h.save(ctx, payload, existing) + if err != nil { + sendError(w, newErrorResonse(code, err)) + return + } + + code = http.StatusOK + if res.UpdatedAt == nil { + code = http.StatusCreated + } + + send(w, code, newGroupResourceResponse(res)) } -func (h groupsHandler) createFromJSON(ctx context.Context, j io.Reader) (res *types.Role, code int, err error) { +func (h groupsHandler) replace(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + var ( - svc = h.svc.With(ctx) - payload = &groupResourceRequest{} + ctx = h.sec(r) + existing = h.lookup(ctx, chi.URLParam(r, "id"), w) + payload = &groupResourceRequest{} ) - code = http.StatusBadRequest - if err = payload.decodeJSON(j); err != nil { + if err := payload.decodeJSON(r.Body); err != nil { + sendError(w, newErrorResonse(http.StatusBadRequest, err)) + return } - // do we need to upsert? - if payload.ExternalId != nil { - res, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) - if err != nil && code != http.StatusNotFound { - return - } - } else if payload.Name != nil { - res, err = svc.FindByName(*payload.Name) - if err != nil && !errors.Is(err, service.RoleErrNotFound()) { - return nil, http.StatusInternalServerError, err - } + res, code, err := h.save(ctx, payload, existing) + if err != nil { + sendError(w, newErrorResonse(code, err)) + return } - if res == nil || res.ID == 0 { + code = http.StatusOK + if res.UpdatedAt == nil { + code = http.StatusCreated + } + + send(w, code, newGroupResourceResponse(res)) +} + +func (h groupsHandler) save(ctx context.Context, req *groupResourceRequest, existing *types.Role) (res *types.Role, code int, err error) { + var ( + svc = h.svc.With(ctx) + ) + + if existing == nil { // in case when we did not find a valid group, // start from blank - res = &types.Role{} + existing = &types.Role{} } - payload.applyTo(res) + res = existing + req.applyTo(res) if res.ID > 0 { res, err = svc.Update(res) @@ -93,39 +139,6 @@ func (h groupsHandler) createFromJSON(ctx context.Context, j io.Reader) (res *ty return res, 0, nil } -func (h groupsHandler) replace(w http.ResponseWriter, r *http.Request) { - defer r.Body.Close() - - var ( - ctx = h.sec(r) - existing = h.lookup(ctx, chi.URLParam(r, "id"), w) - ) - - if existing == nil { - return - } - - if res, err := h.updateFromJSON(ctx, existing, r.Body); err != nil { - sendError(w, newErrorResonse(http.StatusBadRequest, err)) - } else { - send(w, http.StatusOK, newGroupResourceResponse(res)) - } -} - -func (h groupsHandler) updateFromJSON(ctx context.Context, res *types.Role, j io.Reader) (*types.Role, error) { - var ( - payload = &groupResourceRequest{} - ) - - if err := payload.decodeJSON(j); err != nil { - return nil, err - } - - payload.applyTo(res) - - return h.svc.With(ctx).Update(res) -} - func (h groupsHandler) delete(w http.ResponseWriter, r *http.Request) { var ( ctx = h.sec(r) @@ -161,13 +174,13 @@ func (h groupsHandler) lookup(ctx context.Context, id string, w http.ResponseWri return role } else { - resId, err := strconv.ParseUint(id, 10, 64) - if err != nil || resId == 0 { + groupId, err := strconv.ParseUint(id, 10, 64) + if err != nil || groupId == 0 { sendError(w, newErrorResonse(http.StatusBadRequest, err)) return nil } - role, err := svc.FindByID(resId) + role, err := svc.FindByID(groupId) if err != nil { sendError(w, newErrorResonse(http.StatusBadRequest, err)) return nil @@ -178,6 +191,7 @@ func (h groupsHandler) lookup(ctx context.Context, id string, w http.ResponseWri } func (h groupsHandler) lookupByExternalId(ctx context.Context, id string) (r *types.Role, code int, err error) { + spew.Dump(id) if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) { return nil, http.StatusBadRequest, fmt.Errorf("invalid external ID") } @@ -189,10 +203,10 @@ func (h groupsHandler) lookupByExternalId(ctx context.Context, id string) (r *ty switch len(rr) { case 0: - return nil, http.StatusNotFound, fmt.Errorf("role not found") + return nil, http.StatusNotFound, fmt.Errorf("group not found") case 1: return rr[0], 0, nil default: - return nil, http.StatusPreconditionFailed, fmt.Errorf("more than one role matches this externalId") + return nil, http.StatusPreconditionFailed, fmt.Errorf("more than one group matches this externalId") } } diff --git a/system/scim/user_handler.go b/system/scim/user_handler.go index e639d1cd3..8f581e8f1 100644 --- a/system/scim/user_handler.go +++ b/system/scim/user_handler.go @@ -8,7 +8,6 @@ import ( "github.com/cortezaproject/corteza-server/system/types" "github.com/davecgh/go-spew/spew" "github.com/go-chi/chi" - "io" "net/http" "regexp" "strconv" @@ -45,48 +44,91 @@ func (h usersHandler) create(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() var ( - ctx = h.sec(r) + ctx = h.sec(r) + svc = h.svc.With(ctx) + payload = &userResourceRequest{} + err error + existing *types.User + code = http.StatusBadRequest ) - if u, code, err := h.createFromJSON(ctx, r.Body); err != nil { + if err = payload.decodeJSON(r.Body); err != nil { sendError(w, newErrorResonse(code, err)) - } else { - send(w, http.StatusCreated, newUserResourceResponse(u)) - } -} - -func (h usersHandler) createFromJSON(ctx context.Context, j io.Reader) (res *types.User, code int, err error) { - var ( - svc = h.svc.With(ctx) - //roles = h.rleSvc.With(ctx) - payload = &userResourceRequest{} - ) - - code = http.StatusBadRequest - if err = payload.decodeJSON(j); err != nil { return } - // do we need to upsert? - if payload.ExternalId != nil { - res, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) - if err != nil && code != http.StatusNotFound { - return - } - } else if email := payload.Emails.getFirst(); email != "" { - res, err = svc.FindByEmail(email) - if err != nil && !errors.Is(err, service.UserErrNotFound()) { - return nil, http.StatusInternalServerError, err + { + // do we need to upsert? + if payload.ExternalId != nil { + existing, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) + if err != nil && code != http.StatusNotFound { + sendError(w, newErrorResonse(code, err)) + return + } + } else if email := payload.Emails.getFirst(); email != "" { + existing, err = svc.FindByEmail(email) + if err != nil && !errors.Is(err, service.UserErrNotFound()) { + sendError(w, newErrorResonse(http.StatusInternalServerError, err)) + return + } } } - if res == nil || !res.Valid() { + res, code, err := h.save(ctx, payload, existing) + if err != nil { + sendError(w, newErrorResonse(code, err)) + return + } + + code = http.StatusOK + if res.UpdatedAt == nil { + code = http.StatusCreated + } + + send(w, code, newUserResourceResponse(res)) +} + +func (h usersHandler) replace(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + + var ( + ctx = h.sec(r) + existing = h.lookup(ctx, chi.URLParam(r, "id"), w) + payload = &userResourceRequest{} + ) + + if err := payload.decodeJSON(r.Body); err != nil { + sendError(w, newErrorResonse(http.StatusBadRequest, err)) + return + } + + res, code, err := h.save(ctx, payload, existing) + if err != nil { + sendError(w, newErrorResonse(code, err)) + return + } + + code = http.StatusOK + if res.UpdatedAt == nil { + code = http.StatusCreated + } + + send(w, code, newUserResourceResponse(res)) +} + +func (h usersHandler) save(ctx context.Context, req *userResourceRequest, existing *types.User) (res *types.User, code int, err error) { + var ( + svc = h.svc.With(ctx) + ) + + if existing == nil || !existing.Valid() { // in case when we did not find a valid user, // start from blank - res = &types.User{} + existing = &types.User{} } - payload.applyTo(res) + res = existing + req.applyTo(res) if res.ID > 0 { res, err = svc.Update(res) @@ -98,8 +140,8 @@ func (h usersHandler) createFromJSON(ctx context.Context, j io.Reader) (res *typ return nil, http.StatusInternalServerError, err } - if payload.Password != nil && *payload.Password != "" { - err = h.passSvc.SetPassword(ctx, res.ID, *payload.Password) + if req.Password != nil && *req.Password != "" { + err = h.passSvc.SetPassword(ctx, res.ID, *req.Password) if err != nil { return } @@ -108,39 +150,6 @@ func (h usersHandler) createFromJSON(ctx context.Context, j io.Reader) (res *typ return res, 0, nil } -func (h usersHandler) replace(w http.ResponseWriter, r *http.Request) { - defer r.Body.Close() - - var ( - ctx = h.sec(r) - existing = h.lookup(ctx, chi.URLParam(r, "id"), w) - ) - - if existing == nil { - return - } - - if res, err := h.updateFromJSON(ctx, existing, r.Body); err != nil { - sendError(w, newErrorResonse(http.StatusBadRequest, err)) - } else { - send(w, http.StatusOK, newUserResourceResponse(res)) - } -} - -func (h usersHandler) updateFromJSON(ctx context.Context, res *types.User, j io.Reader) (*types.User, error) { - var ( - payload = &userResourceRequest{} - ) - - if err := payload.decodeJSON(j); err != nil { - return nil, err - } - - payload.applyTo(res) - - return h.svc.With(ctx).Update(res) -} - func (h usersHandler) delete(w http.ResponseWriter, r *http.Request) { var ( ctx = h.sec(r) @@ -166,7 +175,7 @@ func (h usersHandler) lookup(ctx context.Context, id string, w http.ResponseWrit var ( svc = h.svc.With(ctx) ) - spew.Dump(h.externalIdAsPrimary) + if h.externalIdAsPrimary { role, code, err := h.lookupByExternalId(ctx, id) if err != nil { diff --git a/tests/system/scim_test.go b/tests/system/scim_test.go index 81381f499..87db631dc 100644 --- a/tests/system/scim_test.go +++ b/tests/system/scim_test.go @@ -106,7 +106,7 @@ func TestScimUserCreateOverwrite(t *testing.T) { Post("/Users"). JSON(`{"userName":"UPDATED","emails":[{"value":"foo@bar.com"}],"schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`). Expect(t). - Status(http.StatusCreated). + Status(http.StatusOK). End() u, err := store.LookupUserByEmail(context.Background(), service.DefaultStore, "foo@bar.com") @@ -133,7 +133,7 @@ func TestScimUserExternalID(t *testing.T) { Post("/Users"). JSON(`{"userName":"baz","emails":[{"value":"baz@bar.com"}],"externalId":"foo42","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`). Expect(t). - Status(http.StatusCreated). + Status(http.StatusOK). End() u, err = store.LookupUserByEmail(context.Background(), service.DefaultStore, "baz@bar.com") @@ -253,7 +253,7 @@ func TestScimGroupExternalId(t *testing.T) { Post("/Groups"). JSON(`{"schemas":["urn:ietf:params:scim:schemas:core:2.0:Group"],"displayName":"bar","externalId":"grp42"}`). Expect(t). - Status(http.StatusCreated). + Status(http.StatusOK). End() u, err = store.LookupRoleByName(context.Background(), service.DefaultStore, "bar")