Skip to content

Commit e83d7fc

Browse files
authored
Userlist fixuserupdate (#1301)
* published is a better choice than created for releases. * Fix: Non-admin user updates were creating users erroneously
1 parent dcbaee6 commit e83d7fc

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

.github/workflows/build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: build
22

33
on:
44
release:
5-
types: [created]
5+
types: [published]
66
push:
77
branches:
88
- main

app/services/userlist/userlist.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,21 @@ func addOrRemoveUserListUser(ctx context.Context, u *cmd.UserListHandleRoleChang
112112

113113
func updateUserListUser(ctx context.Context, u *cmd.UserListUpdateUser) error {
114114

115+
dbUser := &query.GetUserByID{
116+
UserID: u.Id,
117+
}
118+
err := bus.Dispatch(ctx, dbUser)
119+
if err != nil {
120+
return err
121+
}
122+
123+
if dbUser.Result.Role != enum.RoleAdministrator {
124+
return nil
125+
}
126+
115127
user := &UserListUser{
116128
Identifier: strconv.Itoa(u.Id),
117-
Company: strconv.Itoa(u.TenantId),
129+
Company: strconv.Itoa(dbUser.Result.Tenant.ID),
118130
}
119131

120132
if len(u.Email) > 0 {
@@ -127,7 +139,7 @@ func updateUserListUser(ctx context.Context, u *cmd.UserListUpdateUser) error {
127139
}
128140
}
129141

130-
err := pushUserListUpdate(user, ctx)
142+
err = pushUserListUpdate(user, ctx)
131143
if err != nil {
132144
return err
133145
}

app/services/userlist/userlist_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/getfider/fider/app/models/cmd"
1212
"github.com/getfider/fider/app/models/entity"
1313
"github.com/getfider/fider/app/models/enum"
14+
"github.com/getfider/fider/app/models/query"
1415
"github.com/getfider/fider/app/pkg/bus"
1516
"github.com/getfider/fider/app/pkg/env"
1617
"github.com/getfider/fider/app/services/httpclient/httpclientmock"
@@ -202,6 +203,33 @@ func TestUpdateUser_EmailOnly(t *testing.T) {
202203
Expect(containsEmail).IsTrue()
203204
}
204205

206+
func TestDoNothingIfUserNotAdmin(t *testing.T) {
207+
RegisterT(t)
208+
env.Config.HostMode = "multi"
209+
reset()
210+
211+
// Change the DB hit to return a visitor
212+
bus.AddHandler(func(ctx context.Context, q *query.GetUserByID) error {
213+
q.Result = &entity.User{
214+
ID: 1,
215+
Name: "John Doe",
216+
Email: "john.doe@example.com",
217+
Role: enum.RoleVisitor,
218+
}
219+
return nil
220+
})
221+
222+
err := bus.Dispatch(ctx, &cmd.UserListUpdateUser{
223+
Id: 1,
224+
TenantId: 1,
225+
Email: "Freddy@example.com",
226+
})
227+
Expect(err).IsNil()
228+
229+
Expect(httpclientmock.RequestsHistory).HasLen(0)
230+
231+
}
232+
205233
func TestMakeUserAdministrator(t *testing.T) {
206234
RegisterT(t)
207235
env.Config.HostMode = "multi"

0 commit comments

Comments
 (0)