From d359a7c2cf4bd41e30baccb9f3ac27ef75424939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 12 Dec 2022 12:46:10 +0100 Subject: [PATCH] [full-ci] standalone graph service with LDAP (#5199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * standalone graph service with LDAP Signed-off-by: Jörn Friedrich Dreyer * no panic on PATCH and DELETE Signed-off-by: Jörn Friedrich Dreyer * fix apitoken yaml key Signed-off-by: Jörn Friedrich Dreyer * update user, fix response codes Signed-off-by: Jörn Friedrich Dreyer * fix group creation return code Signed-off-by: Jörn Friedrich Dreyer * remove unknown user property Signed-off-by: Jörn Friedrich Dreyer * fix create return code checks in graph feature context Signed-off-by: Jörn Friedrich Dreyer * updating uses 200 OK when returning a body Signed-off-by: Jörn Friedrich Dreyer * revert user statusCreated change for now Signed-off-by: Jörn Friedrich Dreyer * revert return code changes Signed-off-by: Jörn Friedrich Dreyer Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/config/http.go | 1 + services/graph/pkg/identity/ldap.go | 1 + services/graph/pkg/server/http/server.go | 58 +++++-- services/graph/pkg/service/v0/groups.go | 4 +- services/graph/pkg/service/v0/groups_test.go | 2 +- services/graph/pkg/service/v0/option.go | 26 ++- services/graph/pkg/service/v0/service.go | 28 +-- services/graph/pkg/service/v0/users.go | 159 +++++++++--------- .../features/bootstrap/GraphContext.php | 2 +- 9 files changed, 157 insertions(+), 124 deletions(-) diff --git a/services/graph/pkg/config/http.go b/services/graph/pkg/config/http.go index ec521094c8..00d4144d9a 100644 --- a/services/graph/pkg/config/http.go +++ b/services/graph/pkg/config/http.go @@ -8,4 +8,5 @@ type HTTP struct { Namespace string `yaml:"-"` Root string `yaml:"root" env:"GRAPH_HTTP_ROOT" desc:"Subdirectory that serves as the root for this HTTP service."` TLS shared.HTTPServiceTLS `yaml:"tls"` + APIToken string `yaml:"apitoken" env:"GRAPH_HTTP_API_TOKEN" desc:"An optional API bearer token"` } diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index f9a3d1afab..ea879e6be6 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -280,6 +280,7 @@ func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph. updateNeeded = true } } + // TODO implement account disabled/enabled if updateNeeded { if err := i.conn.Modify(&mr); err != nil { diff --git a/services/graph/pkg/server/http/server.go b/services/graph/pkg/server/http/server.go index fdb0c2fa08..5e7bb62f5e 100644 --- a/services/graph/pkg/server/http/server.go +++ b/services/graph/pkg/server/http/server.go @@ -4,16 +4,20 @@ import ( "crypto/tls" "crypto/x509" "fmt" + stdhttp "net/http" "os" "github.com/cs3org/reva/v2/pkg/events/server" + "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" chimiddleware "github.com/go-chi/chi/v5/middleware" "github.com/go-micro/plugins/v4/events/natsjs" "github.com/owncloud/ocis/v2/ocis-pkg/account" ociscrypto "github.com/owncloud/ocis/v2/ocis-pkg/crypto" "github.com/owncloud/ocis/v2/ocis-pkg/middleware" + "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" "github.com/owncloud/ocis/v2/ocis-pkg/service/http" "github.com/owncloud/ocis/v2/ocis-pkg/version" + settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" graphMiddleware "github.com/owncloud/ocis/v2/services/graph/pkg/middleware" svc "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0" "github.com/pkg/errors" @@ -82,25 +86,49 @@ func Server(opts ...Option) (http.Service, error) { } } - handle := svc.NewService( - svc.Logger(options.Logger), - svc.Config(options.Config), - svc.Middleware( - middleware.TraceContext, - chimiddleware.RequestID, - middleware.Version( - "graph", - version.GetString(), - ), - middleware.Logger( - options.Logger, - ), + middlewares := []func(stdhttp.Handler) stdhttp.Handler{ + middleware.TraceContext, + chimiddleware.RequestID, + middleware.Version( + "graph", + version.GetString(), + ), + middleware.Logger( + options.Logger, + ), + } + // how do we secure the api? + var requireAdminMiddleware func(stdhttp.Handler) stdhttp.Handler + var roleService svc.RoleService + var gatewayClient svc.GatewayClient + if options.Config.HTTP.APIToken == "" { + middlewares = append(middlewares, graphMiddleware.Auth( account.Logger(options.Logger), account.JWTSecret(options.Config.TokenManager.JWTSecret), - ), - ), + )) + roleService = settingssvc.NewRoleService("com.owncloud.api.settings", grpc.DefaultClient()) + gatewayClient, err = pool.GetGatewayServiceClient(options.Config.Reva.Address, options.Config.Reva.GetRevaOptions()...) + if err != nil { + return http.Service{}, errors.Wrap(err, "could not initialize gateway client") + } + } else { + middlewares = append(middlewares, middleware.Token(options.Config.HTTP.APIToken)) + // use a dummy admin middleware for the chi router + requireAdminMiddleware = func(next stdhttp.Handler) stdhttp.Handler { + return next + } + // no gatewayclient needed + } + + handle := svc.NewService( + svc.Logger(options.Logger), + svc.Config(options.Config), + svc.Middleware(middlewares...), svc.EventsPublisher(publisher), + svc.WithRoleService(roleService), + svc.WithRequireAdminMiddleware(requireAdminMiddleware), + svc.WithGatewayClient(gatewayClient), ) if handle == nil { diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go index 50c0977c7d..3dd8abbe86 100644 --- a/services/graph/pkg/service/v0/groups.go +++ b/services/graph/pkg/service/v0/groups.go @@ -92,7 +92,7 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) { currentUser := revactx.ContextMustGetUser(r.Context()) g.publishEvent(events.GroupCreated{Executant: currentUser.Id, GroupID: *grp.Id}) } - render.Status(r, http.StatusOK) + render.Status(r, http.StatusOK) // FIXME 201 should return 201 created render.JSON(w, r, grp) } @@ -167,7 +167,7 @@ func (g Graph) PatchGroup(w http.ResponseWriter, r *http.Request) { } return } - render.Status(r, http.StatusNoContent) + render.Status(r, http.StatusNoContent) // TODO StatusNoContent when prefer=minimal is used, otherwise OK and the resource in the body render.NoContent(w, r) } diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go index c09be377f9..02cbe9d71f 100644 --- a/services/graph/pkg/service/v0/groups_test.go +++ b/services/graph/pkg/service/v0/groups_test.go @@ -222,7 +222,7 @@ var _ = Describe("Groups", func() { Expect(rr.Code).To(Equal(http.StatusBadRequest)) }) - It("disallows user create ids", func() { + It("disallows group create ids", func() { newGroup = libregraph.NewGroup() newGroup.SetId("disallowed") newGroup.SetDisplayName("New Group") diff --git a/services/graph/pkg/service/v0/option.go b/services/graph/pkg/service/v0/option.go index a9a70b5bbd..8588fe4dd8 100644 --- a/services/graph/pkg/service/v0/option.go +++ b/services/graph/pkg/service/v0/option.go @@ -16,15 +16,16 @@ type Option func(o *Options) // Options defines the available options for this package. type Options struct { - Logger log.Logger - Config *config.Config - Middleware []func(http.Handler) http.Handler - GatewayClient GatewayClient - IdentityBackend identity.Backend - RoleService RoleService - PermissionService Permissions - RoleManager *roles.Manager - EventsPublisher events.Publisher + Logger log.Logger + Config *config.Config + Middleware []func(http.Handler) http.Handler + RequireAdminMiddleware func(http.Handler) http.Handler + GatewayClient GatewayClient + IdentityBackend identity.Backend + RoleService RoleService + PermissionService Permissions + RoleManager *roles.Manager + EventsPublisher events.Publisher } // newOptions initializes the available default options. @@ -59,6 +60,13 @@ func Middleware(val ...func(http.Handler) http.Handler) Option { } } +// WithRequireAdminMiddleware provides a function to set the RequireAdminMiddleware option. +func WithRequireAdminMiddleware(val func(http.Handler) http.Handler) Option { + return func(o *Options) { + o.RequireAdminMiddleware = val + } +} + // WithGatewayClient provides a function to set the gateway client option. func WithGatewayClient(val GatewayClient) Option { return func(o *Options) { diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 6e3f67a18c..c08c5ebdd6 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -7,7 +7,6 @@ import ( "os" "strconv" - "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" "github.com/jellydator/ttlcache/v2" @@ -69,17 +68,9 @@ func NewService(opts ...Option) Service { logger: &options.Logger, spacePropertiesCache: ttlcache.NewCache(), eventsPublisher: options.EventsPublisher, + gatewayClient: options.GatewayClient, } - if options.GatewayClient == nil { - var err error - svc.gatewayClient, err = pool.GetGatewayServiceClient(options.Config.Reva.Address, options.Config.Reva.GetRevaOptions()...) - if err != nil { - options.Logger.Error().Err(err).Msg("Could not get gateway client") - return nil - } - } else { - svc.gatewayClient = options.GatewayClient - } + if options.IdentityBackend == nil { switch options.Config.Identity.Backend { case "cs3": @@ -145,12 +136,6 @@ func NewService(opts ...Option) Service { svc.identityBackend = options.IdentityBackend } - if options.RoleService == nil { - svc.roleService = settingssvc.NewRoleService("com.owncloud.api.settings", grpc.DefaultClient()) - } else { - svc.roleService = options.RoleService - } - if options.PermissionService == nil { svc.permissionsService = settingssvc.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient()) } else { @@ -167,12 +152,17 @@ func NewService(opts ...Option) Service { m := roles.NewManager( roles.StoreOptions(storeOptions), roles.Logger(options.Logger), - roles.RoleService(svc.roleService), + roles.RoleService(options.RoleService), ) roleManager = &m } - requireAdmin := graphm.RequireAdmin(roleManager, options.Logger) + var requireAdmin func(http.Handler) http.Handler + if options.RequireAdminMiddleware == nil { + requireAdmin = graphm.RequireAdmin(roleManager, options.Logger) + } else { + requireAdmin = options.RequireAdminMiddleware + } m.Route(options.Config.HTTP.Root, func(r chi.Router) { r.Use(middleware.StripSlashes) diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index 9089a00e2d..dc9253ffaa 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -158,28 +158,28 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) { return } - // All users get the user role by default currently. - // to all new users for now, as create Account request does not have any role field - if g.roleService == nil { - // log as error, admin needs to do something about it - logger.Error().Str("id", *u.Id).Msg("could not create user: role service not configured") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not assign role to account: roleService not configured") - return - } - if _, err = g.roleService.AssignRoleToUser(r.Context(), &settings.AssignRoleToUserRequest{ - AccountUuid: *u.Id, - RoleId: settingssvc.BundleUUIDRoleUser, - }); err != nil { - // log as error, admin eventually needs to do something - logger.Error().Err(err).Str("id", *u.Id).Str("role", settingssvc.BundleUUIDRoleUser).Msg("could not create user: role assignment failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "role assignment failed") - return + // assign roles if possible + if g.roleService != nil { + // All users get the user role by default currently. + // to all new users for now, as create Account request does not have any role field + if _, err = g.roleService.AssignRoleToUser(r.Context(), &settings.AssignRoleToUserRequest{ + AccountUuid: *u.Id, + RoleId: settingssvc.BundleUUIDRoleUser, + }); err != nil { + // log as error, admin eventually needs to do something + logger.Error().Err(err).Str("id", *u.Id).Str("role", settingssvc.BundleUUIDRoleUser).Msg("could not create user: role assignment failed") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "role assignment failed") + return + } } - currentUser := revactx.ContextMustGetUser(r.Context()) - g.publishEvent(events.UserCreated{Executant: currentUser.Id, UserID: *u.Id}) + e := events.UserCreated{UserID: *u.Id} + if currentUser, ok := revactx.ContextGetUser(r.Context()); ok { + e.Executant = currentUser.GetId() + } + g.publishEvent(e) - render.Status(r, http.StatusOK) + render.Status(r, http.StatusOK) // FIXME 201 should return 201 created render.JSON(w, r, u) } @@ -307,65 +307,69 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { return } - currentUser := revactx.ContextMustGetUser(r.Context()) - - if currentUser.GetId().GetOpaqueId() == user.GetId() { - logger.Debug().Msg("could not delete user: self deletion forbidden") - errorcode.NotAllowed.Render(w, r, http.StatusForbidden, "self deletion forbidden") - return - } - - logger.Debug(). - Str("user", user.GetId()). - Msg("calling list spaces with user filter to fetch the personal space for deletion") - opaque := utils.AppendPlainToOpaque(nil, "unrestricted", "T") - f := listStorageSpacesUserFilter(user.GetId()) - lspr, err := g.gatewayClient.ListStorageSpaces(r.Context(), &storageprovider.ListStorageSpacesRequest{ - Opaque: opaque, - Filters: []*storageprovider.ListStorageSpacesRequest_Filter{f}, - }) - if err != nil { - // transport error, log as error - logger.Error().Err(err).Msg("could not fetch spaces: transport error") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not fetch spaces for deletion, aborting") - return - } - for _, sp := range lspr.GetStorageSpaces() { - if !(sp.SpaceType == "personal" && sp.Owner.Id.OpaqueId == user.GetId()) { - continue + e := events.UserDeleted{UserID: user.GetId()} + if currentUser, ok := revactx.ContextGetUser(r.Context()); ok { + if currentUser.GetId().GetOpaqueId() == user.GetId() { + logger.Debug().Msg("could not delete user: self deletion forbidden") + errorcode.NotAllowed.Render(w, r, http.StatusForbidden, "self deletion forbidden") + return } - // TODO: check if request contains a homespace and if, check if requesting user has the privilege to - // delete it and make sure it is not deleting its own homespace - // needs modification of the cs3api + e.Executant = currentUser.GetId() + } - // Deleting a space a two step process (1. disabling/trashing, 2. purging) - // Do the "disable/trash" step only if the space is not marked as trashed yet: - if _, ok := sp.Opaque.Map["trashed"]; !ok { + if g.gatewayClient != nil { + logger.Debug(). + Str("user", user.GetId()). + Msg("calling list spaces with user filter to fetch the personal space for deletion") + opaque := utils.AppendPlainToOpaque(nil, "unrestricted", "T") + f := listStorageSpacesUserFilter(user.GetId()) + lspr, err := g.gatewayClient.ListStorageSpaces(r.Context(), &storageprovider.ListStorageSpacesRequest{ + Opaque: opaque, + Filters: []*storageprovider.ListStorageSpacesRequest_Filter{f}, + }) + if err != nil { + // transport error, log as error + logger.Error().Err(err).Msg("could not fetch spaces: transport error") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not fetch spaces for deletion, aborting") + return + } + for _, sp := range lspr.GetStorageSpaces() { + if !(sp.SpaceType == "personal" && sp.Owner.Id.OpaqueId == user.GetId()) { + continue + } + // TODO: check if request contains a homespace and if, check if requesting user has the privilege to + // delete it and make sure it is not deleting its own homespace + // needs modification of the cs3api + + // Deleting a space a two step process (1. disabling/trashing, 2. purging) + // Do the "disable/trash" step only if the space is not marked as trashed yet: + if _, ok := sp.Opaque.Map["trashed"]; !ok { + _, err := g.gatewayClient.DeleteStorageSpace(r.Context(), &storageprovider.DeleteStorageSpaceRequest{ + Id: &storageprovider.StorageSpaceId{ + OpaqueId: sp.Id.OpaqueId, + }, + }) + if err != nil { + logger.Error().Err(err).Msg("could not disable homespace: transport error") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not disable homespace, aborting") + return + } + } + purgeFlag := utils.AppendPlainToOpaque(nil, "purge", "") _, err := g.gatewayClient.DeleteStorageSpace(r.Context(), &storageprovider.DeleteStorageSpaceRequest{ + Opaque: purgeFlag, Id: &storageprovider.StorageSpaceId{ OpaqueId: sp.Id.OpaqueId, }, }) if err != nil { - logger.Error().Err(err).Msg("could not disable homespace: transport error") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not disable homespace, aborting") + // transport error, log as error + logger.Error().Err(err).Msg("could not delete homespace: transport error") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not delete homespace, aborting") return } + break } - purgeFlag := utils.AppendPlainToOpaque(nil, "purge", "") - _, err := g.gatewayClient.DeleteStorageSpace(r.Context(), &storageprovider.DeleteStorageSpaceRequest{ - Opaque: purgeFlag, - Id: &storageprovider.StorageSpaceId{ - OpaqueId: sp.Id.OpaqueId, - }, - }) - if err != nil { - // transport error, log as error - logger.Error().Err(err).Msg("could not delete homespace: transport error") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "could not delete homespace, aborting") - return - } - break } logger.Debug().Str("id", user.GetId()).Msg("calling delete user on backend") @@ -382,7 +386,7 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { } } - g.publishEvent(events.UserDeleted{Executant: currentUser.Id, UserID: user.GetId()}) + g.publishEvent(e) render.Status(r, http.StatusNoContent) render.NoContent(w, r) @@ -443,15 +447,16 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { return } - currentUser := revactx.ContextMustGetUser(r.Context()) - g.publishEvent( - events.UserFeatureChanged{ - Executant: currentUser.Id, - UserID: nameOrID, - Features: features, - }, - ) - render.Status(r, http.StatusOK) + e := events.UserFeatureChanged{ + UserID: nameOrID, + Features: features, + } + if currentUser, ok := revactx.ContextGetUser(r.Context()); ok { + e.Executant = currentUser.GetId() + } + g.publishEvent(e) + + render.Status(r, http.StatusOK) // TODO StatusNoContent when prefer=minimal is used render.JSON(w, r, u) } diff --git a/tests/acceptance/features/bootstrap/GraphContext.php b/tests/acceptance/features/bootstrap/GraphContext.php index dc5bca8276..c9e4276f87 100644 --- a/tests/acceptance/features/bootstrap/GraphContext.php +++ b/tests/acceptance/features/bootstrap/GraphContext.php @@ -84,7 +84,7 @@ class GraphContext implements Context { $displayName ); $this->featureContext->setResponse($response); - $this->featureContext->theHttpStatusCodeShouldBe(200); + $this->featureContext->theHttpStatusCodeShouldBe(200); // TODO 204 when prefer=minimal header was sent } /**