fix: remove sanitize but still crop fields (#1113)

api/dashboard/create.go

@@ -143,7 +143,7 @@ func CreateDashboard(c *gin.Context) {
 // when a user is inactive or not found in the database. It returns a sanitized slice of admins.
 func createAdminSet(c context.Context, caller *types.User, users []*types.User) ([]*types.User, error) {
 	// add user creating the dashboard to admin list
-	admins := []*types.User{caller.Sanitize()}
+	admins := []*types.User{caller.Crop()}
 
 	dupMap := make(map[string]bool)
 
@@ -158,7 +158,7 @@ func createAdminSet(c context.Context, caller *types.User, users []*types.User)
 			return nil, fmt.Errorf("unable to create dashboard: %s is not an active user", u.GetName())
 		}
 
-		admins = append(admins, dbUser.Sanitize())
+		admins = append(admins, dbUser.Crop())
 
 		dupMap[dbUser.GetName()] = true
 	}

api/types/user.go

@@ -4,8 +4,6 @@ package types
 
 import (
 	"fmt"
-
-	"github.com/go-vela/types/constants"
 )
 
 // User is the API representation of a user.
@@ -22,18 +20,15 @@ type User struct {
 	Dashboards   *[]string `json:"dashboards,omitempty"`
 }
 
-// Sanitize creates a duplicate of the User without the token values.
-func (u *User) Sanitize() *User {
-	// create a variable since constants can not be addressable
-	//
-	// https://golang.org/ref/spec#Address_operators
-	value := constants.SecretMask
-
+// Crop creates a duplicate of the User with certain fields cropped.
+//
+// Generally used for cropping large fields that aren't useful for all API calls like favorites and dashboards.
+func (u *User) Crop() *User {
 	return &User{
 		ID:           u.ID,
 		Name:         u.Name,
-		RefreshToken: &value,
-		Token:        &value,
+		RefreshToken: u.RefreshToken,
+		Token:        u.Token,
 		Active:       u.Active,
 	}
 }

api/types/user_test.go

@@ -6,26 +6,22 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
-
-	"github.com/go-vela/types/constants"
 )
 
-func TestTypes_User_Sanitize(t *testing.T) {
+func TestTypes_User_Crop(t *testing.T) {
 	// setup types
 	u := testUser()
 
-	want := new(User)
-	want.SetID(1)
-	want.SetName("octocat")
-	want.SetActive(true)
-	want.SetToken(constants.SecretMask)
-	want.SetRefreshToken(constants.SecretMask)
+	want := testUser()
+	want.Favorites = nil
+	want.Dashboards = nil
+	want.Admin = nil
 
 	// run test
-	got := u.Sanitize()
+	got := u.Crop()
 
 	if !reflect.DeepEqual(got, want) {
-		t.Errorf("Sanitize is %v, want %v", got, want)
+		t.Errorf("Crop is %v, want %v", got, want)
 	}
 }
 

database/integration_test.go

@@ -438,18 +438,14 @@ func testDashboards(t *testing.T, db Interface, resources *Resources) {
 	for _, dashboard := range resources.Dashboards {
 		got, err := db.GetDashboard(ctx, dashboard.GetID())
 		if err != nil {
-			t.Errorf("unable to get schedule %s: %v", dashboard.GetID(), err)
+			t.Errorf("unable to get dashboard %s: %v", dashboard.GetID(), err)
 		}
 
 		// JSON tags of `-` prevent unmarshaling of tokens, but they are sanitized anyway
 		cmpAdmins := []*api.User{}
 		for _, admin := range got.GetAdmins() {
-			admin.SetToken(constants.SecretMask)
-			admin.SetRefreshToken(constants.SecretMask)
-
-			cmpAdmins = append(cmpAdmins, admin)
+			cmpAdmins = append(cmpAdmins, admin.Crop())
 		}
-
 		got.SetAdmins(cmpAdmins)
 
 		if !cmp.Equal(got, dashboard, CmpOptApproxUpdatedAt()) {
@@ -476,7 +472,7 @@ func testDashboards(t *testing.T, db Interface, resources *Resources) {
 	for _, dashboard := range resources.Dashboards {
 		err := db.DeleteDashboard(ctx, dashboard)
 		if err != nil {
-			t.Errorf("unable to delete schedule %s: %v", dashboard.GetID(), err)
+			t.Errorf("unable to delete dashboard %s: %v", dashboard.GetID(), err)
 		}
 	}
 	methods["DeleteDashboard"] = true
@@ -2131,6 +2127,13 @@ func newResources() *Resources {
 	userTwo.SetAdmin(false)
 	userTwo.SetDashboards([]string{"45bcf19b-c151-4e2d-b8c6-80a62ba2eae7", "ba657dab-bc6e-421f-9188-86272bd0069a"})
 
+	// crop and set "-" JSON tag fields to nil for dashboard admins
+	dashboardAdmins := []*api.User{userOne.Crop(), userTwo.Crop()}
+	for _, admin := range dashboardAdmins {
+		admin.Token = nil
+		admin.RefreshToken = nil
+	}
+
 	buildOne := new(library.Build)
 	buildOne.SetID(1)
 	buildOne.SetRepoID(1)
@@ -2216,7 +2219,7 @@ func newResources() *Resources {
 	dashboardOne.SetCreatedBy("octocat")
 	dashboardOne.SetUpdatedAt(2)
 	dashboardOne.SetUpdatedBy("octokitty")
-	dashboardOne.SetAdmins([]*api.User{userOne.Sanitize(), userTwo.Sanitize()})
+	dashboardOne.SetAdmins(dashboardAdmins)
 	dashboardOne.SetRepos([]*api.DashboardRepo{dashRepo})
 
 	dashboardTwo := new(api.Dashboard)
@@ -2226,7 +2229,7 @@ func newResources() *Resources {
 	dashboardTwo.SetCreatedBy("octocat")
 	dashboardTwo.SetUpdatedAt(2)
 	dashboardTwo.SetUpdatedBy("octokitty")
-	dashboardTwo.SetAdmins([]*api.User{userOne.Sanitize(), userTwo.Sanitize()})
+	dashboardTwo.SetAdmins(dashboardAdmins)
 	dashboardTwo.SetRepos([]*api.DashboardRepo{dashRepo})
 
 	executableOne := new(library.BuildExecutable)
@@ -2386,7 +2389,7 @@ func newResources() *Resources {
 
 	repoOne := new(api.Repo)
 	repoOne.SetID(1)
-	repoOne.SetOwner(userOne.Sanitize())
+	repoOne.SetOwner(userOne.Crop())
 	repoOne.SetHash("MzM4N2MzMDAtNmY4Mi00OTA5LWFhZDAtNWIzMTlkNTJkODMy")
 	repoOne.SetOrg("github")
 	repoOne.SetName("octocat")
@@ -2409,7 +2412,7 @@ func newResources() *Resources {
 
 	repoTwo := new(api.Repo)
 	repoTwo.SetID(2)
-	repoTwo.SetOwner(userOne.Sanitize())
+	repoTwo.SetOwner(userOne.Crop())
 	repoTwo.SetHash("MzM4N2MzMDAtNmY4Mi00OTA5LWFhZDAtNWIzMTlkNTJkODMy")
 	repoTwo.SetOrg("github")
 	repoTwo.SetName("octokitty")

database/repo/get_org_test.go

@@ -30,6 +30,7 @@ func TestRepo_Engine_GetRepoForOrg(t *testing.T) {
 	_owner := testOwner()
 	_owner.SetID(1)
 	_owner.SetName("foo")
+	_owner.SetToken("bar")
 
 	_repo.SetOwner(_owner)
 

database/repo/get_test.go

@@ -30,6 +30,7 @@ func TestRepo_Engine_GetRepo(t *testing.T) {
 	_owner := testOwner()
 	_owner.SetID(1)
 	_owner.SetName("foo")
+	_owner.SetToken("bar")
 
 	_repo.SetOwner(_owner)
 

database/repo/list_org_test.go

@@ -56,6 +56,7 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) {
 	_owner := testOwner()
 	_owner.SetID(1)
 	_owner.SetName("foo")
+	_owner.SetToken("bar")
 
 	_repoOne.SetOwner(_owner)
 	_repoTwo.SetOwner(_owner)

database/repo/list_test.go

@@ -41,6 +41,7 @@ func TestRepo_Engine_ListRepos(t *testing.T) {
 	_owner := testOwner()
 	_owner.SetID(1)
 	_owner.SetName("foo")
+	_owner.SetToken("bar")
 
 	_repoOne.SetOwner(_owner)
 	_repoTwo.SetOwner(_owner)

database/repo/list_user_test.go

@@ -58,6 +58,7 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) {
 	_owner := testOwner()
 	_owner.SetID(1)
 	_owner.SetName("foo")
+	_owner.SetToken("bar")
 
 	_repoOne.SetOwner(_owner)
 	_repoTwo.SetOwner(_owner)

database/repo/repo.go

@@ -293,7 +293,7 @@ func (r *Repo) ToAPI() *api.Repo {
 	repo := new(api.Repo)
 
 	repo.SetID(r.ID.Int64)
-	repo.SetOwner(r.Owner.ToAPI().Sanitize())
+	repo.SetOwner(r.Owner.ToAPI().Crop())
 	repo.SetHash(r.Hash.String)
 	repo.SetOrg(r.Org.String)
 	repo.SetName(r.Name.String)

database/repo/repo_test.go

@@ -232,13 +232,11 @@ func testEvents() *api.Events {
 
 // testOwner is a helper function that returns a sanitized user.
 func testOwner() *api.User {
-	mask := constants.SecretMask
-
 	return &api.User{
 		ID:           new(int64),
 		Name:         new(string),
-		RefreshToken: &mask,
-		Token:        &mask,
+		RefreshToken: new(string),
+		Token:        new(string),
 		Active:       new(bool),
 	}
 }

router/router.go

@@ -124,7 +124,7 @@ func Load(options ...gin.HandlerFunc) *gin.Engine {
 		// Source code management endpoints
 		ScmHandlers(baseAPI)
 
-		// Search endpoints
+		// Dashboard endpoints
 		DashboardHandlers(baseAPI)
 
 		// Secret endpoints