change list grants to not show infra grants by default

api/grant.go

@@ -35,13 +35,14 @@ func (r *CreateGrantResponse) StatusCode() int {
 }
 
 type ListGrantsRequest struct {
-	User          uid.ID `form:"user" note:"ID of user granted access" example:"6TjWTAgYYu"`
-	Group         uid.ID `form:"group" note:"ID of group granted access" example:"6k3Eqcqu6B"`
-	Resource      string `form:"resource" example:"production.namespace" note:"a resource name"`
-	Destination   string `form:"destination" example:"production" note:"name of the destination where a connector is installed"`
-	Privilege     string `form:"privilege" example:"view" note:"a role or permission"`
-	ShowInherited bool   `form:"showInherited" note:"if true, this field includes grants that the user inherits through groups" example:"true"`
-	ShowSystem    bool   `form:"showSystem" note:"if true, this shows the connector and other internal grants" example:"false"`
+	User             uid.ID `form:"user" note:"ID of user granted access" example:"6TjWTAgYYu"`
+	Group            uid.ID `form:"group" note:"ID of group granted access" example:"6k3Eqcqu6B"`
+	Resource         string `form:"resource" example:"production.namespace" note:"a resource name"`
+	Destination      string `form:"destination" example:"production" note:"name of the destination where a connector is installed"`
+	Privilege        string `form:"privilege" example:"view" note:"a role or permission"`
+	ShowInherited    bool   `form:"showInherited" note:"if true, this field includes grants that the user inherits through groups" example:"true"`
+	ExcludeConnector bool   `form:"-"`
+	ShowSystem       bool   `form:"showSystem" note:"if true, this shows the connector and other internal grants" example:"false"`
 	BlockingRequest
 	PaginationRequest
 }

internal/server/data/grant.go

@@ -131,9 +131,14 @@ type ListGrantsOptions struct {
 	IncludeInheritedFromGroups bool
 
 	// ExcludeConnectorGrant instructs ListGrants to exclude grants where
-	// privilege=connector and resource=infra.
+	// privilege=connector and resource=infra. Note that this is only used
+	// for legacy API calls.
 	ExcludeConnectorGrant bool
 
+	// ExcludeInfraGrants instructs ListGrants to exclude grants which
+	// have their resource set to infra
+	ExcludeInfraGrants bool
+
 	Pagination *Pagination
 }
 
