Skip to content

Commit

Permalink
api, pkg: fix response from create public key without fingerprint
Browse files Browse the repository at this point in the history
  • Loading branch information
@henrybarreto @gustavosbarreto
henrybarreto authored and gustavosbarreto committed Jul 14, 2022
1 parent 0dbd451 commit d042184
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 51 deletions.
7 changes: 5 additions & 2 deletions api/routes/sshkeys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/shellhub-io/shellhub/api/store"
"github.com/shellhub-io/shellhub/pkg/api/paginator"
"github.com/shellhub-io/shellhub/pkg/api/request"
"github.com/shellhub-io/shellhub/pkg/api/response"
"github.com/shellhub-io/shellhub/pkg/models"
)

Expand Down Expand Up @@ -88,16 +89,18 @@ func (h *Handler) CreatePublicKey(c gateway.Context) error {
req.TenantID = tenant
}

var res *response.PublicKeyCreate
err := guard.EvaluatePermission(c.Role(), guard.Actions.PublicKey.Create, func() error {
err := h.service.CreatePublicKey(c.Ctx(), req, tenant)
var err error
res, err = h.service.CreatePublicKey(c.Ctx(), req, tenant)

return err
})
if err != nil {
return err
}

return c.JSON(http.StatusOK, req)
return c.JSON(http.StatusOK, res)
}

