Diff
1diff --git a/pkg/backend/repo.go b/pkg/backend/repo.go
2index 98be7a8a77315b0d3da08a251270d5ed520cd108..0d2a5b546dbc0c059a495b3fd54853af174943bb 100644
3--- a/pkg/backend/repo.go
4+++ b/pkg/backend/repo.go
5@@ -293,16 +293,19 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error {
6 name = utils.SanitizeRepo(name)
7 rp := filepath.Join(d.repoPath(name))
8
9- user := proto.UserFromContext(ctx)
10+ // Build the webhook event before deleting so it can be sent after. A
11+ // repository whose directory is gone has no event, but its database row
12+ // still has to go.
13+ var wh *webhook.RepositoryEvent
14 r, err := d.Repository(ctx, name)
15- if err != nil {
16- return err
17- }
18-
19- // We create the webhook event before deleting the repository so we can
20- // send the event after deleting the repository.
21- wh, err := webhook.NewRepositoryEvent(ctx, user, r, webhook.RepositoryEventActionDelete)
22- if err != nil {
23+ switch {
24+ case err == nil:
25+ ev, err := webhook.NewRepositoryEvent(ctx, proto.UserFromContext(ctx), r, webhook.RepositoryEventActionDelete)
26+ if err != nil {
27+ return err
28+ }
29+ wh = &ev
30+ case !errors.Is(err, proto.ErrRepoNotFound):
31 return err
32 }
33
34@@ -352,34 +355,11 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error {
35 return db.WrapError(err)
36 }
37
38- return webhook.SendEvent(ctx, wh)
39-}
40-
41-// DeleteUserRepositories deletes all user repositories.
42-func (d *Backend) DeleteUserRepositories(ctx context.Context, username string) error {
43- if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error {
44- user, err := d.store.FindUserByUsername(ctx, tx, username)
45- if err != nil {
46- return err
47- }
48-
49- repos, err := d.store.GetUserRepos(ctx, tx, user.ID)
50- if err != nil {
51- return err
52- }
53-
54- for _, repo := range repos {
55- if err := d.DeleteRepository(ctx, repo.Name); err != nil {
56- return err
57- }
58- }
59-
60+ if wh == nil {
61 return nil
62- }); err != nil {
63- return db.WrapError(err)
64 }
65
66- return nil
67+ return webhook.SendEvent(ctx, *wh)
68 }
69
70 // RenameRepository renames a repository.
71diff --git a/pkg/backend/user.go b/pkg/backend/user.go
72index 11e24058eb61c2597189bd3757ec0d284ce03536..d2945062efd5e9d1bc006b84e75e570285250e74 100644
73--- a/pkg/backend/user.go
74+++ b/pkg/backend/user.go
75@@ -3,6 +3,7 @@ package backend
76 import (
77 "context"
78 "errors"
79+ "fmt"
80 "strings"
81 "time"
82
83@@ -301,13 +302,33 @@ func (d *Backend) DeleteUser(ctx context.Context, username string) error {
84 return err
85 }
86
87- return d.db.TransactionContext(ctx, func(tx *db.Tx) error {
88- if err := d.store.DeleteUserByUsername(ctx, tx, username); err != nil {
89- return db.WrapError(err)
90+ err := d.db.TransactionContext(ctx, func(tx *db.Tx) error {
91+ user, err := d.store.FindUserByUsername(ctx, tx, username)
92+ if err != nil {
93+ return err
94+ }
95+
96+ // Deleting the user would cascade to their repositories' rows and
97+ // strand the repositories on disk, so their owner must deal with
98+ // them first.
99+ repos, err := d.store.GetUserRepos(ctx, tx, user.ID)
100+ if err != nil {
101+ return err
102+ }
103+ if len(repos) > 0 {
104+ names := make([]string, len(repos))
105+ for i, r := range repos {
106+ names[i] = r.Name
107+ }
108+ return fmt.Errorf("%w: %s", proto.ErrUserOwnsRepos, strings.Join(names, ", "))
109 }
110
111- return d.DeleteUserRepositories(ctx, username)
112+ return d.store.DeleteUserByUsername(ctx, tx, username)
113 })
114+ if errors.Is(err, db.ErrRecordNotFound) {
115+ return proto.ErrUserNotFound
116+ }
117+ return db.WrapError(err)
118 }
119
120 // RemovePublicKey removes a public key from a user.
121diff --git a/pkg/backend/user_test.go b/pkg/backend/user_test.go
122index 7140725d1e06fc8f5ff2193ccfcd251e97d71f38..aa93841f4730b6acea7cf3d216a450fb592735f3 100644
123--- a/pkg/backend/user_test.go
124+++ b/pkg/backend/user_test.go
125@@ -2,11 +2,15 @@ package backend
126
127 import (
128 "context"
129+ "errors"
130+ "os"
131 "testing"
132
133 "github.com/charmbracelet/soft-serve/pkg/access"
134+ "github.com/charmbracelet/soft-serve/pkg/config"
135 "github.com/charmbracelet/soft-serve/pkg/db"
136 "github.com/charmbracelet/soft-serve/pkg/proto"
137+ "github.com/charmbracelet/soft-serve/pkg/store"
138 "github.com/matryer/is"
139 )
140
141@@ -67,3 +71,58 @@ func TestAccessLevelPrivateRepoNonCollaborator(t *testing.T) {
142 is.Equal(be.AccessLevelForUser(ctx, "repo", user), access.NoAccess)
143 is.Equal(be.AccessLevelForUser(ctx, "repo", nil), access.NoAccess)
144 }
145+
146+// TestDeleteUserWithRepositories verifies that a user who owns
147+// repositories is not deleted until those repositories are gone, and that
148+// their repositories are left untouched by the refusal.
149+func TestDeleteUserWithRepositories(t *testing.T) {
150+ is := is.New(t)
151+ be, cfg := newTestBackend(t)
152+
153+ ctx := context.Background()
154+ ctx = config.WithContext(ctx, cfg)
155+ ctx = db.WithContext(ctx, be.db)
156+ ctx = store.WithContext(ctx, be.store)
157+ admin, err := be.User(ctx, "admin")
158+ is.NoErr(err)
159+ ctx = proto.WithUserContext(ctx, admin)
160+
161+ owner, err := be.CreateUser(ctx, "owner", proto.UserOptions{})
162+ is.NoErr(err)
163+ _, err = be.CreateRepository(ctx, "owned", owner, proto.RepositoryOptions{})
164+ is.NoErr(err)
165+
166+ err = be.DeleteUser(ctx, "owner")
167+ is.True(errors.Is(err, proto.ErrUserOwnsRepos))
168+ _, err = be.User(ctx, "owner")
169+ is.NoErr(err)
170+ _, err = be.Repository(ctx, "owned")
171+ is.NoErr(err)
172+
173+ is.NoErr(be.DeleteRepository(ctx, "owned"))
174+ is.NoErr(be.DeleteUser(ctx, "owner"))
175+ _, err = be.User(ctx, "owner")
176+ is.True(errors.Is(err, proto.ErrUserNotFound))
177+
178+ is.True(errors.Is(be.DeleteUser(ctx, "owner"), proto.ErrUserNotFound))
179+}
180+
181+// TestDeleteRepositoryWithoutDirectory verifies that a repository whose
182+// directory is gone can still be deleted, and that deleting a repository
183+// that does not exist at all reports it as not found.
184+func TestDeleteRepositoryWithoutDirectory(t *testing.T) {
185+ is := is.New(t)
186+ be, cfg := newTestBackend(t)
187+
188+ ctx := context.Background()
189+ ctx = config.WithContext(ctx, cfg)
190+ ctx = db.WithContext(ctx, be.db)
191+ ctx = store.WithContext(ctx, be.store)
192+
193+ _, err := be.CreateRepository(ctx, "orphan", nil, proto.RepositoryOptions{})
194+ is.NoErr(err)
195+ is.NoErr(os.RemoveAll(be.repoPath("orphan")))
196+
197+ is.NoErr(be.DeleteRepository(ctx, "orphan"))
198+ is.True(errors.Is(be.DeleteRepository(ctx, "orphan"), proto.ErrRepoNotFound))
199+}
200diff --git a/pkg/proto/errors.go b/pkg/proto/errors.go
201index ef126a3546aeb226b292d2039a3ab4b0af7001a6..948b4bc110c441862e3df2fc26e7192f4ab380b9 100644
202--- a/pkg/proto/errors.go
203+++ b/pkg/proto/errors.go
204@@ -17,6 +17,9 @@ var (
205 ErrRepoExist = errors.New("repository already exists")
206 // ErrUserNotFound is returned when a user is not found.
207 ErrUserNotFound = errors.New("user not found")
208+ // ErrUserOwnsRepos is returned when deleting a user who still owns
209+ // repositories.
210+ ErrUserOwnsRepos = errors.New("user still owns repositories; delete them first")
211 // ErrTokenNotFound is returned when a token is not found.
212 ErrTokenNotFound = errors.New("token not found")
213 // ErrTokenExpired is returned when a token is expired.
214diff --git a/testscript/testdata/user-delete-owner.txtar b/testscript/testdata/user-delete-owner.txtar
215new file mode 100644
216index 0000000000000000000000000000000000000000..b3b8d586ea31cf46d92a574d4e9a122074b3f605
217--- /dev/null
218+++ b/testscript/testdata/user-delete-owner.txtar
219@@ -0,0 +1,24 @@
220+# vi: set ft=conf
221+
222+# start soft serve
223+exec soft serve &
224+# wait for SSH server to start
225+ensureserverrunning SSH_PORT
226+
227+soft user create foo --key "$USER1_AUTHORIZED_KEY"
228+usoft repo create foo-repo
229+
230+# a user who owns repositories cannot be removed until they are gone
231+! soft user delete foo
232+stderr '.*still owns repositories.*foo-repo.*'
233+soft repo list
234+stdout 'foo-repo'
235+
236+soft repo delete foo-repo
237+soft user delete foo
238+! soft user info foo
239+stderr '.*not found.*'
240+
241+# stop the server
242+[windows] stopserver
243+[windows] ! stderr .