Skip to content

feat(SPV-1583) errorx & RFC 7807 problem details for admin endpoints #953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
18 changes: 18 additions & 0 deletions actions/testabilities/assert_spvwallet_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type SPVWalletResponseAssertions interface {
HasStatus(status int) SPVWalletResponseAssertions
WithJSONf(expectedFormat string, args ...any)
WithJSONMatching(expectedTemplateFormat string, params map[string]any)
WithProblemDetails(status int, errType string, containDetails ...string)
JSONValue() JsonValueGetter
// IsUnauthorized asserts that the response status code is 401 and the error is about lack of authorization.
IsUnauthorized()
Expand Down Expand Up @@ -157,6 +158,23 @@ func (a *responseAssertions) JSONValue() JsonValueGetter {
return jsonrequire.NewGetterWithJSON(a.t, a.response.String())
}

func (a *responseAssertions) WithProblemDetails(status int, errType string, containDetails ...string) {
a.t.Helper()
a.assert.Equal(status, a.response.StatusCode())

a.WithJSONMatching(`{
"detail": "{{ containsAll .detailsParts }}",
"instance": {{ anything }},
"status": {{ .status }},
"title": {{ anything }},
"type": "{{ .type }}"
}`, map[string]any{
"status": status,
"type": errType,
"detailsParts": containDetails,
})
}

func (a *responseAssertions) assertJSONContentType() {
a.t.Helper()
contentType := a.response.Header().Get("Content-Type")
Expand Down
24 changes: 0 additions & 24 deletions actions/v2/admin/errors/errors.go

This file was deleted.

12 changes: 9 additions & 3 deletions actions/v2/admin/internal/mapping/paymail.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package mapping

import (
"github.com/bitcoin-sv/go-paymail"
adminerrors "github.com/bitcoin-sv/spv-wallet/actions/v2/admin/errors"
"github.com/bitcoin-sv/spv-wallet/api"
"github.com/bitcoin-sv/spv-wallet/engine/v2/paymails/paymailsmodels"
"github.com/bitcoin-sv/spv-wallet/errdef/clienterr"
"github.com/bitcoin-sv/spv-wallet/lox"
)

Expand Down Expand Up @@ -84,14 +84,20 @@ func parsePaymail(r *api.RequestsAddPaymail) (string, string, error) {
if request.HasAddress() &&
(request.HasAlias() || request.HasDomain()) &&
!request.AddressEqualsTo(request.Alias+"@"+request.Domain) {
return "", "", adminerrors.ErrPaymailInconsistent
return "", "", clienterr.BadRequest.
Detailed(
"inconsistent_alias_domain_and_address",
"Inconsistent alias@domain and address fields: %s, %s, %s. Hint: use either alias and domain or address (not both)",
request.Alias, request.Domain, request.Address,
).Err()
}
if !request.HasAddress() {
request.Address = request.Alias + "@" + request.Domain
}
alias, domain, sanitized := paymail.SanitizePaymail(request.Address)
if sanitized == "" {
return "", "", adminerrors.ErrInvalidPaymail
return "", "", clienterr.BadRequest.
Detailed("invalid_paymail_address", "Invalid paymail address: %s", request.Address).Err()
}
return alias, domain, nil
}
30 changes: 20 additions & 10 deletions actions/v2/admin/users/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,48 @@ import (
"net/http"

primitives "github.com/bitcoin-sv/go-sdk/primitives/ec"
adminerrors "github.com/bitcoin-sv/spv-wallet/actions/v2/admin/errors"
"github.com/bitcoin-sv/spv-wallet/actions/v2/admin/internal/mapping"
"github.com/bitcoin-sv/spv-wallet/api"
configerrors "github.com/bitcoin-sv/spv-wallet/config/errors"
"github.com/bitcoin-sv/spv-wallet/engine/spverrors"
"github.com/bitcoin-sv/spv-wallet/engine/v2/paymails/paymailerrors"
"github.com/bitcoin-sv/spv-wallet/errdef/clienterr"
"github.com/gin-gonic/gin"
)

// CreateUser creates a new user
func (s *APIAdminUsers) CreateUser(c *gin.Context) {
var request api.RequestsCreateUser

if err := c.Bind(&request); err != nil {
spverrors.ErrorResponse(c, spverrors.ErrCannotBindRequest.Wrap(err), s.logger)
// TODO: Bind does AbortWithError internally, so we should not call Response, I guess
clienterr.UnprocessableEntity.New().Wrap(err).Response(c, s.logger)
return
}

if err := validatePubKey(request.PublicKey); err != nil {
spverrors.ErrorResponse(c, err, s.logger)
clienterr.Response(c, err, s.logger)
return
}

newUser, err := mapping.RequestCreateUserToNewUserModel(&request)
if err != nil {
spverrors.ErrorResponse(c, err, s.logger)
clienterr.Response(c, err, s.logger)
return
}

createdUser, err := s.engine.UsersService().Create(c, newUser)

if err != nil {
spverrors.MapResponse(c, err, s.logger).
If(configerrors.ErrUnsupportedDomain).Then(adminerrors.ErrInvalidDomain).
If(paymailerrors.ErrInvalidAvatarURL).Then(adminerrors.ErrInvalidAvatarURL).
Else(adminerrors.ErrCreatingUser)
clienterr.Map(err).
IfOfType(configerrors.UnsupportedDomain).
Then(
clienterr.BadRequest.Detailed("unsupported_domain", "Unsupported domain: '%s'", newUser.Paymail.Domain),
).
IfOfType(paymailerrors.InvalidAvatarURL).
Then(
clienterr.UnprocessableEntity.Detailed("invalid_avatar_url", "Invalid avatar URL: '%s'", newUser.Paymail.Avatar),
).
Response(c, s.logger)
return
}

Expand All @@ -47,7 +55,9 @@ func (s *APIAdminUsers) CreateUser(c *gin.Context) {
func validatePubKey(pubKey string) error {
_, err := primitives.PublicKeyFromString(pubKey)
if err != nil {
return adminerrors.ErrInvalidPublicKey.Wrap(err)
return clienterr.BadRequest.
Detailed("invalid_public_key", "Invalid public key: '%s'", pubKey).
Wrap(err).Err()
}
return nil
}
32 changes: 25 additions & 7 deletions actions/v2/admin/users/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

"github.com/bitcoin-sv/spv-wallet/actions/testabilities"
"github.com/bitcoin-sv/spv-wallet/actions/testabilities/apierror"
testengine "github.com/bitcoin-sv/spv-wallet/engine/testabilities"
"github.com/bitcoin-sv/spv-wallet/engine/tester/fixtures"
)
Expand Down Expand Up @@ -131,8 +130,7 @@ func TestCreateUserWithBadURLAvatar(t *testing.T) {

// then:
then.Response(res).
HasStatus(422).
WithJSONf(apierror.ExpectedJSON("error-user-invalid-avatar-url", "invalid avatar url"))
WithProblemDetails(422, "invalid_avatar_url", "Invalid avatar URL")

}

Expand Down Expand Up @@ -441,8 +439,7 @@ func TestAddUserWithWrongPaymailDomain(t *testing.T) {

// then:
then.Response(res).
HasStatus(400).
WithJSONf(apierror.ExpectedJSON("error-invalid-domain", "invalid domain"))
WithProblemDetails(400, "unsupported_domain", "Unsupported domain")
})

t.Run("Try to add using alias and domain as address", func(t *testing.T) {
Expand All @@ -461,8 +458,29 @@ func TestAddUserWithWrongPaymailDomain(t *testing.T) {

// then:
then.Response(res).
HasStatus(400).
WithJSONf(apierror.ExpectedJSON("error-invalid-domain", "invalid domain"))
WithProblemDetails(400, "unsupported_domain", "Unsupported domain")
})
}

func TestTryToAddWithWrongPubKey(t *testing.T) {
// given:
given, then := testabilities.New(t)
cleanup := given.StartedSPVWalletWithConfiguration(
testengine.WithV2(),
)
defer cleanup()

// and:
client := given.HttpClient().ForAdmin()

// when:
res, _ := client.R().
SetBody(map[string]any{
"publicKey": "wrong",
}).
Post("/api/v2/admin/users")

// then:
then.Response(res).
WithProblemDetails(400, "invalid_public_key", "Invalid public key")
}
7 changes: 5 additions & 2 deletions actions/v2/admin/users/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ import (
"net/http"

"github.com/bitcoin-sv/spv-wallet/actions/v2/admin/internal/mapping"
"github.com/bitcoin-sv/spv-wallet/engine/spverrors"
dberrors "github.com/bitcoin-sv/spv-wallet/engine/v2/database/errors"
"github.com/bitcoin-sv/spv-wallet/errdef/clienterr"
"github.com/gin-gonic/gin"
)

// UserById returns a user by ID
func (s *APIAdminUsers) UserById(c *gin.Context, id string) {
user, err := s.engine.UsersService().GetByID(c, id)
if err != nil {
spverrors.ErrorResponse(c, err, s.logger)
clienterr.Map(err).
IfOfType(dberrors.NotFound).Then(clienterr.NotFound.New()).
Response(c, s.logger)
return
}

Expand Down
72 changes: 72 additions & 0 deletions actions/v2/admin/users/get_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package users_test

import (
"testing"

"github.com/bitcoin-sv/spv-wallet/actions/testabilities"
testengine "github.com/bitcoin-sv/spv-wallet/engine/testabilities"
"github.com/bitcoin-sv/spv-wallet/engine/tester/fixtures"
)

func TestGetUser(t *testing.T) {
// given:
given, then := testabilities.New(t)
cleanup := given.StartedSPVWalletWithConfiguration(testengine.WithV2())
defer cleanup()

// and:
client := given.HttpClient().ForAdmin()

// when:
resp, _ := client.
R().
SetPathParam("id", fixtures.Sender.ID()).
Get("/api/v2/admin/users/{id}")

// then:
then.Response(resp).
HasStatus(200).
WithJSONMatching(`{
"id": "{{ .id }}",
"createdAt": "{{ matchTimestamp }}",
"updatedAt": "{{ matchTimestamp }}",
"publicKey": "{{ .publicKey }}",
"paymails": [
{
"alias": "{{ .alias }}",
"avatar": "",
"domain": "{{ .domain }}",
"id": 3,
"paymail": "{{ .paymail }}",
"publicName": "{{ .publicName }}"
}
]
}`, map[string]any{
"id": fixtures.Sender.ID(),
"publicKey": fixtures.Sender.PublicKey().ToDERHex(),
"paymail": fixtures.Sender.DefaultPaymail(),
"publicName": fixtures.Sender.DefaultPaymail().PublicName(),
"alias": fixtures.Sender.DefaultPaymail().Alias(),
"domain": fixtures.Sender.DefaultPaymail().Domain(),
})
}

func TestTryGetNonExistingUser(t *testing.T) {
// given:
given, then := testabilities.New(t)
cleanup := given.StartedSPVWalletWithConfiguration(testengine.WithV2())
defer cleanup()

// and:
client := given.HttpClient().ForAdmin()

// when:
resp, _ := client.
R().
SetPathParam("id", "non-existing-id").
Get("/api/v2/admin/users/{id}")

// then:
then.Response(resp).
WithProblemDetails(404, "not_found")
}
22 changes: 14 additions & 8 deletions actions/v2/admin/users/paymail_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,41 @@ package users
import (
"net/http"

adminerrors "github.com/bitcoin-sv/spv-wallet/actions/v2/admin/errors"
"github.com/bitcoin-sv/spv-wallet/actions/v2/admin/internal/mapping"
"github.com/bitcoin-sv/spv-wallet/api"
configerrors "github.com/bitcoin-sv/spv-wallet/config/errors"
"github.com/bitcoin-sv/spv-wallet/engine/spverrors"
"github.com/bitcoin-sv/spv-wallet/engine/v2/paymails/paymailerrors"
"github.com/bitcoin-sv/spv-wallet/errdef/clienterr"
"github.com/gin-gonic/gin"
)

// AddPaymailToUser add paymails to the user
func (s *APIAdminUsers) AddPaymailToUser(c *gin.Context, id string) {
var request api.RequestsAddPaymail
if err := c.Bind(&request); err != nil {
spverrors.ErrorResponse(c, spverrors.ErrCannotBindRequest.Wrap(err), s.logger)
clienterr.UnprocessableEntity.New().Wrap(err).Response(c, s.logger)
return
}

newPaymail, err := mapping.RequestAddPaymailToNewPaymailModel(&request, id)
if err != nil {
spverrors.ErrorResponse(c, err, s.logger)
clienterr.Response(c, err, s.logger)
return
}

createdPaymail, err := s.engine.PaymailsService().Create(c, newPaymail)

if err != nil {
spverrors.MapResponse(c, err, s.logger).
If(configerrors.ErrUnsupportedDomain).Then(adminerrors.ErrInvalidDomain).
If(paymailerrors.ErrInvalidAvatarURL).Then(adminerrors.ErrInvalidAvatarURL).
Else(adminerrors.ErrAddingPaymail)
clienterr.Map(err).
IfOfType(configerrors.UnsupportedDomain).
Then(
clienterr.BadRequest.Detailed("unsupported_domain", "Unsupported domain: '%s'", newPaymail.Domain),
).
IfOfType(paymailerrors.InvalidAvatarURL).
Then(
clienterr.UnprocessableEntity.Detailed("invalid_avatar_url", "Invalid avatar URL: '%s'", newPaymail.Avatar),
).
Response(c, s.logger)
return
}

Expand Down
Loading
Loading