Skip to content
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.

Sign up for GitHub

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

feat(SPV-1248) add domain check for paymail entry #828

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(SPV-1248) add domain check for paymail entry #828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions actions/admin/paymail_addresses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package admin
import (
"net/http"

"github.com/bitcoin-sv/go-paymail"
"github.com/bitcoin-sv/spv-wallet/actions/common"
"github.com/bitcoin-sv/spv-wallet/engine"
"github.com/bitcoin-sv/spv-wallet/engine/spverrors"
Expand Down Expand Up @@ -130,6 +131,15 @@ func paymailCreateAddress(c *gin.Context, _ *reqctx.AdminContext) {
opts = append(opts, engine.WithMetadatas(requestBody.Metadata))
}

config := reqctx.AppConfig(c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO PR is a good opportunity to write some endpoint tests for create paymail address endpoint.

if config.Paymail.DomainValidationEnabled {
_, actualDomain, _ := paymail.SanitizePaymail(requestBody.Address)
if !checkPaymailDomain(actualDomain, config.Paymail.Domains) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could simply use slices.Contains

Suggested change
if !checkPaymailDomain(actualDomain, config.Paymail.Domains) {
if !slices.Contains(config.Paymail.Domains, actualDomain) {

spverrors.ErrorResponse(c, spverrors.ErrInvalidDomain, logger)
return
}
}

var paymailAddress *engine.PaymailAddress
paymailAddress, err := reqctx.Engine(c).NewPaymailAddress(
c.Request.Context(), requestBody.Key, requestBody.Address, requestBody.PublicName, requestBody.Avatar, opts...)
Expand All @@ -143,6 +153,15 @@ func paymailCreateAddress(c *gin.Context, _ *reqctx.AdminContext) {
c.JSON(http.StatusCreated, paymailAddressContract)
}

func checkPaymailDomain(domain string, domains []string) bool {
for _, d := range domains {
if d == domain {
return true
}
}
return false
}

// paymailDeleteAddress will delete a paymail address
// Delete Paymail godoc
// @Summary Delete paymail
Expand Down
10 changes: 10 additions & 0 deletions actions/admin/paymail_addresses_old.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package admin
import (
"net/http"

"github.com/bitcoin-sv/go-paymail"
"github.com/bitcoin-sv/spv-wallet/engine"
"github.com/bitcoin-sv/spv-wallet/engine/spverrors"
"github.com/bitcoin-sv/spv-wallet/mappings"
Expand Down Expand Up @@ -163,6 +164,15 @@ func paymailCreateAddressOld(c *gin.Context, _ *reqctx.AdminContext) {
opts = append(opts, engine.WithMetadatas(requestBody.Metadata))
}

config := reqctx.AppConfig(c)
if config.Paymail.DomainValidationEnabled {
_, actualDomain, _ := paymail.SanitizePaymail(requestBody.Address)
if !checkPaymailDomain(actualDomain, config.Paymail.Domains) {
spverrors.ErrorResponse(c, spverrors.ErrInvalidDomain, logger)
return
}
}
Comment on lines +167 to +174
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicated code.
But since we plan to remove the "old" handlers soon (I believe 🙈), it is totally fine to me 👍


var paymailAddress *engine.PaymailAddress
paymailAddress, err := reqctx.Engine(c).NewPaymailAddress(
c.Request.Context(), requestBody.Key, requestBody.Address, requestBody.PublicName, requestBody.Avatar, opts...)
Expand Down
3 changes: 3 additions & 0 deletions engine/spverrors/definitions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@ var ErrMissingAddress = models.SPVError{Message: "missing required field: addres
// ErrMissingFieldScriptPubKey is when the field is required but missing
var ErrMissingFieldScriptPubKey = models.SPVError{Message: "missing required field: script_pub_key", StatusCode: 400, Code: "error-missing-field-script-pub-key"}

// ErrInvalidDomain is when the domain is wrong
var ErrInvalidDomain = models.SPVError{Message: "invalid domain", StatusCode: 400, Code: "error-invalid-domain"}

// ErrMissingFieldSatoshis is when the field satoshis is required but missing
var ErrMissingFieldSatoshis = models.SPVError{Message: "missing required field: satoshis", StatusCode: 400, Code: "error-missing-field-satoshis"}

Expand Down
Loading