Skip to content

Commit

Permalink
satellite/admin: Fix user account delete
Browse files Browse the repository at this point in the history 
Delete a user's account that was member of foreign projects failed
because the endpoint was getting the list of projects that the account
is member which implies owned project, but also the foreign projects
that's member of.

This commit fix the issue calling the right method to only get the list
of owned projects to check that they are zero before proceeding and add
a test to verify it.

Change-Id: I0071394b1b26366bad8dfff3be1fc6edd418ac85
  • Loading branch information
@ifraixedes
ifraixedes authored and Storj Robot committed Jan 3, 2024
1 parent 54ef3f1 commit 9586849
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 26 deletions.
3 changes: 1 addition & 2 deletions satellite/admin/user.go
View file
Expand Up @@ -625,7 +625,6 @@ func (server *Server) updateLimits(w http.ResponseWriter, r *http.Request) {
err.Error(), http.StatusInternalServerError)
}
}

}

func (server *Server) disableUserMFA(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -938,7 +937,7 @@ func (server *Server) deleteUser(w http.ResponseWriter, r *http.Request) {
}

// Ensure user has no own projects any longer
projects, err := server.db.Console().Projects().GetByUserID(ctx, user.ID)
projects, err := server.db.Console().Projects().GetOwn(ctx, user.ID)
if err != nil {
sendJSONError(w, "unable to list projects",
err.Error(), http.StatusInternalServerError)
Expand Down
130 changes: 106 additions & 24 deletions satellite/admin/user_test.go
View file
Expand Up @@ -19,6 +19,7 @@ import (
"storj.io/common/memory"
"storj.io/common/storj"
"storj.io/common/testcontext"
"storj.io/common/testrand"
"storj.io/common/uuid"
"storj.io/storj/private/testplanet"
"storj.io/storj/satellite"
Expand Down Expand Up @@ -671,35 +672,116 @@ func TestBillingWarnUnwarnUser(t *testing.T) {
}

func TestUserDelete(t *testing.T) {
testplanet.Run(t, testplanet.Config{
SatelliteCount: 1,
StorageNodeCount: 0,
UplinkCount: 1,
Reconfigure: testplanet.Reconfigure{
Satellite: func(_ *zap.Logger, _ int, config *satellite.Config) {
config.Admin.Address = "127.0.0.1:0"
t.Run("no member of foreign projects", func(t *testing.T) {
testplanet.Run(t, testplanet.Config{
SatelliteCount: 1,
StorageNodeCount: 0,
UplinkCount: 1,
Reconfigure: testplanet.Reconfigure{
Satellite: func(_ *zap.Logger, _ int, config *satellite.Config) {
config.Admin.Address = "127.0.0.1:0"
},
},
},
}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
address := planet.Satellites[0].Admin.Admin.Listener.Addr()
user, err := planet.Satellites[0].DB.Console().Users().GetByEmail(ctx, planet.Uplinks[0].Projects[0].Owner.Email)
require.NoError(t, err)
}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
address := planet.Satellites[0].Admin.Admin.Listener.Addr()
user, err := planet.Satellites[0].DB.Console().Users().GetByEmail(ctx, planet.Uplinks[0].Projects[0].Owner.Email)
require.NoError(t, err)

// Deleting the user should fail, as project exists.
link := fmt.Sprintf("http://"+address.String()+"/api/users/%s", user.Email)
body := assertReq(ctx, t, link, http.MethodDelete, "", http.StatusConflict, "", planet.Satellites[0].Config.Console.AuthToken)
require.Greater(t, len(body), 0)
// Deleting the user should fail, as project exists.
link := fmt.Sprintf("http://"+address.String()+"/api/users/%s", user.Email)
body := assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusConflict,
"",
planet.Satellites[0].Config.Console.AuthToken,
)
require.Greater(t, len(body), 0)

err = planet.Satellites[0].DB.Console().Projects().Delete(ctx, planet.Uplinks[0].Projects[0].ID)
require.NoError(t, err)

err = planet.Satellites[0].DB.Console().Projects().Delete(ctx, planet.Uplinks[0].Projects[0].ID)
require.NoError(t, err)
// Deleting the user should pass, as no project exists for given user.
body = assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusOK,
"",
planet.Satellites[0].Config.Console.AuthToken,
)
require.Len(t, body, 0)

// Deleting non-existing user returns Not Found.
body = assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusNotFound,
"",
planet.Satellites[0].Config.Console.AuthToken,
)
require.Contains(t, string(body), "does not exist")
})
})

// Deleting the user should pass, as no project exists for given user.
body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusOK, "", planet.Satellites[0].Config.Console.AuthToken)
require.Len(t, body, 0)
t.Run("member of foreign projects", func(t *testing.T) {
testplanet.Run(t, testplanet.Config{
SatelliteCount: 1,
StorageNodeCount: 0,
UplinkCount: 1,
Reconfigure: testplanet.Reconfigure{
Satellite: func(_ *zap.Logger, _ int, config *satellite.Config) {
config.Admin.Address = "127.0.0.1:0"
},
},
}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
dbconsole := planet.Satellites[0].DB.Console()
address := planet.Satellites[0].Admin.Admin.Listener.Addr()
user, err := dbconsole.Users().GetByEmail(ctx, planet.Uplinks[0].Projects[0].Owner.Email)
require.NoError(t, err)

// Deleting non-existing user returns Not Found.
body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusNotFound, "", planet.Satellites[0].Config.Console.AuthToken)
require.Contains(t, string(body), "does not exist")
userSharing, err := dbconsole.Users().Insert(ctx, &console.User{
ID: testrand.UUID(),
FullName: "Sharing",
Email: testrand.UUID().String() + "@storj.test",
PasswordHash: testrand.UUID().Bytes(),
})
require.NoError(t, err)

sharedProject, err := dbconsole.Projects().Insert(ctx, &console.Project{
Name: "sharing",
OwnerID: userSharing.ID,
})
require.NoError(t, err)

_, err = dbconsole.ProjectMembers().Insert(ctx, user.ID, sharedProject.ID)
require.NoError(t, err)

err = planet.Satellites[0].DB.Console().Projects().Delete(ctx, planet.Uplinks[0].Projects[0].ID)
require.NoError(t, err)

// Deleting the user should pass, as no project exists for given user.
link := fmt.Sprintf("http://"+address.String()+"/api/users/%s", user.Email)
body := assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusOK,
"",
planet.Satellites[0].Config.Console.AuthToken,
)
require.Len(t, body, 0)
})
})
}

Expand Down

0 comments on commit 9586849

Please sign in to comment.