diff --git a/api/grant.go b/api/grant.go index 5df84b386b..89d52cb6be 100644 --- a/api/grant.go +++ b/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 } diff --git a/internal/server/data/grant.go b/internal/server/data/grant.go index 1f93297c11..f77552f517 100644 --- a/internal/server/data/grant.go +++ b/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 { diff --git a/internal/server/data/grant_test.go b/internal/server/data/grant_test.go index f1aa10e1cd..dde64ee610 100644 --- a/internal/server/data/grant_test.go +++ b/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, diff --git a/internal/server/grants.go b/internal/server/grants.go index 8edf3bc717..e375399a07 100644 --- a/internal/server/grants.go +++ b/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,6 +240,8 @@ 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) @@ -246,6 +249,16 @@ func (a *API) addPreviousVersionHandlersGrants() { }, }) + 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, diff --git a/internal/server/grants_test.go b/internal/server/grants_test.go index ed066a6b35..f10eb8b5ef 100644 --- a/internal/server/grants_test.go +++ b/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) { diff --git a/internal/server/openapi.go b/internal/server/openapi.go index abc7b7e4dc..b78ff31d0d 100644 --- a/internal/server/openapi.go +++ b/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, "-") { diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index 21d5cf55e0..3cc115b6fb 100644 --- a/internal/server/routes_test.go +++ b/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) { diff --git a/ui/lib/fetch.js b/ui/lib/fetch.js index 5b9d7cac06..b0169c2705 100644 --- a/ui/lib/fetch.js +++ b/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 diff --git a/ui/lib/hooks.js b/ui/lib/hooks.js index 772429db0f..79f74cfcad 100644 --- a/ui/lib/hooks.js +++ b/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 ) diff --git a/ui/pages/groups/[id].js b/ui/pages/groups/[id].js index 9358d94f6c..30b2a10ae6 100644 --- a/ui/pages/groups/[id].js +++ b/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=999`) const { data: { items: infraAdmins } = {} } = useSWR( - '/api/grants?resource=infra&privilege=admin&limit=999' + '/api/grants?resource=infra&privilege=admin&limit=999&showSystem=true' ) const [addUser, setAddUser] = useState('') diff --git a/ui/pages/settings/index.js b/ui/pages/settings/index.js index 6a1eb0c61a..7e45aa89ad 100644 --- a/ui/pages/settings/index.js +++ b/ui/pages/settings/index.js @@ -544,7 +544,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`