@@ -186,6 +191,9 @@ func ListGrants(tx ReadTxn, opts ListGrantsOptions) ([]models.Grant, error) {
 	if opts.ExcludeConnectorGrant {
 		query.B("AND NOT (privilege = 'connector' AND resource = 'infra')")
 	}
+	if opts.ExcludeInfraGrants {
+		query.B("AND NOT resource = 'infra'")
+	}
 
 	query.B("ORDER BY id ASC")
 	if opts.Pagination != nil {

internal/server/data/grant_test.go

@@ -622,15 +622,14 @@ func TestListGrants(t *testing.T) {
 			expected := []models.Grant{*grant1, *grant3, *grant4, *gGrant1, *gGrant2}
 			assert.DeepEqual(t, actual, expected, cmpModelByID)
 		})
-		t.Run("exclude connector grant", func(t *testing.T) {
-			actual, err := ListGrants(tx, ListGrantsOptions{ExcludeConnectorGrant: true})
+		t.Run("exclude infra grants", func(t *testing.T) {
+			actual, err := ListGrants(tx, ListGrantsOptions{ExcludeInfraGrants: true})
 			assert.NilError(t, err)
 
 			expected := []models.Grant{
 				*grant1,
 				*grant2,
 				*grant3,
-				*grant4,
 				*grant5,
 				*gGrant1,
 				*gGrant2,

internal/server/grants.go

@@ -36,7 +36,8 @@ func (a *API) ListGrants(c *gin.Context, r *api.ListGrantsRequest) (*api.ListRes
 		ByResource:                 r.Resource,
 		BySubject:                  subject,
 		ByDestination:              r.Destination,
-		ExcludeConnectorGrant:      !r.ShowSystem,
+		ExcludeConnectorGrant:      r.ExcludeConnector,
+		ExcludeInfraGrants:         !r.ShowSystem,
 		IncludeInheritedFromGroups: r.ShowInherited,
 	}
 	if r.Privilege != "" {
@@ -239,13 +240,25 @@ func (a *API) addPreviousVersionHandlersGrants() {
 		route[api.ListGrantsRequest, *api.ListResponse[grantV0_18_1]]{
 			routeSettings: defaultRouteSettingsGet,
 			handler: func(c *gin.Context, req *api.ListGrantsRequest) (*api.ListResponse[grantV0_18_1], error) {
+				req.ExcludeConnector = !req.ShowSystem
+				req.ShowSystem = true
 				resp, err := a.ListGrants(c, req)
 				return api.CopyListResponse(resp, func(item api.Grant) grantV0_18_1 {
 					return *newGrantsV0_18_1FromLatest(&item)
 				}), err
 			},
 		})
 
+	addVersionHandler(a, http.MethodGet, "/api/grants", "0.20.0",
+		route[api.ListGrantsRequest, *api.ListResponse[api.Grant]]{
+			routeSettings: defaultRouteSettingsGet,
+			handler: func(c *gin.Context, req *api.ListGrantsRequest) (*api.ListResponse[api.Grant], error) {
+				req.ExcludeConnector = !req.ShowSystem
+				req.ShowSystem = true
+				return a.ListGrants(c, req)
+			},
+		})
+
 	addVersionHandler(a, http.MethodGet, "/api/grants/:id", "0.18.1",
 		route[api.Resource, *grantV0_18_1]{
 			routeSettings: defaultRouteSettingsGet,

internal/server/grants_test.go

@@ -103,6 +103,9 @@ func TestAPI_ListGrants(t *testing.T) {
 	admin, err := data.GetIdentity(srv.DB(), data.GetIdentityOptions{ByName: "admin@example.com"})
 	assert.NilError(t, err)
 
+	connector, err := data.GetIdentity(srv.DB(), data.GetIdentityOptions{ByName: "connector"})
+	assert.NilError(t, err)
+
 	otherOrg := createOtherOrg(t, srv.db)
 
 	type testCase struct {
@@ -114,7 +117,7 @@ func TestAPI_ListGrants(t *testing.T) {
 	run := func(t *testing.T, tc testCase) {
 		req := httptest.NewRequest(http.MethodGet, tc.urlPath, nil)
 		req.Header.Set("Authorization", "Bearer "+accessKey)
-		req.Header.Add("Infra-Version", "0.18.2")
+		req.Header.Add("Infra-Version", "0.21.0")
 
 		if tc.setup != nil {
 			tc.setup(t, req)
@@ -220,31 +223,18 @@ func TestAPI_ListGrants(t *testing.T) {
 				assert.DeepEqual(t, respBody.FieldErrors, expected)
 			},
 		},
-		"no filters": {
-			urlPath: "/api/grants?showSystem=true",
+		"no filter args": {
+			urlPath: "/api/grants",
 			setup: func(t *testing.T, req *http.Request) {
 				req.Header.Set("Authorization", "Bearer "+adminAccessKey(srv))
 			},
 			expected: func(t *testing.T, resp *httptest.ResponseRecorder) {
-				connector, err := data.GetIdentity(srv.DB(), data.GetIdentityOptions{ByName: "connector"})
-				assert.NilError(t, err)
-
 				assert.Equal(t, resp.Code, http.StatusOK, resp.Body.String())
 				var grants api.ListResponse[api.Grant]
 				err = json.NewDecoder(resp.Body).Decode(&grants)
 				assert.NilError(t, err)
 
 				expected := []api.Grant{
-					{
-						User:      connector.ID,
-						Privilege: "connector",
-						Resource:  "infra",
-					},
-					{
-						User:      admin.ID,
-						Privilege: "admin",
-						Resource:  "infra",
-					},
 					{
 						User:      idInGroup,
 						Privilege: "custom1",
@@ -268,8 +258,8 @@ func TestAPI_ListGrants(t *testing.T) {
 				assert.DeepEqual(t, grants.Items, expected, cmpAPIGrantShallow)
 			},
 		},
-		"no filter, page 2": {
-			urlPath: "/api/grants?page=2&limit=2&showSystem=true",
+		"no filter args, page 2": {
+			urlPath: "/api/grants?page=2&limit=2",
 			setup: func(t *testing.T, req *http.Request) {
 				req.Header.Set("Authorization", "Bearer "+adminAccessKey(srv))
 			},
@@ -280,23 +270,18 @@ func TestAPI_ListGrants(t *testing.T) {
 				assert.NilError(t, err)
 
 				expected := []api.Grant{
-					{
-						User:      idInGroup,
-						Privilege: "custom1",
-						Resource:  "res1",
-					},
 					{
 						User:      idOther,
-						Privilege: "custom2",
-						Resource:  "res1.ns1",
+						Privilege: "connector",
+						Resource:  "res1.ns2",
 					},
 				}
 				assert.DeepEqual(t, grants.Items, expected, cmpAPIGrantShallow)
-				assert.Equal(t, grants.PaginationResponse, api.PaginationResponse{Limit: 2, Page: 2, TotalCount: 5, TotalPages: 3})
+				assert.Equal(t, grants.PaginationResponse, api.PaginationResponse{Limit: 2, Page: 2, TotalCount: 3, TotalPages: 2})
 			},
 		},
-		"hide infra connector": {
-			urlPath: "/api/grants",
+		"show infra grants": {
+			urlPath: "/api/grants?showSystem=true",
 			setup: func(t *testing.T, req *http.Request) {
 				req.Header.Set("Authorization", "Bearer "+adminAccessKey(srv))
 			},
@@ -307,6 +292,11 @@ func TestAPI_ListGrants(t *testing.T) {
 				assert.NilError(t, err)
 
 				expected := []api.Grant{
+					{
+						User:      connector.ID,
+						Privilege: "connector",
+						Resource:  "infra",
+					},
 					{
 						User:      admin.ID,
 						Privilege: "admin",
@@ -504,6 +494,51 @@ func TestAPI_ListGrants(t *testing.T) {
 				assert.Equal(t, resp.Result().Header.Get("Last-Update-Index"), "10004")
 			},
 		},
+		"migration from <= 0.20.0 show system grants": {
+			urlPath: "/api/grants?showSystem=true",
+			setup: func(t *testing.T, req *http.Request) {
+				req.Header.Set("Authorization", "Bearer "+adminAccessKey(srv))
+				req.Header.Set("Infra-Version", "0.19.0")
+			},
+			expected: func(t *testing.T, resp *httptest.ResponseRecorder) {
+				assert.Equal(t, resp.Code, http.StatusOK, resp.Body.String())
+
+				var grants api.ListResponse[api.Grant]
+				err = json.NewDecoder(resp.Body).Decode(&grants)
+				assert.NilError(t, err)
+
+				expected := []api.Grant{
+					{User: connector.ID, Privilege: "connector", Resource: "infra"},
+					{User: admin.ID, Privilege: "admin", Resource: "infra"},
+					{User: idInGroup, Privilege: "custom1", Resource: "res1"},
+					{User: idOther, Privilege: "custom2", Resource: "res1.ns1"},
+					{User: idOther, Privilege: "connector", Resource: "res1.ns2"},
+				}
+				assert.DeepEqual(t, grants.Items, expected, cmpAPIGrantShallow)
+			},
+		},
+		"migration from <= 0.20.0 don't show system grants": {
+			urlPath: "/api/grants?showSystem=false",
+			setup: func(t *testing.T, req *http.Request) {
+				req.Header.Set("Authorization", "Bearer "+adminAccessKey(srv))
+				req.Header.Set("Infra-Version", "0.19.0")
+			},
+			expected: func(t *testing.T, resp *httptest.ResponseRecorder) {
+				assert.Equal(t, resp.Code, http.StatusOK, resp.Body.String())
+
+				var grants api.ListResponse[api.Grant]
+				err = json.NewDecoder(resp.Body).Decode(&grants)
+				assert.NilError(t, err)
+
+				expected := []api.Grant{
+					{User: admin.ID, Privilege: "admin", Resource: "infra"},
+					{User: idInGroup, Privilege: "custom1", Resource: "res1"},
+					{User: idOther, Privilege: "custom2", Resource: "res1.ns1"},
+					{User: idOther, Privilege: "connector", Resource: "res1.ns2"},
+				}
+				assert.DeepEqual(t, grants.Items, expected, cmpAPIGrantShallow)
+			},
+		},
 		"migration from <= 0.18.1": {
 			urlPath: "/api/grants?user=" + idInGroup.String(),
 			setup: func(t *testing.T, req *http.Request) {

internal/server/openapi.go

@@ -534,6 +534,9 @@ func removeString(seq []string, name string) []string {
 
 func getFieldName(f reflect.StructField, parent reflect.Type) string {
 	if name, ok := f.Tag.Lookup("form"); ok {
+		if name == "-" {
+			return ""
+		}
 		validateFieldName(name)
 		return name
 	}
@@ -562,6 +565,8 @@ func validateFieldName(name string) {
 	switch name {
 	case "unique_id":
 		return
+	case "-":
+		return
 	}
 
 	if strings.Contains(name, "_") || strings.Contains(name, "-") {

internal/server/routes_test.go

@@ -164,7 +164,7 @@ func TestTrimWhitespace(t *testing.T) {
 	// nolint:noctx
 	req = httptest.NewRequest(http.MethodGet, "/api/grants?privilege=%20admin%20&userID="+userID.String(), nil)
 	req.Header.Add("Authorization", "Bearer "+adminAccessKey(srv))
-	req.Header.Add("Infra-Version", "0.13.1")
+	req.Header.Add("Infra-Version", "0.21.0")
 
 	resp = httptest.NewRecorder()
 	routes.ServeHTTP(resp, req)
@@ -174,13 +174,13 @@ func TestTrimWhitespace(t *testing.T) {
 	err := json.Unmarshal(resp.Body.Bytes(), rb)
 	assert.NilError(t, err)
 
-	assert.Equal(t, len(rb.Items), 2, rb.Items)
+	assert.Equal(t, len(rb.Items), 1, rb.Items)
 	expected := api.Grant{
 		User:      userID,
 		Privilege: "admin",
 		Resource:  "kubernetes.production.*",
 	}
-	assert.DeepEqual(t, rb.Items[1], expected, cmpAPIGrantShallow)
+	assert.DeepEqual(t, rb.Items[0], expected, cmpAPIGrantShallow)
 }
 
 func TestWrapRoute_TxnRollbackOnError(t *testing.T) {

ui/lib/fetch.js

@@ -1,6 +1,6 @@
 const fetch = global.fetch
 
-const base = '0.19.1'
+const base = '0.21.0'
 
 // Patch the global fetch to include our base API
 // version for requests to the same domain

ui/lib/hooks.js

@@ -8,7 +8,7 @@ export function useUser() {
   const { data: org } = useSWR(() => (user ? '/api/organizations/self' : null))
   const { data: { items: grants } = {}, grantsError } = useSWR(() =>
     user
-      ? `/api/grants?user=${user?.id}&showInherited=1&resource=infra&limit=1000`
+      ? `/api/grants?user=${user?.id}&showInherited=1&resource=infra&limit=1000&showSystem=true`
       : null
   )
 

ui/pages/groups/[id].js

@@ -104,7 +104,7 @@ export default function GroupDetails() {
   } = useSWR(`/api/users?group=${group?.id}&limit=${limit}&p=${page}`)
   const { data: { items: allUsers } = {} } = useSWR(`/api/users?limit=1000`)
   const { data: { items: infraAdmins } = {} } = useSWR(
-    '/api/grants?resource=infra&privilege=admin&limit=1000'
+    '/api/grants?resource=infra&privilege=admin&limit=1000&showSystem=true'
   )
   const [addUser, setAddUser] = useState('')
   const [selectedDeleteIds, setSelectedDeleteIds] = useState([])

ui/pages/settings/index.js

@@ -547,7 +547,7 @@ function Admins() {
   const { data: { items: users } = {} } = useSWR('/api/users?limit=1000')
   const { data: { items: groups } = {} } = useSWR('/api/groups?limit=1000')
   const { data: { items: grants } = {}, mutate } = useSWR(
-    '/api/grants?resource=infra&privilege=admin&limit=1000'
+    '/api/grants?resource=infra&privilege=admin&limit=1000&showSystem=true'
   )
   const { data: { items: selfGroups } = {} } = useSWR(
     () => `/api/groups?userID=${user?.id}&limit=1000`