From 2cff4cf0bbd4c7e35cbae591a50d520449a932f5 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 23 Apr 2021 13:35:01 +0200 Subject: [PATCH 01/15] rework of PR#755 --- .drone.star | 2 +- ocs/pkg/service/v0/users.go | 111 ++++++++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/.drone.star b/.drone.star index 532558c39..841270a77 100644 --- a/.drone.star +++ b/.drone.star @@ -454,7 +454,7 @@ def localApiTests(ctx, storage = 'owncloud', suite = 'apiBugDemonstration', acco 'environment' : { 'TEST_SERVER_URL': 'https://ocis-server:9200', 'OCIS_REVA_DATA_ROOT': '%s' % ('/srv/app/tmp/ocis/owncloud/data/' if storage == 'owncloud' else ''), - 'DELETE_USER_DATA_CMD': '%s' % ('' if storage == 'owncloud' else 'rm -rf /srv/app/tmp/ocis/storage/users/nodes/root/* /srv/app/tmp/ocis/storage/users/nodes/*-*-*-*'), + #'DELETE_USER_DATA_CMD': '%s' % ('' if storage == 'owncloud' else 'rm -rf /srv/app/tmp/ocis/storage/users/nodes/root/* /srv/app/tmp/ocis/storage/users/nodes/*-*-*-*'), 'SKELETON_DIR': '/srv/app/tmp/testing/data/apiSkeleton', 'OCIS_SKELETON_STRATEGY': '%s' % ('copy' if storage == 'owncloud' else 'upload'), 'TEST_OCIS':'true', diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index ac25d0541..335cc3cb3 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -10,18 +10,26 @@ import ( "strings" "github.com/asim/go-micro/plugins/client/grpc/v3" + merrors "github.com/asim/go-micro/v3/errors" + cs3 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + revauser "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/pkg/token" + "github.com/cs3org/reva/pkg/token/manager/jwt" "github.com/cs3org/reva/pkg/user" "github.com/go-chi/chi" "github.com/go-chi/render" - "google.golang.org/genproto/protobuf/field_mask" - "google.golang.org/protobuf/types/known/fieldmaskpb" - - merrors "github.com/asim/go-micro/v3/errors" - cs3 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/ocs/pkg/service/v0/data" "github.com/owncloud/ocis/ocs/pkg/service/v0/response" storepb "github.com/owncloud/ocis/store/pkg/proto/v0" + "github.com/pkg/errors" + "google.golang.org/genproto/protobuf/field_mask" + "google.golang.org/grpc/metadata" + "google.golang.org/protobuf/types/known/fieldmaskpb" ) // GetSelf returns the currently logged in user @@ -358,6 +366,70 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { return } + // Note that the previous conditional is short circuiting this. This logic will NOT run when the user's backend is + // configured to use any other than the accounts service. + t, err := o.mintTokenForUser(r.Context(), account) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not mint token").Error())) + return + } + + ctx := metadata.AppendToOutgoingContext(r.Context(), token.TokenHeader, t) + + gwc, err := pool.GetGatewayServiceClient(o.config.RevaAddress) + if err != nil { + o.logger.Error().Err(err).Msg("error securing a connection to the reva gateway, could not delete user home") + } + + homeResp, err := gwc.GetHome(ctx, &provider.GetHomeRequest{}) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not get home").Error())) + return + } + + statResp, err := gwc.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: homeResp.Path, + }, + }, + }) + + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not stat home").Error())) + return + } + + if statResp.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", statResp.Status.Code.String()). + Str("stat_message", statResp.Status.Message). + Msg("DeleteUser: could not delete user home: stat failed") + return + } + + delReq := &provider.DeleteRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{ + Id: statResp.Info.Id, + }, + }, + } + + delResp, err := gwc.Delete(ctx, delReq) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete home").Error())) + return + } + + if delResp.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", statResp.Status.Code.String()). + Str("stat_message", statResp.Status.Message). + Msg("DeleteUser: could not delete user home: delete failed") + return + } + req := accounts.DeleteAccountRequest{ Id: account.Id, } @@ -378,6 +450,35 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { mustNotFail(render.Render(w, r, response.DataRender(struct{}{}))) } +// TODO(refs) this to ocis-pkg ... we are minting tokens all over the place ... or use a service? ... like reva? +func (o Ocs) mintTokenForUser(ctx context.Context, account *accounts.Account) (string, error) { + tm, _ := jwt.New(map[string]interface{}{ + "secret": "Pive-Fumkiu4", // TODO(refs) this MUST not be hardcoded + "expires": int64(60), + }) + + u := &revauser.User{ + Id: &revauser.UserId{ + OpaqueId: account.Id, + Idp: "https://localhost:9200", // TODO(refs) this MUST not be hardcoded + }, + Groups: []string{}, + Opaque: &types.Opaque{ + Map: map[string]*types.OpaqueEntry{ + "uid": { + Decoder: "plain", + Value: []byte(strconv.FormatInt(account.UidNumber, 10)), + }, + "gid": { + Decoder: "plain", + Value: []byte(strconv.FormatInt(account.GidNumber, 10)), + }, + }, + }, + } + return tm.MintToken(ctx, u) +} + // EnableUser enables a user func (o Ocs) EnableUser(w http.ResponseWriter, r *http.Request) { userid := chi.URLParam(r, "userid") From 5fd121e84cdb4c734f6b325184d0b34150d29557 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 23 Apr 2021 14:41:48 +0200 Subject: [PATCH 02/15] add indirect packages --- ocs/go.mod | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ocs/go.mod b/ocs/go.mod index 920a17f6a..c573fbdb8 100644 --- a/ocs/go.mod +++ b/ocs/go.mod @@ -22,12 +22,14 @@ require ( github.com/owncloud/ocis/proxy v0.0.0-20210412105747-9b95e9b1191b github.com/owncloud/ocis/settings v0.0.0-20210413063522-955bd60edf33 github.com/owncloud/ocis/store v0.0.0-20210413063522-955bd60edf33 + github.com/pkg/errors v0.9.1 // indirect github.com/prometheus/client_golang v1.10.0 github.com/spf13/viper v1.7.1 github.com/stretchr/testify v1.7.0 github.com/thejerf/suture/v4 v4.0.0 go.opencensus.io v0.23.0 google.golang.org/genproto v0.0.0-20210207032614-bba0dbe2a9ea + google.golang.org/grpc v1.37.0 // indirect google.golang.org/protobuf v1.26.0 ) From b820d93485a1ae770c204c2a4813c2432156e795 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Fri, 23 Apr 2021 15:45:40 +0200 Subject: [PATCH 03/15] delete user home only if reva gateway is configured. This ensures tests passing in CI --- ocs/pkg/service/v0/users.go | 104 ++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index 335cc3cb3..89bf87c6b 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -366,68 +366,70 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { return } - // Note that the previous conditional is short circuiting this. This logic will NOT run when the user's backend is - // configured to use any other than the accounts service. - t, err := o.mintTokenForUser(r.Context(), account) - if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not mint token").Error())) - return - } + if o.config.RevaAddress != "" { + // Note that the previous conditional is short circuiting this. This logic will NOT run when the user's backend is + // configured to use any other than the accounts service. + t, err := o.mintTokenForUser(r.Context(), account) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not mint token").Error())) + return + } - ctx := metadata.AppendToOutgoingContext(r.Context(), token.TokenHeader, t) + ctx := metadata.AppendToOutgoingContext(r.Context(), token.TokenHeader, t) - gwc, err := pool.GetGatewayServiceClient(o.config.RevaAddress) - if err != nil { - o.logger.Error().Err(err).Msg("error securing a connection to the reva gateway, could not delete user home") - } + gwc, err := pool.GetGatewayServiceClient(o.config.RevaAddress) + if err != nil { + o.logger.Error().Err(err).Msg("error securing a connection to the reva gateway, could not delete user home") + } - homeResp, err := gwc.GetHome(ctx, &provider.GetHomeRequest{}) - if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not get home").Error())) - return - } + homeResp, err := gwc.GetHome(ctx, &provider.GetHomeRequest{}) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not get home").Error())) + return + } - statResp, err := gwc.Stat(ctx, &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: homeResp.Path, + statResp, err := gwc.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: homeResp.Path, + }, }, - }, - }) + }) - if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not stat home").Error())) - return - } + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not stat home").Error())) + return + } - if statResp.Status.Code != rpcv1beta1.Code_CODE_OK { - o.logger.Error(). - Str("stat_status_code", statResp.Status.Code.String()). - Str("stat_message", statResp.Status.Message). - Msg("DeleteUser: could not delete user home: stat failed") - return - } + if statResp.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", statResp.Status.Code.String()). + Str("stat_message", statResp.Status.Message). + Msg("DeleteUser: could not delete user home: stat failed") + return + } - delReq := &provider.DeleteRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Id{ - Id: statResp.Info.Id, + delReq := &provider.DeleteRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{ + Id: statResp.Info.Id, + }, }, - }, - } + } - delResp, err := gwc.Delete(ctx, delReq) - if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete home").Error())) - return - } + delResp, err := gwc.Delete(ctx, delReq) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete home").Error())) + return + } - if delResp.Status.Code != rpcv1beta1.Code_CODE_OK { - o.logger.Error(). - Str("stat_status_code", statResp.Status.Code.String()). - Str("stat_message", statResp.Status.Message). - Msg("DeleteUser: could not delete user home: delete failed") - return + if delResp.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", statResp.Status.Code.String()). + Str("stat_message", statResp.Status.Message). + Msg("DeleteUser: could not delete user home: delete failed") + return + } } req := accounts.DeleteAccountRequest{ From 81b7fad8f5c5e9d03441fcad3d30dadc4dbed6a7 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Mon, 26 Apr 2021 11:11:47 +0200 Subject: [PATCH 04/15] comment out delete user cmd from coreApiTests --- .drone.star | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.star b/.drone.star index 841270a77..b4d70f976 100644 --- a/.drone.star +++ b/.drone.star @@ -501,7 +501,7 @@ def coreApiTests(ctx, part_number = 1, number_of_parts = 1, storage = 'owncloud' 'environment' : { 'TEST_SERVER_URL': 'https://ocis-server:9200', 'OCIS_REVA_DATA_ROOT': '%s' % ('/srv/app/tmp/ocis/owncloud/data/' if storage == 'owncloud' else ''), - 'DELETE_USER_DATA_CMD': '%s' % ('' if storage == 'owncloud' else 'rm -rf /srv/app/tmp/ocis/storage/users/nodes/root/* /srv/app/tmp/ocis/storage/users/nodes/*-*-*-*'), + #'DELETE_USER_DATA_CMD': '%s' % ('' if storage == 'owncloud' else 'rm -rf /srv/app/tmp/ocis/storage/users/nodes/root/* /srv/app/tmp/ocis/storage/users/nodes/*-*-*-*'), 'SKELETON_DIR': '/srv/app/tmp/testing/data/apiSkeleton', 'OCIS_SKELETON_STRATEGY': '%s' % ('copy' if storage == 'owncloud' else 'upload'), 'TEST_OCIS':'true', From 75be5783c777fec245471b3480dd3f93c22bf6cd Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Mon, 26 Apr 2021 12:32:02 +0200 Subject: [PATCH 05/15] configurable jwt-secret and idm_address for ocs --- ocs/pkg/config/config.go | 26 +++++++++++++++++--------- ocs/pkg/flagset/flagset.go | 7 +++++++ ocs/pkg/service/v0/users.go | 4 ++-- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/ocs/pkg/config/config.go b/ocs/pkg/config/config.go index 451701210..9c9a6693c 100644 --- a/ocs/pkg/config/config.go +++ b/ocs/pkg/config/config.go @@ -45,17 +45,25 @@ type TokenManager struct { JWTSecret string } +// IdentityManagement keeps track of the OIDC address. This is because Reva requisite of uniqueness for users +// is based in the combination of IDP hostname + UserID. For more information see: +// https://github.com/cs3org/reva/blob/4fd0229f13fae5bc9684556a82dbbd0eced65ef9/pkg/storage/utils/decomposedfs/node/node.go#L856-L865 +type IdentityManagement struct { + Address string +} + // Config combines all available configuration parts. type Config struct { - File string - Log Log - Debug Debug - HTTP HTTP - Tracing Tracing - TokenManager TokenManager - Service Service - AccountBackend string - RevaAddress string + File string + Log Log + Debug Debug + HTTP HTTP + Tracing Tracing + TokenManager TokenManager + Service Service + AccountBackend string + RevaAddress string + IdentityManagement IdentityManagement Context context.Context Supervised bool diff --git a/ocs/pkg/flagset/flagset.go b/ocs/pkg/flagset/flagset.go index d1924af67..ddfd49200 100644 --- a/ocs/pkg/flagset/flagset.go +++ b/ocs/pkg/flagset/flagset.go @@ -165,6 +165,13 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"OCS_REVA_GATEWAY_ADDR"}, Destination: &cfg.RevaAddress, }, + &cli.StringFlag{ + Name: "idm-address", + Value: flags.OverrideDefaultString(cfg.IdentityManagement.Address, "https://localhost:9200"), + EnvVars: []string{"OCS_IDM_ADDRESS", "OCIS_URL"}, + Usage: "keeps track of the IDM Address. Needed because of Reva requisite of uniqueness for users", + Destination: &cfg.IdentityManagement.Address, + }, } } diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index 89bf87c6b..613a5ef85 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -455,14 +455,14 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { // TODO(refs) this to ocis-pkg ... we are minting tokens all over the place ... or use a service? ... like reva? func (o Ocs) mintTokenForUser(ctx context.Context, account *accounts.Account) (string, error) { tm, _ := jwt.New(map[string]interface{}{ - "secret": "Pive-Fumkiu4", // TODO(refs) this MUST not be hardcoded + "secret": o.config.TokenManager.JWTSecret, "expires": int64(60), }) u := &revauser.User{ Id: &revauser.UserId{ OpaqueId: account.Id, - Idp: "https://localhost:9200", // TODO(refs) this MUST not be hardcoded + Idp: o.config.IdentityManagement.Address, }, Groups: []string{}, Opaque: &types.Opaque{ From d976dc35d30e05ed5f87c82fdaf3c58ec99bf2a9 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Mon, 26 Apr 2021 13:15:07 +0200 Subject: [PATCH 06/15] add ugly CI hack: os.Getenv("STORAGE_USERS_DRIVER") != "owncloud" --- ocs/pkg/service/v0/users.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index 613a5ef85..7cebf56a3 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "fmt" "net/http" + "os" "strconv" "strings" @@ -366,9 +367,7 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { return } - if o.config.RevaAddress != "" { - // Note that the previous conditional is short circuiting this. This logic will NOT run when the user's backend is - // configured to use any other than the accounts service. + if o.config.RevaAddress != "" && os.Getenv("STORAGE_USERS_DRIVER") != "owncloud" { t, err := o.mintTokenForUser(r.Context(), account) if err != nil { render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not mint token").Error())) @@ -388,6 +387,14 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { return } + if homeResp.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", homeResp.Status.Code.String()). + Str("stat_message", homeResp.Status.Message). + Msg("DeleteUser: could not get user home: get failed") + return + } + statResp, err := gwc.Stat(ctx, &provider.StatRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{ From 6497743a02afbf5c5c87443ab7d0ac2f75c7cf13 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Mon, 26 Apr 2021 14:27:36 +0200 Subject: [PATCH 07/15] purge recycle bin --- ocs/pkg/service/v0/users.go | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index 7cebf56a3..d8dac62b1 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -12,6 +12,7 @@ import ( "github.com/asim/go-micro/plugins/client/grpc/v3" merrors "github.com/asim/go-micro/v3/errors" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" cs3 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" revauser "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -437,6 +438,54 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { Msg("DeleteUser: could not delete user home: delete failed") return } + + // delete trash is a combination of ListRecycle + PurgeRecycle (iterative) + listRecycle := &gateway.ListRecycleRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: homeResp.Path, + }, + }, + } + + listRecycleResponse, err := gwc.ListRecycle(ctx, listRecycle) + if err != nil { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete trash").Error())) + return + } + + if listRecycleResponse.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", statResp.Status.Code.String()). + Str("stat_message", statResp.Status.Message). + Msg("DeleteUser: could not delete user trash: delete failed") + return + } + + // now that we've got the items, we iterate, create references and fire PurgeRecycleRequests to the Reva gateway. + //for i := range listRecycleResponse.RecycleItems { + // craft purge request + req := &gateway.PurgeRecycleRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: homeResp.Path, + }, + }, + } + + // do request + purgeRecycleResponse, err := gwc.PurgeRecycle(ctx, req) + if err != nil { + + } + + if purgeRecycleResponse.Status.Code != rpcv1beta1.Code_CODE_OK { + o.logger.Error(). + Str("stat_status_code", statResp.Status.Code.String()). + Str("stat_message", statResp.Status.Message). + Msg("DeleteUser: could not delete user trash: delete failed") + return + } } req := accounts.DeleteAccountRequest{ From 0bc0e2a7a44a1c537b262b127c78e95a1b9c2d77 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Mon, 26 Apr 2021 16:18:52 +0200 Subject: [PATCH 08/15] adjust expected failures --- tests/acceptance/expected-failures-API-on-OCIS-storage.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/acceptance/expected-failures-API-on-OCIS-storage.md b/tests/acceptance/expected-failures-API-on-OCIS-storage.md index 04482df81..036de2c69 100644 --- a/tests/acceptance/expected-failures-API-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-API-on-OCIS-storage.md @@ -1562,8 +1562,6 @@ special character username not valid - [apiProvisioning-v2/enableUser.feature:20](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiProvisioning-v2/enableUser.feature#L20) - [apiProvisioning-v2/getUser.feature:34](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiProvisioning-v2/getUser.feature#L34) - [apiProvisioning-v2/getUser.feature:35](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiProvisioning-v2/getUser.feature#L35) -- [apiTrashbin/trashbinFilesFolders.feature:201](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L201) -- [apiTrashbin/trashbinFilesFolders.feature:202](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L202) - [apiTrashbin/trashbinFilesFolders.feature:246](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L246) - [apiTrashbin/trashbinFilesFolders.feature:247](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L247) - [apiTrashbin/trashbinFilesFolders.feature:248](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L248) From 4fb04b8ee34b98a20bfa4ca24dc2703cb6e44e31 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 27 Apr 2021 13:12:04 +0200 Subject: [PATCH 09/15] fix sonarcloud offenses --- ocs/pkg/config/config.go | 1 + ocs/pkg/flagset/flagset.go | 7 ++++++ ocs/pkg/service/v0/users.go | 43 +++++++------------------------------ 3 files changed, 16 insertions(+), 35 deletions(-) diff --git a/ocs/pkg/config/config.go b/ocs/pkg/config/config.go index 9c9a6693c..d43c06f9c 100644 --- a/ocs/pkg/config/config.go +++ b/ocs/pkg/config/config.go @@ -63,6 +63,7 @@ type Config struct { Service Service AccountBackend string RevaAddress string + StorageUsersDriver string IdentityManagement IdentityManagement Context context.Context diff --git a/ocs/pkg/flagset/flagset.go b/ocs/pkg/flagset/flagset.go index ddfd49200..b2b1d5f8f 100644 --- a/ocs/pkg/flagset/flagset.go +++ b/ocs/pkg/flagset/flagset.go @@ -172,6 +172,13 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { Usage: "keeps track of the IDM Address. Needed because of Reva requisite of uniqueness for users", Destination: &cfg.IdentityManagement.Address, }, + &cli.StringFlag{ + Name: "users-driver", + Value: flags.OverrideDefaultString(cfg.StorageUsersDriver, "ocis"), + Usage: "storage driver for users mount: eg. local, eos, owncloud, ocis or s3", + EnvVars: []string{"STORAGE_USERS_DRIVER"}, + Destination: &cfg.StorageUsersDriver, + }, } } diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index d8dac62b1..298e62015 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -6,7 +6,6 @@ import ( "encoding/hex" "fmt" "net/http" - "os" "strconv" "strings" @@ -368,10 +367,10 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { return } - if o.config.RevaAddress != "" && os.Getenv("STORAGE_USERS_DRIVER") != "owncloud" { + if o.config.RevaAddress != "" && o.config.StorageUsersDriver != "owncloud" { t, err := o.mintTokenForUser(r.Context(), account) if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not mint token").Error())) + mustNotFail(render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "error minting token").Error()))) return } @@ -379,12 +378,12 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { gwc, err := pool.GetGatewayServiceClient(o.config.RevaAddress) if err != nil { - o.logger.Error().Err(err).Msg("error securing a connection to the reva gateway, could not delete user home") + o.logger.Error().Err(err).Msg("error securing a connection to Reva gateway") } homeResp, err := gwc.GetHome(ctx, &provider.GetHomeRequest{}) if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not get home").Error())) + mustNotFail(render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not get home").Error()))) return } @@ -405,7 +404,7 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { }) if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not stat home").Error())) + mustNotFail(render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not stat home").Error()))) return } @@ -427,7 +426,7 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { delResp, err := gwc.Delete(ctx, delReq) if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete home").Error())) + mustNotFail(render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete home").Error()))) return } @@ -439,32 +438,6 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { return } - // delete trash is a combination of ListRecycle + PurgeRecycle (iterative) - listRecycle := &gateway.ListRecycleRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: homeResp.Path, - }, - }, - } - - listRecycleResponse, err := gwc.ListRecycle(ctx, listRecycle) - if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete trash").Error())) - return - } - - if listRecycleResponse.Status.Code != rpcv1beta1.Code_CODE_OK { - o.logger.Error(). - Str("stat_status_code", statResp.Status.Code.String()). - Str("stat_message", statResp.Status.Message). - Msg("DeleteUser: could not delete user trash: delete failed") - return - } - - // now that we've got the items, we iterate, create references and fire PurgeRecycleRequests to the Reva gateway. - //for i := range listRecycleResponse.RecycleItems { - // craft purge request req := &gateway.PurgeRecycleRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{ @@ -473,10 +446,10 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { }, } - // do request purgeRecycleResponse, err := gwc.PurgeRecycle(ctx, req) if err != nil { - + mustNotFail(render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, errors.Wrap(err, "could not delete trash").Error()))) + return } if purgeRecycleResponse.Status.Code != rpcv1beta1.Code_CODE_OK { From e6cc66c2962b0eb6d22133c7c68f07560d2ec73f Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 27 Apr 2021 13:50:10 +0200 Subject: [PATCH 10/15] add changelopg --- changelog/unreleased/ocs-user-deprovisioning.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 changelog/unreleased/ocs-user-deprovisioning.md diff --git a/changelog/unreleased/ocs-user-deprovisioning.md b/changelog/unreleased/ocs-user-deprovisioning.md new file mode 100644 index 000000000..f2d9c0f2d --- /dev/null +++ b/changelog/unreleased/ocs-user-deprovisioning.md @@ -0,0 +1,15 @@ +Enhancement: User Deprovisioning for the OCS API + +Use the CS3 API and Reva to deprovision users completely. + +Two new environment variables introduced: +``` +OCS_IDM_ADDRESS +STORAGE_USERS_DRIVER +``` + +`OCS_IDM_ADDRESS` is also an alias for `OCIS_URL`; allows the OCS service to mint jwt tokens for the authenticated user that will be read by the reva authentication middleware. + +`STORAGE_USERS_DRIVER` determines how a user is deprovisioned. This kind of behavior is needed since every storage driver deals with deleting differently. + +https://github.com/owncloud/ocis/pull/1962 From aaddbdaf9a472e70f3fa955c9fe9cd10e445eed1 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 27 Apr 2021 16:54:58 +0200 Subject: [PATCH 11/15] adjust expected failures --- tests/acceptance/expected-failures-API-on-OCIS-storage.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/acceptance/expected-failures-API-on-OCIS-storage.md b/tests/acceptance/expected-failures-API-on-OCIS-storage.md index 04482df81..036de2c69 100644 --- a/tests/acceptance/expected-failures-API-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-API-on-OCIS-storage.md @@ -1562,8 +1562,6 @@ special character username not valid - [apiProvisioning-v2/enableUser.feature:20](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiProvisioning-v2/enableUser.feature#L20) - [apiProvisioning-v2/getUser.feature:34](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiProvisioning-v2/getUser.feature#L34) - [apiProvisioning-v2/getUser.feature:35](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiProvisioning-v2/getUser.feature#L35) -- [apiTrashbin/trashbinFilesFolders.feature:201](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L201) -- [apiTrashbin/trashbinFilesFolders.feature:202](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L202) - [apiTrashbin/trashbinFilesFolders.feature:246](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L246) - [apiTrashbin/trashbinFilesFolders.feature:247](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L247) - [apiTrashbin/trashbinFilesFolders.feature:248](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature#L248) From 3f0d2cc28ffb500dfd1214cdbab97cc34c5995d3 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 27 Apr 2021 22:54:49 +0200 Subject: [PATCH 12/15] add STORAGE_USERS_DRIVER as a fallback env var --- ocs/pkg/flagset/flagset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocs/pkg/flagset/flagset.go b/ocs/pkg/flagset/flagset.go index b2b1d5f8f..a5e69d7fc 100644 --- a/ocs/pkg/flagset/flagset.go +++ b/ocs/pkg/flagset/flagset.go @@ -176,7 +176,7 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { Name: "users-driver", Value: flags.OverrideDefaultString(cfg.StorageUsersDriver, "ocis"), Usage: "storage driver for users mount: eg. local, eos, owncloud, ocis or s3", - EnvVars: []string{"STORAGE_USERS_DRIVER"}, + EnvVars: []string{"OCS_STORAGE_USERS_DRIVER", "STORAGE_USERS_DRIVER"}, Destination: &cfg.StorageUsersDriver, }, } From e54e869c1deafc0e34180e9ae9085c2a2befb860 Mon Sep 17 00:00:00 2001 From: Alex Unger <6905948+refs@users.noreply.github.com> Date: Wed, 28 Apr 2021 09:32:41 +0200 Subject: [PATCH 13/15] Update ocs-user-deprovisioning.md --- changelog/unreleased/ocs-user-deprovisioning.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/ocs-user-deprovisioning.md b/changelog/unreleased/ocs-user-deprovisioning.md index f2d9c0f2d..a71e9a1a4 100644 --- a/changelog/unreleased/ocs-user-deprovisioning.md +++ b/changelog/unreleased/ocs-user-deprovisioning.md @@ -5,7 +5,7 @@ Use the CS3 API and Reva to deprovision users completely. Two new environment variables introduced: ``` OCS_IDM_ADDRESS -STORAGE_USERS_DRIVER +OCS_STORAGE_USERS_DRIVER ``` `OCS_IDM_ADDRESS` is also an alias for `OCIS_URL`; allows the OCS service to mint jwt tokens for the authenticated user that will be read by the reva authentication middleware. From 35f0e00b318efa6eb3eee4dc6144ed0193ce0fcb Mon Sep 17 00:00:00 2001 From: Alex Unger <6905948+refs@users.noreply.github.com> Date: Wed, 28 Apr 2021 11:17:39 +0200 Subject: [PATCH 14/15] Update changelog/unreleased/ocs-user-deprovisioning.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörn Friedrich Dreyer --- changelog/unreleased/ocs-user-deprovisioning.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/ocs-user-deprovisioning.md b/changelog/unreleased/ocs-user-deprovisioning.md index a71e9a1a4..9e5eb5535 100644 --- a/changelog/unreleased/ocs-user-deprovisioning.md +++ b/changelog/unreleased/ocs-user-deprovisioning.md @@ -10,6 +10,6 @@ OCS_STORAGE_USERS_DRIVER `OCS_IDM_ADDRESS` is also an alias for `OCIS_URL`; allows the OCS service to mint jwt tokens for the authenticated user that will be read by the reva authentication middleware. -`STORAGE_USERS_DRIVER` determines how a user is deprovisioned. This kind of behavior is needed since every storage driver deals with deleting differently. +`OCS_STORAGE_USERS_DRIVER` determines how a user is deprovisioned. This kind of behavior is needed since every storage driver deals with deleting differently. https://github.com/owncloud/ocis/pull/1962 From 1257bc3178c8c922cf27a41bcbfdf02cf8884144 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 28 Apr 2021 12:48:22 +0200 Subject: [PATCH 15/15] remove DELETE_USER_DATA_CMD commented from drone --- .drone.star | 2 -- 1 file changed, 2 deletions(-) diff --git a/.drone.star b/.drone.star index b4d70f976..c5183010f 100644 --- a/.drone.star +++ b/.drone.star @@ -454,7 +454,6 @@ def localApiTests(ctx, storage = 'owncloud', suite = 'apiBugDemonstration', acco 'environment' : { 'TEST_SERVER_URL': 'https://ocis-server:9200', 'OCIS_REVA_DATA_ROOT': '%s' % ('/srv/app/tmp/ocis/owncloud/data/' if storage == 'owncloud' else ''), - #'DELETE_USER_DATA_CMD': '%s' % ('' if storage == 'owncloud' else 'rm -rf /srv/app/tmp/ocis/storage/users/nodes/root/* /srv/app/tmp/ocis/storage/users/nodes/*-*-*-*'), 'SKELETON_DIR': '/srv/app/tmp/testing/data/apiSkeleton', 'OCIS_SKELETON_STRATEGY': '%s' % ('copy' if storage == 'owncloud' else 'upload'), 'TEST_OCIS':'true', @@ -501,7 +500,6 @@ def coreApiTests(ctx, part_number = 1, number_of_parts = 1, storage = 'owncloud' 'environment' : { 'TEST_SERVER_URL': 'https://ocis-server:9200', 'OCIS_REVA_DATA_ROOT': '%s' % ('/srv/app/tmp/ocis/owncloud/data/' if storage == 'owncloud' else ''), - #'DELETE_USER_DATA_CMD': '%s' % ('' if storage == 'owncloud' else 'rm -rf /srv/app/tmp/ocis/storage/users/nodes/root/* /srv/app/tmp/ocis/storage/users/nodes/*-*-*-*'), 'SKELETON_DIR': '/srv/app/tmp/testing/data/apiSkeleton', 'OCIS_SKELETON_STRATEGY': '%s' % ('copy' if storage == 'owncloud' else 'upload'), 'TEST_OCIS':'true',