Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@heiytor
heiytor committed May 27, 2024
1 parent dd9c65f commit e377369
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 94 deletions.
30 changes: 15 additions & 15 deletions api/routes/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,23 @@ func (h *Handler) UpdateUserData(c gateway.Context) error {
return err
}

if fields, err := h.service.UpdateDataUser(c.Ctx(), req); err != nil {
// FIXME: API compatibility.
//
// The UI uses the fields with error messages to identify if it is invalid or duplicated.
var e errors.Error
if ok := errors.As(err, &e); !ok {
return err
}
// TODO: remove the user id from the params and use only the authenticated header.
if c.Param("id") != c.ID().ID {
return services.NewErrForbidden(nil, nil)
}

switch e.Code {
case services.ErrCodeInvalid:
return c.JSON(http.StatusBadRequest, fields)
case services.ErrCodeDuplicated:
return c.JSON(http.StatusConflict, fields)
default:
return err
fields, err := h.service.UpdateDataUser(c.Ctx(), c.ID().ID, req)
if err != nil {
e := new(errors.Error)
if ok := errors.As(err, &e); ok && e.Code == services.ErrCodeInvalid {
return c.JSON(http.StatusBadRequest, []string{})
}

return err
}

if len(fields) > 0 {
return c.JSON(http.StatusConflict, fields)
}

return c.NoContent(http.StatusOK)
Expand Down
186 changes: 132 additions & 54 deletions api/routes/user_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,92 +12,170 @@ import (
svc "github.com/shellhub-io/shellhub/api/services"
"github.com/shellhub-io/shellhub/api/services/mocks"
"github.com/shellhub-io/shellhub/pkg/api/requests"
"github.com/shellhub-io/shellhub/pkg/models"
"github.com/stretchr/testify/assert"
gomock "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func TestUpdateUserData(t *testing.T) {
mock := new(mocks.Service)
type Expected struct {
status int
}

svcMock := new(mocks.Service)

cases := []struct {
title string
uid string
updatePayloadMock requests.UserDataUpdate
requiredMocks func(updatePayloadMock models.UserData)
expectedStatus int
description string
headers map[string]string
body requests.UserDataUpdate
requiredMocks func()
expected Expected
}{
{
title: "fails when bind fails to validate uid",
uid: "1234",
updatePayloadMock: requests.UserDataUpdate{
Name: "new name",
Username: "usernameteste",
Email: "newemail@example.com",
description: "fails when bind fails to validate e-mail",
headers: map[string]string{
"X-ID": "000000000000000000000000",
"X-Role": "owner",
},
requiredMocks: func(updatePayloadMock models.UserData) {},
expectedStatus: http.StatusBadRequest,
body: requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "invalid.com",
RecoveryEmail: "invalid.com",
},
requiredMocks: func() {},
expected: Expected{http.StatusBadRequest},
},
{
title: "fails when try to updating a non-existing user",
uid: "1234",
updatePayloadMock: requests.UserDataUpdate{
UserParam: requests.UserParam{
ID: "1234",
},
Name: "new name",
Username: "usernameteste",
Email: "newemail@example.com",
description: "fails when bind fails to validate username",
headers: map[string]string{
"X-ID": "000000000000000000000000",
"X-Role": "owner",
},
requiredMocks: func(updatePayloadMock models.UserData) {
mock.On("UpdateDataUser", gomock.Anything, "1234", updatePayloadMock).Return(nil, svc.ErrUserNotFound)
body: requests.UserDataUpdate{
Name: "John Doe",
Username: "_",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
expectedStatus: http.StatusNotFound,
requiredMocks: func() {},
expected: Expected{http.StatusBadRequest},
},
{
title: "success when try to updating an existing user",
uid: "123",
updatePayloadMock: requests.UserDataUpdate{
UserParam: requests.UserParam{
ID: "123",
},
Name: "new name",
Username: "usernameteste",
Email: "newemail@example.com",
description: "fails when try to updating a non-existing user",
headers: map[string]string{
"X-ID": "000000000000000000000000",
"X-Role": "owner",
},
requiredMocks: func(updatePayloadMock models.UserData) {
mock.On("UpdateDataUser", gomock.Anything, "123", updatePayloadMock).Return(nil, nil)
body: requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
expectedStatus: http.StatusOK,
requiredMocks: func() {
svcMock.
On(
"UpdateDataUser",
gomock.Anything,
"000000000000000000000000",
&requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
).
Return(nil, svc.ErrUserNotFound).
Once()
},
expected: Expected{http.StatusNotFound},
},
{
description: "fails when try to updating with a conflict",
headers: map[string]string{
"X-ID": "000000000000000000000000",
"X-Role": "owner",
},
body: requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
requiredMocks: func() {
svcMock.
On(
"UpdateDataUser",
gomock.Anything,
"000000000000000000000000",
&requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
).
Return([]string{"email"}, nil).
Once()
},
expected: Expected{http.StatusConflict},
},
{
description: "success when try to updating an existing user",
body: requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
headers: map[string]string{
"X-ID": "000000000000000000000000",
"X-Role": "owner",
},
requiredMocks: func() {
svcMock.
On(
"UpdateDataUser",
gomock.Anything,
"000000000000000000000000",
&requests.UserDataUpdate{
Name: "John Doe",
Username: "john_doe",
Email: "john.doe@test.com",
RecoveryEmail: "john.doe@test.com",
},
).
Return(nil, nil).
Once()
},
expected: Expected{http.StatusOK},
},
}

for _, tc := range cases {
t.Run(tc.title, func(t *testing.T) {
tc.requiredMocks(models.UserData{
Name: tc.updatePayloadMock.Name,
Username: tc.updatePayloadMock.Username,
Email: tc.updatePayloadMock.Email,
})
t.Run(tc.description, func(t *testing.T) {
tc.requiredMocks()

jsonData, err := json.Marshal(tc.updatePayloadMock)
if err != nil {
assert.NoError(t, err)
}
data, err := json.Marshal(tc.body)
require.NoError(t, err)

req := httptest.NewRequest(http.MethodPatch, fmt.Sprintf("/api/users/%s/data", tc.uid), strings.NewReader(string(jsonData)))
req := httptest.NewRequest(http.MethodPatch, fmt.Sprintf("/api/users/%s/data", tc.headers["X-ID"]), strings.NewReader(string(data)))
req.Header.Set("Content-Type", "application/json")
req.Header.Set("X-Role", guard.RoleOwner)
for k, v := range tc.headers {
req.Header.Set(k, v)
}

rec := httptest.NewRecorder()

e := NewRouter(mock)
e := NewRouter(svcMock)
e.ServeHTTP(rec, req)

assert.Equal(t, tc.expectedStatus, rec.Result().StatusCode)
assert.Equal(t, tc.expected, Expected{rec.Result().StatusCode})
})
}

mock.AssertExpectations(t)
svcMock.AssertExpectations(t)
}

func TestUpdateUserPassword(t *testing.T) {
Expand Down
20 changes: 10 additions & 10 deletions api/services/mocks/services.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions api/services/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ type UserService interface {
// UpdateDataUser updates the user's data, such as email and username. Since some attributes must be unique per user,
// it returns a list of duplicated unique values and an error if any.
// TODO: rename this function to UpdateUserData.
UpdateDataUser(ctx context.Context, req *requests.UserDataUpdate) (conflicts []string, err error)
UpdateDataUser(ctx context.Context, userID string, req *requests.UserDataUpdate) (conflicts []string, err error)

UpdatePasswordUser(ctx context.Context, id string, currentPassword, newPassword string) error
}

func (s *service) UpdateDataUser(ctx context.Context, req *requests.UserDataUpdate) ([]string, error) {
user, _, err := s.store.UserGetByID(ctx, req.UserID, false)
func (s *service) UpdateDataUser(ctx context.Context, userID string, req *requests.UserDataUpdate) ([]string, error) {
user, _, err := s.store.UserGetByID(ctx, userID, false)
if err != nil {
return nil, NewErrUserNotFound(req.UserID, nil)
return nil, NewErrUserNotFound(userID, nil)
}

if req.RecoveryEmail == user.Email || req.RecoveryEmail == req.Email {
Expand All @@ -39,7 +39,7 @@ func (s *service) UpdateDataUser(ctx context.Context, req *requests.UserDataUpda
RecoveryEmail: strings.ToLower(req.RecoveryEmail),
}

return nil, s.store.UserUpdate(ctx, req.UserID, changes)
return nil, s.store.UserUpdate(ctx, userID, changes)
}

func (s *service) UpdatePasswordUser(ctx context.Context, id, currentPassword, newPassword string) error {
Expand Down
Loading

0 comments on commit e377369

Please sign in to comment.