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.

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

Unable to correctly bind field value when using .Sensitive() method with Entgo and Golang Echo #3891

Closed
willie-lin opened this issue Jan 6, 2024 · 2 comments

Comments

@willie-lin
Copy link

willie-lin commented Jan 6, 2024

Current Behavior:
When defining a field with the .Sensitive() method in Entgo, the value of this field cannot be correctly bound to the entity.
Expected Behavior:
The value of the field should be correctly bound to the entity, even when the field is defined as sensitive.

Steps to Reproduce:

  1. Define an entity with a sensitive field:
func (User) Fields() []ent.Field {
	return []ent.Field{
		field.String("password").NotEmpty().MinLen(8).MaxLen(120).Sensitive(),
		// other fields...
	}
}
  1. Bind the request body to the entity using Echo’s c.Bind(u) method:
u := new(ent.User)
if err := c.Bind(u); err != nil {
	log.Printf("Error binding user: %v", err)
	return c.JSON(http.StatusBadRequest, map[string]string{"error": err.Error()})
}
  1. Print the value of the sensitive field:
fmt.Println(u.Password)
fmt.Println(len(u.Password))

The length of u.Password is always 0, indicating that the value of the sensitive field has not been correctly bound.
To verify this issue, I conducted the following test:

body, err := ioutil.ReadAll(c.Request().Body)
if err != nil {
    // handle error
}
fmt.Println(string(body))
c.Request().Body = ioutil.NopCloser(bytes.NewBuffer(body))

Through this test, I can confirm that the request body indeed contains the value of the password field:

{"email":"1234567@123.com","password":"ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f","totp_Secret":""}

I hope to get some help to solve this problem. Thank you!

Runtime Environment:

Tech Version
Go 1.21.5.
Ent 0.12.5
Echo. v4.11.4
Database MySQL
Driver https://github.com/go-sql-driver/mysql
@willie-lin willie-lin changed the title Unable to correctly bind sensitive field value using .Sensitive() method with Golang Echo Unable to correctly bind field value when using .Sensitive() method with Entgo and Golang Echo Jan 7, 2024
@lrstanley
Copy link

lrstanley commented Jan 9, 2024

Sensitive() means that the field is not serializable (i.e. json struct tags aren't added and/or are set to -), so this is working as expected, I think. Since Echos Bind() likely uses JSON struct tags behind the scenes, and JSON doesn't support one-way struct tags for marshalling vs unmarshalling, there is no way to have both.

I don't believe you should be unmarshalling directly to an ent type -- as that bypasses various checks. You should have structs specifically for your HTTP methods that can then be used with Set() and similar. This will also ensure that if you add a field that you don't want the user to set, Bind() can't be used maliciously to override those fields. Without this, I see it as a future security risk.

@willie-lin
Copy link
Author

Sensitive() means that the field is not serializable (i.e. json struct tags aren't added and/or are set to -), so this is working as expected, I think. Since Echos Bind() likely uses JSON struct tags behind the scenes, and JSON doesn't support one-way struct tags for marshalling vs unmarshalling, there is no way to have both.

I don't believe you should be unmarshalling directly to an ent type -- as that bypasses various checks. You should have structs specifically for your HTTP methods that can then be used with Set() and similar. This will also ensure that if you add a field that you don't want the user to set, Bind() can't be used maliciously to override those fields. Without this, I see it as a future security risk.

Thank you for your explanation, it’s very clear. I understand that Sensitive() means the field is not serializable, which is why Echo’s Bind() function can’t use it directly. I also understand that JSON doesn’t support one-way struct tags for marshalling vs unmarshalling, so there’s no way to have both.

I agree with your point that I shouldn’t be unmarshalling directly to an ent type, as this bypasses various checks. I should create structs specifically for my HTTP methods that can then be used with Set() and similar. This will ensure that if I add a field that I don’t want the user to set, Bind() can’t be used maliciously to override those fields, thereby avoiding a future security risk.

package main

import (
	"github.com/labstack/echo/v4"
	"net/http"
)

type User struct {
	Username string `json:"-"`
	Password string `json:"password"`
}

type UserDTO struct {
	Username string `json:"username"`
	Password string `json:"password"`
}

func main() {
	e := echo.New()

	e.POST("/users", func(c echo.Context) error {
		u := new(UserDTO)
		if err := c.Bind(u); err != nil {
			return err
		}

		user := User{
			Username: u.Username,
			Password: u.Password,
		}

		
		// saveUser(user)

		return c.JSON(http.StatusCreated, u)
	})

	e.Start(":8080")
}

This code creates a UserDTO structure to receive json data transmitted from the front end. Then, we safely transmit this data to our User entity. In this way, we can effectively bind data while protecting sensitive information. Thanks again for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants