From 3cf7cd8e2b4b572a25f31494c82a1c84a40082bd Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Mon, 7 Dec 2020 19:26:23 +0100 Subject: [PATCH] Support conf. primary ID (corteza or external) --- app/options.go | 1 - app/servers.go | 28 ++++-- pkg/options/SCIM.gen.go | 11 ++- pkg/options/SCIM.yaml | 12 ++- system/scim/group_handler.go | 152 ++++++++++++++++++++----------- system/scim/routes.go | 16 +++- system/scim/user_handler.go | 169 +++++++++++++++++++++-------------- tests/system/scim_test.go | 57 +++++++++--- 8 files changed, 304 insertions(+), 142 deletions(-) diff --git a/app/options.go b/app/options.go index 3bd147a7e..6318611aa 100644 --- a/app/options.go +++ b/app/options.go @@ -28,7 +28,6 @@ type ( ) func NewOptions() *Options { - return &Options{ Environment: *options.Environment(), ActionLog: *options.ActionLog(), diff --git a/app/servers.go b/app/servers.go index ab843df8d..75e0a1d7a 100644 --- a/app/servers.go +++ b/app/servers.go @@ -14,6 +14,7 @@ import ( "github.com/go-chi/chi" "go.uber.org/zap" "net/http" + "regexp" "strings" "sync" ) @@ -82,7 +83,11 @@ func (app *CortezaApp) mountHttpRoutes(r chi.Router) { app.Log.Info("JSON REST API disabled") } - if app.Opt.SCIM.Enabled { + func() { + if !app.Opt.SCIM.Enabled { + return + } + if app.Opt.SCIM.Secret == "" { app.Log. WithOptions(zap.AddStacktrace(zap.PanicLevel)). @@ -90,9 +95,20 @@ func (app *CortezaApp) mountHttpRoutes(r chi.Router) { } var ( - baseUrl = "/" + strings.Trim(app.Opt.SCIM.BaseURL, "/") + baseUrl = "/" + strings.Trim(app.Opt.SCIM.BaseURL, "/") + extIdValidation *regexp.Regexp + err error ) + if len(app.Opt.SCIM.ExternalIdValidation) > 0 { + extIdValidation, err = regexp.Compile(app.Opt.SCIM.ExternalIdValidation) + } + + if err != nil { + app.Log.Error("failed to compile SCIM external ID validation", zap.Error(err)) + return + } + app.Log.Debug( "SCIM enabled", zap.String("baseUrl", baseUrl), @@ -100,14 +116,16 @@ func (app *CortezaApp) mountHttpRoutes(r chi.Router) { ) r.Route(baseUrl, func(r chi.Router) { - if !app.Opt.Environment.IsDevelopment() { r.Use(scim.Guard(app.Opt.SCIM)) } - scim.Routes(r) + scim.Routes(r, scim.Config{ + ExternalIdAsPrimary: app.Opt.SCIM.ExternalIdAsPrimary, + ExternalIdValidator: extIdValidation, + }) }) - } + }() if app.Opt.HTTPServer.WebappEnabled { r.Route("/"+webappBaseUrl, webapp.MakeWebappServer(app.Opt.HTTPServer)) diff --git a/pkg/options/SCIM.gen.go b/pkg/options/SCIM.gen.go index 800040438..6f1c02ccd 100644 --- a/pkg/options/SCIM.gen.go +++ b/pkg/options/SCIM.gen.go @@ -10,16 +10,19 @@ package options type ( SCIMOpt struct { - Enabled bool `env:"SCIM_ENABLED"` - BaseURL string `env:"SCIM_BASE_URL"` - Secret string `env:"SCIM_SECRET"` + Enabled bool `env:"SCIM_ENABLED"` + BaseURL string `env:"SCIM_BASE_URL"` + Secret string `env:"SCIM_SECRET"` + ExternalIdAsPrimary bool `env:"SCIM_EXTERNAL_ID_AS_PRIMARY"` + ExternalIdValidation string `env:"SCIM_EXTERNAL_ID_VALIDATION"` } ) // SCIM initializes and returns a SCIMOpt with default values func SCIM() (o *SCIMOpt) { o = &SCIMOpt{ - BaseURL: "/scim", + BaseURL: "/scim", + ExternalIdValidation: "$[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}^", } fill(o) diff --git a/pkg/options/SCIM.yaml b/pkg/options/SCIM.yaml index d2b22dbbb..086c0f62c 100644 --- a/pkg/options/SCIM.yaml +++ b/pkg/options/SCIM.yaml @@ -1,8 +1,18 @@ -name: SCIM +docs: + title: SCIM Server props: - name: enabled type: bool + description: Enable SCIM subsystem - name: baseURL default: "/scim" + description: Prefix for SCIM API endpoints - name: secret + description: Secret to use to validate requests on SCIM API endpoints + - name: externalIdAsPrimary + type: bool + description: Use external IDs in SCIM API endpoints + - name: externalIdValidation + default: "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$" + description: Validates format of external IDs. Defaults to UUID diff --git a/system/scim/group_handler.go b/system/scim/group_handler.go index 0f2ddf792..ce1d84e5a 100644 --- a/system/scim/group_handler.go +++ b/system/scim/group_handler.go @@ -9,11 +9,15 @@ import ( "github.com/go-chi/chi" "io" "net/http" + "regexp" "strconv" ) type ( groupsHandler struct { + externalIdAsPrimary bool + externalIdValidator *regexp.Regexp + svc service.RoleService sec getSecurityContextFn } @@ -21,21 +25,14 @@ type ( func (h groupsHandler) get(w http.ResponseWriter, r *http.Request) { var ( - id, _ = strconv.ParseUint(chi.URLParam(r, "id"), 10, 64) - ctx = h.sec(r) - svc = h.svc.With(ctx) + res = h.lookup(h.sec(r), chi.URLParam(r, "id"), w) ) - if id == 0 { - http.Error(w, "invalid group id", http.StatusBadRequest) + if res == nil { return } - if u, err := svc.FindByID(id); err != nil { - sendError(w, newErrorResonse(http.StatusBadRequest, err)) - } else { - send(w, http.StatusOK, newGroupResourceResponse(u)) - } + send(w, http.StatusOK, newGroupResourceResponse(res)) } func (h groupsHandler) create(w http.ResponseWriter, r *http.Request) { @@ -45,104 +42,157 @@ func (h groupsHandler) create(w http.ResponseWriter, r *http.Request) { ctx = h.sec(r) ) - if u, err := h.createFromJSON(ctx, r.Body); err != nil { - sendError(w, newErrorResonse(http.StatusBadRequest, err)) + if u, code, err := h.createFromJSON(ctx, r.Body); err != nil { + sendError(w, newErrorResonse(code, err)) } else { send(w, http.StatusCreated, newGroupResourceResponse(u)) } } -func (h groupsHandler) createFromJSON(ctx context.Context, j io.Reader) (r *types.Role, err error) { +func (h groupsHandler) createFromJSON(ctx context.Context, j io.Reader) (res *types.Role, code int, err error) { var ( svc = h.svc.With(ctx) payload = &groupResourceRequest{} ) + code = http.StatusBadRequest if err = payload.decodeJSON(j); err != nil { - return } // do we need to upsert? if payload.ExternalId != nil { - var rr types.RoleSet - rr, _, err = svc.Find(types.RoleFilter{Labels: map[string]string{groupLabel_SCIM_externalId: *payload.ExternalId}}) - if err != nil { + res, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) + if err != nil && code != http.StatusNotFound { return } - - if len(rr) > 0 { - r = rr[0] - } } else if payload.Name != nil { - r, err = svc.FindByName(*payload.Name) + res, err = svc.FindByName(*payload.Name) if err != nil && !errors.Is(err, service.RoleErrNotFound()) { - return + return nil, http.StatusInternalServerError, err } } - if r == nil || r.ID == 0 { + if res == nil || res.ID == 0 { // in case when we did not find a valid group, // start from blank - r = &types.Role{} + res = &types.Role{} } - payload.applyTo(r) + payload.applyTo(res) - if r.ID > 0 { - return svc.Update(r) + if res.ID > 0 { + res, err = svc.Update(res) } else { - return svc.Create(r) + res, err = svc.Create(res) } + + if err != nil { + return nil, http.StatusInternalServerError, err + } + + return res, 0, nil } func (h groupsHandler) replace(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() var ( - ctx = h.sec(r) - groupID, _ = strconv.ParseUint(chi.URLParam(r, "id"), 10, 64) + ctx = h.sec(r) + existing = h.lookup(ctx, chi.URLParam(r, "id"), w) ) - if u, err := h.updateFromJSON(ctx, groupID, r.Body); err != nil { + 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(u)) + send(w, http.StatusOK, newGroupResourceResponse(res)) } } -func (h groupsHandler) updateFromJSON(ctx context.Context, id uint64, j io.Reader) (r *types.Role, err error) { +func (h groupsHandler) updateFromJSON(ctx context.Context, res *types.Role, j io.Reader) (*types.Role, error) { var ( - svc = h.svc.With(ctx) payload = &groupResourceRequest{} ) - if r, err = svc.FindByID(id); err != nil { - return + if err := payload.decodeJSON(j); err != nil { + return nil, err } - if r == nil { - return nil, fmt.Errorf("refusing to update invalid group") - } + payload.applyTo(res) - if err = payload.decodeJSON(j); err != nil { - return - } - - payload.applyTo(r) - - return h.svc.With(ctx).Update(r) + return h.svc.With(ctx).Update(res) } func (h groupsHandler) delete(w http.ResponseWriter, r *http.Request) { var ( - ctx = h.sec(r) - groupID, _ = strconv.ParseUint(chi.URLParam(r, "id"), 10, 64) - svc = h.svc.With(ctx) + ctx = h.sec(r) + svc = h.svc.With(ctx) + res = h.lookup(ctx, chi.URLParam(r, "id"), w) ) - if err := svc.Delete(groupID); err != nil { + if res == nil { + return + } + + if err := svc.Delete(res.ID); err != nil { sendError(w, newErrorResonse(http.StatusBadRequest, err)) } else { w.WriteHeader(http.StatusNoContent) } } + +// loads role from request path params +// +// handles errors by writing them to response +func (h groupsHandler) lookup(ctx context.Context, id string, w http.ResponseWriter) *types.Role { + var ( + svc = h.svc.With(ctx) + ) + + if h.externalIdAsPrimary { + role, code, err := h.lookupByExternalId(ctx, id) + if err != nil { + sendError(w, newErrorResonse(code, err)) + return nil + } + + return role + } else { + resId, err := strconv.ParseUint(id, 10, 64) + if err != nil || resId == 0 { + sendError(w, newErrorResonse(http.StatusBadRequest, err)) + return nil + } + + role, err := svc.FindByID(resId) + if err != nil { + sendError(w, newErrorResonse(http.StatusBadRequest, err)) + return nil + } + + return role + } +} + +func (h groupsHandler) lookupByExternalId(ctx context.Context, id string) (r *types.Role, code int, err error) { + if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) { + return nil, http.StatusBadRequest, fmt.Errorf("invalid external ID") + } + + rr, _, err := h.svc.With(ctx).Find(types.RoleFilter{Labels: map[string]string{groupLabel_SCIM_externalId: id}}) + if err != nil { + return nil, http.StatusInternalServerError, err + } + + switch len(rr) { + case 0: + return nil, http.StatusNotFound, fmt.Errorf("role not found") + case 1: + return rr[0], 0, nil + default: + return nil, http.StatusPreconditionFailed, fmt.Errorf("more than one role matches this externalId") + } +} diff --git a/system/scim/routes.go b/system/scim/routes.go index 074054b2b..73a21c746 100644 --- a/system/scim/routes.go +++ b/system/scim/routes.go @@ -8,6 +8,14 @@ import ( "github.com/goware/statik/fs" "go.uber.org/zap" "net/http" + "regexp" +) + +type ( + Config struct { + ExternalIdAsPrimary bool + ExternalIdValidator *regexp.Regexp + } ) var ( @@ -42,9 +50,12 @@ func Guard(opt options.SCIMOpt) func(next http.Handler) http.Handler { } } -func Routes(r chi.Router) { +func Routes(r chi.Router, cfg Config) { r.Route("/Users", func(r chi.Router) { uh := &usersHandler{ + externalIdAsPrimary: cfg.ExternalIdAsPrimary, + externalIdValidator: cfg.ExternalIdValidator, + svc: service.DefaultUser, passSvc: service.DefaultAuth, sec: getSecurityContext, @@ -58,6 +69,9 @@ func Routes(r chi.Router) { r.Route("/Groups", func(r chi.Router) { gh := &groupsHandler{ + externalIdAsPrimary: cfg.ExternalIdAsPrimary, + externalIdValidator: cfg.ExternalIdValidator, + svc: service.DefaultRole, sec: getSecurityContext, } diff --git a/system/scim/user_handler.go b/system/scim/user_handler.go index bbb00fab1..e639d1cd3 100644 --- a/system/scim/user_handler.go +++ b/system/scim/user_handler.go @@ -6,9 +6,11 @@ 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 +20,10 @@ type ( } usersHandler struct { + externalIdAsPrimary bool + externalIdValidator *regexp.Regexp + svc service.UserService - rleSvc service.RoleService passSvc passwordSetter sec getSecurityContextFn } @@ -27,24 +31,14 @@ type ( func (h usersHandler) get(w http.ResponseWriter, r *http.Request) { var ( - id, _ = strconv.ParseUint(chi.URLParam(r, "id"), 10, 64) - ctx = h.sec(r) - svc = h.svc.With(ctx) + res = h.lookup(h.sec(r), chi.URLParam(r, "id"), w) ) - if id == 0 { - http.Error(w, "invalid user id", http.StatusBadRequest) + if res == nil { return } - if u, err := svc.FindByID(id); err != nil { - sendError(w, newErrorResonse(http.StatusBadRequest, err)) - return - } else { - send(w, http.StatusOK, newUserResourceResponse(u)) - } - - w.WriteHeader(http.StatusOK) + send(w, http.StatusOK, newUserResourceResponse(res)) } func (h usersHandler) create(w http.ResponseWriter, r *http.Request) { @@ -54,130 +48,167 @@ func (h usersHandler) create(w http.ResponseWriter, r *http.Request) { ctx = h.sec(r) ) - if u, err := h.createFromJSON(ctx, r.Body); err != nil { - sendError(w, newErrorResonse(http.StatusBadRequest, err)) + if u, code, err := h.createFromJSON(ctx, 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) (u *types.User, err error) { +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 { - var uu types.UserSet - uu, _, err = svc.Find(types.UserFilter{Labels: map[string]string{userLabel_SCIM_externalId: *payload.ExternalId}}) - if err != nil { + res, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) + if err != nil && code != http.StatusNotFound { return } - - if len(uu) > 0 { - u = uu[0] - } } else if email := payload.Emails.getFirst(); email != "" { - u, err = svc.FindByEmail(email) + res, err = svc.FindByEmail(email) if err != nil && !errors.Is(err, service.UserErrNotFound()) { - return + return nil, http.StatusInternalServerError, err } } - if u == nil || !u.Valid() { + if res == nil || !res.Valid() { // in case when we did not find a valid user, // start from blank - u = &types.User{} + res = &types.User{} } - payload.applyTo(u) + payload.applyTo(res) - if u.ID > 0 { - u, err = svc.Update(u) + if res.ID > 0 { + res, err = svc.Update(res) } else { - u, err = svc.Create(u) + res, err = svc.Create(res) } if err != nil { - return - } - - if payload.Groups != nil { - // remove existing, add new - // @todo + return nil, http.StatusInternalServerError, err } if payload.Password != nil && *payload.Password != "" { - err = h.passSvc.SetPassword(ctx, u.ID, *payload.Password) + err = h.passSvc.SetPassword(ctx, res.ID, *payload.Password) if err != nil { return } } - return u, nil + return res, 0, nil } func (h usersHandler) replace(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() var ( - ctx = h.sec(r) - userID, _ = strconv.ParseUint(chi.URLParam(r, "id"), 10, 64) + ctx = h.sec(r) + existing = h.lookup(ctx, chi.URLParam(r, "id"), w) ) - if u, err := h.updateFromJSON(ctx, userID, r.Body); err != nil { + 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(u)) + send(w, http.StatusOK, newUserResourceResponse(res)) } } -func (h usersHandler) updateFromJSON(ctx context.Context, id uint64, j io.Reader) (u *types.User, err error) { +func (h usersHandler) updateFromJSON(ctx context.Context, res *types.User, j io.Reader) (*types.User, error) { var ( - svc = h.svc.With(ctx) payload = &userResourceRequest{} ) - if u, err = svc.FindByID(id); err != nil { - return + if err := payload.decodeJSON(j); err != nil { + return nil, err } - if u == nil || !u.Valid() { - return nil, fmt.Errorf("refusing to update invalid user") - } + payload.applyTo(res) - if err = payload.decodeJSON(j); err != nil { - return - } - - payload.applyTo(u) - - if payload.Password != nil && *payload.Password != "" { - err = h.passSvc.SetPassword(ctx, u.ID, *payload.Password) - if err != nil { - return - } - } - - return h.svc.With(ctx).Update(u) + return h.svc.With(ctx).Update(res) } func (h usersHandler) delete(w http.ResponseWriter, r *http.Request) { var ( - ctx = h.sec(r) - userID, _ = strconv.ParseUint(chi.URLParam(r, "id"), 10, 64) - svc = h.svc.With(ctx) + ctx = h.sec(r) + svc = h.svc.With(ctx) + res = h.lookup(ctx, chi.URLParam(r, "id"), w) ) - if err := svc.Delete(userID); err != nil { + if res == nil { + return + } + + if err := svc.Delete(res.ID); err != nil { sendError(w, newErrorResonse(http.StatusBadRequest, err)) } else { w.WriteHeader(http.StatusNoContent) } } + +// loads role from request path params +// +// handles errors by writing them to response +func (h usersHandler) lookup(ctx context.Context, id string, w http.ResponseWriter) *types.User { + var ( + svc = h.svc.With(ctx) + ) + spew.Dump(h.externalIdAsPrimary) + if h.externalIdAsPrimary { + role, code, err := h.lookupByExternalId(ctx, id) + if err != nil { + sendError(w, newErrorResonse(code, err)) + return nil + } + + return role + } else { + groupId, err := strconv.ParseUint(id, 10, 64) + if err != nil || groupId == 0 { + sendError(w, newErrorResonse(http.StatusBadRequest, err)) + return nil + } + + role, err := svc.FindByID(groupId) + if err != nil { + sendError(w, newErrorResonse(http.StatusBadRequest, err)) + return nil + } + + return role + } +} + +func (h usersHandler) lookupByExternalId(ctx context.Context, id string) (r *types.User, code int, err error) { + spew.Dump(id) + if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) { + return nil, http.StatusBadRequest, fmt.Errorf("invalid external ID") + } + + rr, _, err := h.svc.With(ctx).Find(types.UserFilter{Labels: map[string]string{groupLabel_SCIM_externalId: id}}) + if err != nil { + return nil, http.StatusInternalServerError, err + } + + switch len(rr) { + case 0: + return nil, http.StatusNotFound, fmt.Errorf("user not found") + case 1: + return rr[0], 0, nil + default: + return nil, http.StatusPreconditionFailed, fmt.Errorf("more than one user matches this externalId") + } +} diff --git a/tests/system/scim_test.go b/tests/system/scim_test.go index a5154a893..81381f499 100644 --- a/tests/system/scim_test.go +++ b/tests/system/scim_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "github.com/cortezaproject/corteza-server/pkg/api/server" + "github.com/cortezaproject/corteza-server/pkg/label/types" "github.com/cortezaproject/corteza-server/pkg/logger" "github.com/cortezaproject/corteza-server/store" "github.com/cortezaproject/corteza-server/system/scim" @@ -12,23 +13,25 @@ import ( "github.com/steinfletcher/apitest" jsonpath "github.com/steinfletcher/apitest-jsonpath" "net/http" + "regexp" "testing" ) -var ( - scimRoutes chi.Router -) - // apitest basics, initialize, set handler, add auth -func (h helper) scimApiInit() *apitest.APITest { +func (h helper) scimApiInit(ffn ...func(*scim.Config)) *apitest.APITest { InitTestApp() - - if scimRoutes == nil { + var ( + scimConfig scim.Config scimRoutes = chi.NewRouter() - scimRoutes.Use(server.BaseMiddleware(false, logger.Default())...) - scim.Routes(scimRoutes) + ) + + for _, fn := range ffn { + fn(&scimConfig) } + scimRoutes.Use(server.BaseMiddleware(false, logger.Default())...) + scim.Routes(scimRoutes, scimConfig) + return apitest. New(). Handler(scimRoutes) @@ -89,7 +92,7 @@ func TestScimUserCreateNoEmail(t *testing.T) { Post("/Users"). JSON(`{"schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`). Expect(t). - Status(http.StatusBadRequest). + Status(http.StatusInternalServerError). End() } @@ -288,3 +291,37 @@ func TestScimGroupDelete(t *testing.T) { Status(http.StatusNoContent). End() } + +func TestScimUserReplaceOnExternalId(t *testing.T) { + h := newHelper(t) + h.clearUsers() + + // creating a new user and assigning an external ID label to it + u := h.createUserWithEmail(h.randEmail()) + const externalId = `2819c223-7f76-453a-919d-413861904646` + h.a.NoError(store.UpsertLabel(h.secCtx(), service.DefaultStore, &types.Label{ + Kind: u.LabelResourceKind(), + ResourceID: u.LabelResourceID(), + Name: "SCIM_externalId", + Value: externalId, + })) + + h.scimApiInit(scimSetWithExternalId, scimSetWithUUIDValidator). + Put(fmt.Sprintf("/Users/%s", externalId)). + JSON(`{"emails":[{"value":"baz@bar.com"}],"externalId":"` + externalId + `","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`). + Expect(t). + Status(http.StatusOK). + End() + + u, err := store.LookupUserByID(context.Background(), service.DefaultStore, u.ID) + h.a.NoError(err) + h.a.NotNil(u) + h.a.Equal("baz@bar.com", u.Email) +} + +func scimSetWithExternalId(c *scim.Config) { + c.ExternalIdAsPrimary = true +} +func scimSetWithUUIDValidator(c *scim.Config) { + c.ExternalIdValidator = regexp.MustCompile(`^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$`) +}