func (h *Handler) UpdatePublicKey(c gateway.Context) error {
Expand Down
27 changes: 19 additions & 8 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.

51 changes: 32 additions & 19 deletions api/services/sshkeys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/shellhub-io/shellhub/api/store"
"github.com/shellhub-io/shellhub/pkg/api/paginator"
"github.com/shellhub-io/shellhub/pkg/api/request"
"github.com/shellhub-io/shellhub/pkg/api/response"
"github.com/shellhub-io/shellhub/pkg/clock"
"github.com/shellhub-io/shellhub/pkg/models"
"golang.org/x/crypto/ssh"
Expand All @@ -21,7 +22,7 @@ type SSHKeysService interface {
EvaluateKeyUsername(ctx context.Context, key *models.PublicKey, username string) (bool, error)
ListPublicKeys(ctx context.Context, pagination paginator.Query) ([]models.PublicKey, int, error)
GetPublicKey(ctx context.Context, fingerprint, tenant string) (*models.PublicKey, error)
CreatePublicKey(ctx context.Context, key request.PublicKeyCreate, tenant string) error
CreatePublicKey(ctx context.Context, req request.PublicKeyCreate, tenant string) (*response.PublicKeyCreate, error)
UpdatePublicKey(ctx context.Context, fingerprint, tenant string, key request.PublicKeyUpdate) (*models.PublicKey, error)
DeletePublicKey(ctx context.Context, fingerprint, tenant string) error
CreatePrivateKey(ctx context.Context) (*models.PrivateKey, error)
Expand Down Expand Up @@ -74,54 +75,66 @@ func (s *service) GetPublicKey(ctx context.Context, fingerprint, tenant string)
return s.store.PublicKeyGet(ctx, fingerprint, tenant)
}

func (s *service) CreatePublicKey(ctx context.Context, key request.PublicKeyCreate, tenant string) error {
func (s *service) CreatePublicKey(ctx context.Context, req request.PublicKeyCreate, tenant string) (*response.PublicKeyCreate, error) {
// Checks if public key filter type is Tags.
// If it is, checks if there are, at least, one tag on the public key filter and if the all tags exist on database.
if key.Filter.Tags != nil {
if req.Filter.Tags != nil {
tags, _, err := s.store.TagsGet(ctx, tenant)
if err != nil {
return NewErrTagEmpty(tenant, err)
return nil, NewErrTagEmpty(tenant, err)
}

for _, tag := range key.Filter.Tags {
for _, tag := range req.Filter.Tags {
if !contains(tags, tag) {
return NewErrTagNotFound(tag, nil)
return nil, NewErrTagNotFound(tag, nil)
}
}
}

pubKey, _, _, _, err := ssh.ParseAuthorizedKey(key.Data) //nolint:dogsled
pubKey, _, _, _, err := ssh.ParseAuthorizedKey(req.Data) //nolint:dogsled
if err != nil {
return NewErrPublicKeyDataInvalid(key.Data, nil)
return nil, NewErrPublicKeyDataInvalid(req.Data, nil)
}

key.Fingerprint = ssh.FingerprintLegacyMD5(pubKey)
req.Fingerprint = ssh.FingerprintLegacyMD5(pubKey)

returnedKey, err := s.store.PublicKeyGet(ctx, key.Fingerprint, tenant)
returnedKey, err := s.store.PublicKeyGet(ctx, req.Fingerprint, tenant)
if err != nil && err != store.ErrNoDocuments {
return NewErrPublicKeyNotFound(key.Fingerprint, err)
return nil, NewErrPublicKeyNotFound(req.Fingerprint, err)
}

if returnedKey != nil {
return NewErrPublicKeyDuplicated([]string{key.Fingerprint}, err)
return nil, NewErrPublicKeyDuplicated([]string{req.Fingerprint}, err)
}

model := models.PublicKey{
Data: ssh.MarshalAuthorizedKey(pubKey),
Fingerprint: key.Fingerprint,
Fingerprint: req.Fingerprint,
CreatedAt: clock.Now(),
TenantID: key.TenantID,
TenantID: req.TenantID,
PublicKeyFields: models.PublicKeyFields{
Name: key.Name,
Username: key.Username,
Name: req.Name,
Username: req.Username,
Filter: models.PublicKeyFilter{
Hostname: key.Filter.Hostname,
Tags: key.Filter.Tags,
Hostname: req.Filter.Hostname,
Tags: req.Filter.Tags,
},
},
}

return s.store.PublicKeyCreate(ctx, &model)
err = s.store.PublicKeyCreate(ctx, &model)
if err != nil {
return nil, err
}

return &response.PublicKeyCreate{
Data: model.Data,
Filter: response.PublicKeyFilter(model.Filter),
Name: model.Name,
Username: model.Username,
TenantID: model.TenantID,
Fingerprint: model.Fingerprint,
}, nil
}

func (s *service) ListPublicKeys(ctx context.Context, pagination paginator.Query) ([]models.PublicKey, int, error) {
Expand Down
68 changes: 46 additions & 22 deletions api/services/sshkeys_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/shellhub-io/shellhub/api/store/mocks"
"github.com/shellhub-io/shellhub/pkg/api/paginator"
"github.com/shellhub-io/shellhub/pkg/api/request"
"github.com/shellhub-io/shellhub/pkg/api/response"
"github.com/shellhub-io/shellhub/pkg/clock"
"github.com/shellhub-io/shellhub/pkg/errors"
"github.com/shellhub-io/shellhub/pkg/models"
Expand Down Expand Up @@ -617,107 +618,130 @@ func TestCreatePublicKeys(t *testing.T) {
},
}

resWithHostnameModel := &response.PublicKeyCreate{
Data: keyWithHostnameModel.Data,
Filter: response.PublicKeyFilter(keyWithHostnameModel.Filter),
Name: keyWithHostnameModel.Name,
Username: keyWithHostnameModel.Username,
TenantID: keyWithHostnameModel.TenantID,
Fingerprint: keyWithHostnameModel.Fingerprint,
}

resWithTagsModel := &response.PublicKeyCreate{
Data: keyWithTagsModel.Data,
Filter: response.PublicKeyFilter(keyWithTagsModel.Filter),
Name: keyWithTagsModel.Name,
Username: keyWithTagsModel.Username,
TenantID: keyWithTagsModel.TenantID,
Fingerprint: keyWithTagsModel.Fingerprint,
}

type Expected struct {
res *response.PublicKeyCreate
err error
}

cases := []struct {
description string
tenantID string
key request.PublicKeyCreate
req request.PublicKeyCreate
requiredMocks func()
expected error
expected Expected
}{
{
description: "fail to create the key when filter tags is empty",
tenantID: "tenant",
key: keyInvalidEmptyTags,
req: keyInvalidEmptyTags,
requiredMocks: func() {
mock.On("TagsGet", ctx, "tenant").Return([]string{}, 0, err).Once()
},
expected: NewErrTagEmpty("tenant", err),
expected: Expected{nil, NewErrTagEmpty("tenant", err)},
},
{
description: "fail to create the key when a tags does not exist in a device",
tenantID: "tenant",
key: keyInvalidNotFound,
req: keyInvalidNotFound,
requiredMocks: func() {
mock.On("TagsGet", ctx, "tenant").Return([]string{"tag1", "tag4"}, 2, nil).Once()
},
expected: NewErrTagNotFound("tag2", nil),
expected: Expected{nil, NewErrTagNotFound("tag2", nil)},
},
{
description: "fail when data in public key is not valid",
tenantID: "tenant",
key: keyInvalidData,
req: keyInvalidData,
requiredMocks: func() {
},
expected: NewErrPublicKeyDataInvalid(keyInvalidData.Data, nil),
expected: Expected{nil, NewErrPublicKeyDataInvalid(keyInvalidData.Data, nil)},
},
{
description: "fail when cannot get the public key",
tenantID: "tenant",
key: keyWithHostname,
req: keyWithHostname,
requiredMocks: func() {
mock.On("PublicKeyGet", ctx, keyWithHostname.Fingerprint, "tenant").Return(nil, err).Once()
},
expected: NewErrPublicKeyNotFound(keyWithHostname.Fingerprint, err),
expected: Expected{nil, NewErrPublicKeyNotFound(keyWithHostname.Fingerprint, err)},
},
{
description: "fail when public key is duplicated",
tenantID: "tenant",
key: keyWithHostname,
req: keyWithHostname,
requiredMocks: func() {
mock.On("PublicKeyGet", ctx, keyWithHostname.Fingerprint, "tenant").Return(&keyWithHostnameModel, nil).Once()
},
expected: NewErrPublicKeyDuplicated([]string{keyWithHostname.Fingerprint}, nil),
expected: Expected{nil, NewErrPublicKeyDuplicated([]string{keyWithHostname.Fingerprint}, nil)},
},
{
description: "fail to create a public key when filter is hostname",
tenantID: "tenant",
key: keyWithHostname,
req: keyWithHostname,
requiredMocks: func() {
mock.On("PublicKeyGet", ctx, keyWithHostname.Fingerprint, "tenant").Return(nil, nil).Once()
mock.On("PublicKeyCreate", ctx, &keyWithHostnameModel).Return(err).Once()
},
expected: err,
expected: Expected{nil, err},
},
{
description: "success to create a public key when filter is hostname",
tenantID: "tenant",
key: keyWithHostname,
req: keyWithHostname,
requiredMocks: func() {
mock.On("PublicKeyGet", ctx, keyWithHostname.Fingerprint, "tenant").Return(nil, nil).Once()
mock.On("PublicKeyCreate", ctx, &keyWithHostnameModel).Return(nil).Once()
},
expected: nil,
expected: Expected{resWithHostnameModel, nil},
},
{
description: "fail to create a public key when filter is tags",
tenantID: "tenant",
key: keyWithTags,
req: keyWithTags,
requiredMocks: func() {
mock.On("TagsGet", ctx, keyWithTags.TenantID).Return([]string{"tag1", "tag2"}, 2, nil).Once()
mock.On("PublicKeyGet", ctx, keyWithTags.Fingerprint, "tenant").Return(nil, nil).Once()
mock.On("PublicKeyCreate", ctx, &keyWithTagsModel).Return(err).Once()
},
expected: err,
expected: Expected{nil, err},
},
{
description: "success to create a public key when filter is tags",
tenantID: "tenant",
key: keyWithTags,
req: keyWithTags,
requiredMocks: func() {
mock.On("TagsGet", ctx, keyWithTags.TenantID).Return([]string{"tag1", "tag2"}, 2, nil).Once()
mock.On("PublicKeyGet", ctx, keyWithTags.Fingerprint, "tenant").Return(nil, nil).Once()
mock.On("PublicKeyCreate", ctx, &keyWithTagsModel).Return(nil).Once()
},
expected: nil,
expected: Expected{resWithTagsModel, nil},
},
}

for _, tc := range cases {
t.Run(tc.description, func(t *testing.T) {
tc.requiredMocks()

err := s.CreatePublicKey(ctx, tc.key, tc.tenantID)
assert.Equal(t, tc.expected, err)
res, err := s.CreatePublicKey(ctx, tc.req, tc.tenantID)
assert.Equal(t, tc.expected, Expected{res, err})
})
}

Expand Down
20 changes: 20 additions & 0 deletions pkg/api/response/publickey.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package response

type PublicKeyFilter struct {
Hostname string `json:"hostname,omitempty" validate:"required_without=Tags,excluded_with=Tags,regexp"`
// FIXME: add validation for tags when it has at least one item.
//
// If used `min=1` to do that validation, when tags is empty, its zero value, and only hostname is provided,
// it throws a error even with `required_without` and `excluded_with`.
Tags []string `json:"tags,omitempty" validate:"required_without=Hostname,excluded_with=Hostname,max=3,unique,dive,min=3,max=255,alphanum,ascii,excludes=/@&:"`
}

// PublicKeyCreate is the structure to represent the request data for create public key endpoint.
type PublicKeyCreate struct {
Data []byte `json:"data"`
Filter PublicKeyFilter `json:"filter"`
Name string `json:"name"`
Username string `json:"username"`
TenantID string `json:"tenant_id"`
Fingerprint string `json:"fingerprint"`
}

0 comments on commit d042184

Please sign in to comment.