Remove "projects" from grant strings (#143)

This removes the ability for a grant string to switch projects, in
accordance with our updated design. It mostly removes code, with a few
test fixes, and also adds one additional check (and test) to ensure a
grant errors rather than do-nothing succeeds if both id and type are
empty.
revert-143-remove-projects-from-grants
Jeff Mitchell 6 years ago committed by GitHub
parent a02222c1f0
commit 993a6f1680
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -40,7 +40,7 @@ func Test_ACLAllowed(t *testing.T) {
{
scope: Scope{Type: iam.OrganizationScope, Id: "org_b"},
grants: []string{
"project=proj_x;type=host-set;actions=list,create",
"type=host-set;actions=list,create",
"type=host;actions=*",
"id=*;actions=authen",
},
@ -171,29 +171,9 @@ func Test_ACLAllowed(t *testing.T) {
{action: iam.ActionDelete},
},
},
{
name: "cross project, bad project",
resource: Resource{ScopeId: "proj_y", Type: "host-set"},
scopeGrants: commonGrants,
actionsAllowed: []actionAllowed{
{action: iam.ActionList},
{action: iam.ActionCreate},
{action: iam.ActionDelete},
},
},
{
name: "cross project, good project",
resource: Resource{ScopeId: "proj_x", Type: "host-set"},
scopeGrants: commonGrants,
actionsAllowed: []actionAllowed{
{action: iam.ActionList, allowed: true},
{action: iam.ActionCreate, allowed: true},
{action: iam.ActionDelete},
},
},
{
name: "any id",
resource: Resource{ScopeId: "org_b", Type: "host-set"},
resource: Resource{ScopeId: "org_b", Type: "auth-method"},
scopeGrants: commonGrants,
actionsAllowed: []actionAllowed{
{action: iam.ActionList},

@ -38,9 +38,6 @@ type Grant struct {
// The scope ID, which will be a project ID or an org ID
scope Scope
// Project, if defined
project string
// The ID in the grant, if provided.
id string
@ -57,10 +54,9 @@ type Grant struct {
func (g Grant) clone() *Grant {
ret := &Grant{
scope: g.scope,
project: g.project,
id: g.id,
typ: g.typ,
scope: g.scope,
id: g.id,
typ: g.typ,
}
if g.actionsBeingParsed != nil {
ret.actionsBeingParsed = append(ret.actionsBeingParsed, g.actionsBeingParsed...)
@ -77,9 +73,6 @@ func (g Grant) clone() *Grant {
// CanonicalString returns the canonical representation of the grant
func (g Grant) CanonicalString() string {
var builder []string
if g.project != "" {
builder = append(builder, fmt.Sprintf("project=%s", g.project))
}
if g.id != "" {
builder = append(builder, fmt.Sprintf("id=%s", g.id))
@ -104,9 +97,6 @@ func (g Grant) CanonicalString() string {
// MarshalJSON provides a custom marshaller for grants
func (g Grant) MarshalJSON() ([]byte, error) {
res := make(map[string]interface{}, 4)
if g.project != "" {
res["project"] = g.project
}
if g.id != "" {
res["id"] = g.id
}
@ -132,13 +122,6 @@ func (g *Grant) unmarshalJSON(data []byte) error {
if err := json.Unmarshal(data, &raw); err != nil {
return err
}
if rawProj, ok := raw["project"]; ok {
project, ok := rawProj.(string)
if !ok {
return fmt.Errorf("unable to interpret %q as string", "project")
}
g.project = strings.ToLower(project)
}
if rawId, ok := raw["id"]; ok {
id, ok := rawId.(string)
if !ok {
@ -192,9 +175,6 @@ func (g *Grant) unmarshalText(grantString string) error {
}
switch kv[0] {
case "project":
g.project = strings.ToLower(kv[1])
case "id":
g.id = strings.ToLower(kv[1])
@ -225,15 +205,6 @@ func (g *Grant) unmarshalText(grantString string) error {
//
// The scope must be the org and project where this grant originated, not the
// request.
//
// WARNING: It is the responsibility of the caller to validate that a returned
// Grant matches the incoming scope and if not that the relationship is valid.
// Specifically, if a project is specified as part of a grant, the grant's
// returned scope will be a project scope with the associated project ID. The
// caller must validate that the project ID is valid and that its enclosing
// organization is the original organization scope. Likely this can be done in a
// centralized helper context; however it's not done here to avoid reaching into
// the database from within this package.
func Parse(scope Scope, userId, grantString string) (Grant, error) {
if len(grantString) == 0 {
return Grant{}, errors.New("grant string is empty")
@ -278,8 +249,8 @@ func Parse(scope Scope, userId, grantString string) (Grant, error) {
}
}
if err := grant.validateAndModifyProject(); err != nil {
return Grant{}, err
if grant.id == "" && grant.typ == "" {
return Grant{}, errors.New(`"id" and "type" cannot both be empty`)
}
if err := grant.validateType(); err != nil {
@ -293,18 +264,6 @@ func Parse(scope Scope, userId, grantString string) (Grant, error) {
return grant, nil
}
func (g *Grant) validateAndModifyProject() error {
if g.project == "" {
return nil
}
if g.scope.Type != iam.OrganizationScope {
return errors.New("cannot specify a project in the grant when the scope is not an organization")
}
g.scope.Type = iam.ProjectScope
g.scope.Id = g.project
return nil
}
func (g Grant) validateType() error {
switch g.typ {
case TypeNone,

@ -130,72 +130,6 @@ func Test_ValidateType(t *testing.T) {
}
}
func Test_ValidateProject(t *testing.T) {
t.Parallel()
type input struct {
name string
input Grant
output Grant
errResult string
}
tests := []input{
{
name: "no project",
input: Grant{
scope: Scope{
Type: iam.OrganizationScope,
},
},
output: Grant{
scope: Scope{
Type: iam.OrganizationScope,
},
},
},
{
name: "project, organization scope",
input: Grant{
project: "foobar",
scope: Scope{
Type: iam.OrganizationScope,
},
},
output: Grant{
project: "foobar",
scope: Scope{
Type: iam.ProjectScope,
Id: "foobar",
},
},
},
{
name: "project, non-organization scope",
input: Grant{
project: "foobar",
scope: Scope{
Type: iam.ProjectScope,
},
},
errResult: "cannot specify a project in the grant when the scope is not an organization",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := test.input.validateAndModifyProject()
if test.errResult == "" {
require.NoError(t, err)
assert.Equal(t, test.output, test.input)
} else {
require.Error(t, err)
assert.Equal(t, test.errResult, err.Error())
}
})
}
}
func Test_MarshallingAndCloning(t *testing.T) {
t.Parallel()
@ -218,46 +152,21 @@ func Test_MarshallingAndCloning(t *testing.T) {
canonicalString: ``,
},
{
name: "project",
input: Grant{
project: "foobar",
scope: Scope{
Type: iam.OrganizationScope,
},
},
jsonOutput: `{"project":"foobar"}`,
canonicalString: `project=foobar`,
},
{
name: "project and type",
input: Grant{
project: "foobar",
scope: Scope{
Type: iam.ProjectScope,
},
typ: TypeGroup,
},
jsonOutput: `{"project":"foobar","type":"group"}`,
canonicalString: `project=foobar;type=group`,
},
{
name: "project, type, and id",
name: "type and id",
input: Grant{
id: "baz",
project: "foobar",
id: "baz",
scope: Scope{
Type: iam.ProjectScope,
},
typ: TypeGroup,
},
jsonOutput: `{"id":"baz","project":"foobar","type":"group"}`,
canonicalString: `project=foobar;id=baz;type=group`,
jsonOutput: `{"id":"baz","type":"group"}`,
canonicalString: `id=baz;type=group`,
},
{
name: "everything",
input: Grant{
id: "baz",
project: "foobar",
id: "baz",
scope: Scope{
Type: iam.ProjectScope,
},
@ -268,8 +177,8 @@ func Test_MarshallingAndCloning(t *testing.T) {
},
actionsBeingParsed: []string{"create", "read"},
},
jsonOutput: `{"actions":["create","read"],"id":"baz","project":"foobar","type":"group"}`,
canonicalString: `project=foobar;id=baz;type=group;actions=create,read`,
jsonOutput: `{"actions":["create","read"],"id":"baz","type":"group"}`,
canonicalString: `id=baz;type=group;actions=create,read`,
},
}
@ -310,19 +219,11 @@ func Test_Unmarshaling(t *testing.T) {
jsonErr: "invalid character 'w' looking for beginning of value",
},
{
name: "good project",
expected: Grant{
project: "foobar",
},
jsonInput: `{"project":"foobar"}`,
textInput: `project=foobar`,
},
{
name: "bad project",
jsonInput: `{"project":true}`,
jsonErr: `unable to interpret "project" as string`,
textInput: `project=`,
textErr: `segment "project=" not formatted correctly, missing value`,
name: "bad segment",
jsonInput: `{"id":true}`,
jsonErr: `unable to interpret "id" as string`,
textInput: `id=`,
textErr: `segment "id=" not formatted correctly, missing value`,
},
{
name: "good id",
@ -450,17 +351,21 @@ func Test_Parse(t *testing.T) {
input: "id=foobar;type=host-catalog;actions=createread",
err: `unknown action "createread"`,
},
{
name: "empty id and type",
input: "actions=create",
err: `"id" and "type" cannot both be empty`,
},
{
name: "good json",
input: `{"project":"proj","id":"foobar","type":"host-catalog","actions":["create","read"]}`,
input: `{"id":"foobar","type":"host-catalog","actions":["create","read"]}`,
expected: Grant{
scope: Scope{
Id: "proj",
Type: iam.ProjectScope,
Id: "scope",
Type: iam.OrganizationScope,
},
project: "proj",
id: "foobar",
typ: "host-catalog",
id: "foobar",
typ: "host-catalog",
actions: map[iam.Action]bool{
iam.ActionCreate: true,
iam.ActionRead: true,
@ -469,15 +374,14 @@ func Test_Parse(t *testing.T) {
},
{
name: "good text",
input: `project=proj;id=foobar;type=host-catalog;actions=create,read`,
input: `id=foobar;type=host-catalog;actions=create,read`,
expected: Grant{
scope: Scope{
Id: "proj",
Type: iam.ProjectScope,
Id: "scope",
Type: iam.OrganizationScope,
},
project: "proj",
id: "foobar",
typ: "host-catalog",
id: "foobar",
typ: "host-catalog",
actions: map[iam.Action]bool{
iam.ActionCreate: true,
iam.ActionRead: true,
@ -523,10 +427,6 @@ func Test_Parse(t *testing.T) {
require.Error(err)
assert.Equal("no scope ID provided", err.Error())
_, err = Parse(Scope{Id: "foobar", Type: iam.ProjectScope}, "", `project=foobar`)
require.Error(err)
assert.Equal("cannot specify a project in the grant when the scope is not an organization", err.Error())
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
grant, err := Parse(Scope{Id: "scope", Type: iam.OrganizationScope}, test.userId, test.input)

Loading…
Cancel
Save