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

Postgres revocation sql, beta mode #1972

Merged
merged 3 commits into from Oct 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions builtin/logical/mysql/backend_test.go
Expand Up @@ -261,8 +261,8 @@ func testAccStepRole(t *testing.T, wildCard bool) logicaltest.TestStep {
}
} else {
pathData = map[string]interface{}{
"sql": testRoleHost,
"revoke_sql": testRevokeSQL,
"sql": testRoleHost,
"revocation_sql": testRevocationSQL,
}
}

Expand Down Expand Up @@ -362,7 +362,7 @@ const testRoleHost = `
CREATE USER '{{name}}'@'10.1.1.2' IDENTIFIED BY '{{password}}';
GRANT SELECT ON *.* TO '{{name}}'@'10.1.1.2';
`
const testRevokeSQL = `
const testRevocationSQL = `
REVOKE ALL PRIVILEGES, GRANT OPTION FROM '{{name}}'@'10.1.1.2';
DROP USER '{{name}}'@'10.1.1.2';
`
10 changes: 5 additions & 5 deletions builtin/logical/mysql/path_roles.go
Expand Up @@ -37,7 +37,7 @@ func pathRoles(b *backend) *framework.Path {
Description: "SQL string to create a user. See help for more info.",
},

"revoke_sql": {
"revocation_sql": {
Type: framework.TypeString,
Description: "SQL string to revoke a user. See help for more info.",
},
Expand Down Expand Up @@ -117,8 +117,8 @@ func (b *backend) pathRoleRead(

return &logical.Response{
Data: map[string]interface{}{
"sql": role.SQL,
"revoke_sql": role.RevokeSQL,
"sql": role.SQL,
"revocation_sql": role.RevocationSQL,
},
}, nil
}
Expand Down Expand Up @@ -165,7 +165,7 @@ func (b *backend) pathRoleCreate(
// Store it
entry, err := logical.StorageEntryJSON("role/"+name, &roleEntry{
SQL: sql,
RevokeSQL: data.Get("revoke_sql").(string),
RevocationSQL: data.Get("revocation_sql").(string),
UsernameLength: data.Get("username_length").(int),
DisplaynameLength: data.Get("displayname_length").(int),
RolenameLength: data.Get("rolename_length").(int),
Expand All @@ -181,7 +181,7 @@ func (b *backend) pathRoleCreate(

type roleEntry struct {
SQL string `json:"sql" mapstructure:"sql" structs:"sql"`
RevokeSQL string `json:"revoke_sql" mapstructure:"revoke_sql" structs:"revoke_sql"`
RevocationSQL string `json:"revocation_sql" mapstructure:"revocation_sql" structs:"revocation_sql"`
UsernameLength int `json:"username_length" mapstructure:"username_length" structs:"username_length"`
DisplaynameLength int `json:"displayname_length" mapstructure:"displayname_length" structs:"displayname_length"`
RolenameLength int `json:"rolename_length" mapstructure:"rolename_length" structs:"rolename_length"`
Expand Down
17 changes: 6 additions & 11 deletions builtin/logical/mysql/secret_creds.go
Expand Up @@ -11,12 +11,12 @@ import (

const SecretCredsType = "creds"

// defaultRevokeSQL is a default SQL statement for revoking a user. Revoking
// defaultRevocationSQL is a default SQL statement for revoking a user. Revoking
// permissions for the user is done before the drop, because MySQL explicitly
// documents that open user connections will not be closed. By revoking all
// grants, at least we ensure that the open connection is useless. Dropping the
// user will only affect the next connection.
const defaultRevokeSQL = `
const defaultRevocationSQL = `
REVOKE ALL PRIVILEGES, GRANT OPTION FROM '{{name}}'@'%';
DROP USER '{{name}}'@'%'
`
Expand All @@ -34,11 +34,6 @@ func secretCreds(b *backend) *framework.Secret {
Type: framework.TypeString,
Description: "Password",
},

"role": &framework.FieldSchema{
Type: framework.TypeString,
Description: "Role",
},
},

Renew: b.secretCredsRenew,
Expand Down Expand Up @@ -93,10 +88,10 @@ func (b *backend) secretCredsRevoke(
}

// Use a default SQL statement for revocation if one cannot be fetched from the role
revokeSQL := defaultRevokeSQL
revocationSQL := defaultRevocationSQL

if role != nil && role.RevokeSQL != "" {
revokeSQL = role.RevokeSQL
if role != nil && role.RevocationSQL != "" {
revocationSQL = role.RevocationSQL
} else {
if resp == nil {
resp = &logical.Response{}
Expand All @@ -111,7 +106,7 @@ func (b *backend) secretCredsRevoke(
}
defer tx.Rollback()

for _, query := range strutil.ParseArbitraryStringSlice(revokeSQL, ";") {
for _, query := range strutil.ParseArbitraryStringSlice(revocationSQL, ";") {
query = strings.TrimSpace(query)
if len(query) == 0 {
continue
Expand Down
60 changes: 58 additions & 2 deletions builtin/logical/postgresql/backend_test.go
Expand Up @@ -233,6 +233,39 @@ func TestBackend_roleReadOnly(t *testing.T) {
})
}

func TestBackend_roleReadOnly_revocationSQL(t *testing.T) {
config := logical.TestBackendConfig()
config.StorageView = &logical.InmemStorage{}
b, err := Factory(config)
if err != nil {
t.Fatal(err)
}

cid, connURL := prepareTestContainer(t, config.StorageView, b)
if cid != "" {
defer cleanupTestContainer(t, cid)
}
connData := map[string]interface{}{
"connection_url": connURL,
}

logicaltest.Test(t, logicaltest.TestCase{
Backend: b,
Steps: []logicaltest.TestStep{
testAccStepConfig(t, connData, false),
testAccStepCreateRoleWithRevocationSQL(t, "web", testRole, defaultRevocationSQL, false),
testAccStepCreateRoleWithRevocationSQL(t, "web-readonly", testReadOnlyRole, defaultRevocationSQL, false),
testAccStepReadRole(t, "web-readonly", testReadOnlyRole),
testAccStepCreateTable(t, b, config.StorageView, "web", connURL),
testAccStepReadCreds(t, b, config.StorageView, "web-readonly", connURL),
testAccStepDropTable(t, b, config.StorageView, "web", connURL),
testAccStepDeleteRole(t, "web-readonly"),
testAccStepDeleteRole(t, "web"),
testAccStepReadRole(t, "web-readonly", ""),
},
})
}

func testAccStepConfig(t *testing.T, d map[string]interface{}, expectError bool) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.UpdateOperation,
Expand Down Expand Up @@ -273,6 +306,18 @@ func testAccStepCreateRole(t *testing.T, name string, sql string, expectFail boo
}
}

func testAccStepCreateRoleWithRevocationSQL(t *testing.T, name, sql, revocationSQL string, expectFail bool) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: path.Join("roles", name),
Data: map[string]interface{}{
"sql": sql,
"revocation_sql": revocationSQL,
},
ErrorOk: expectFail,
}
}

func testAccStepDeleteRole(t *testing.T, name string) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.DeleteOperation,
Expand Down Expand Up @@ -341,6 +386,7 @@ func testAccStepReadCreds(t *testing.T, b logical.Backend, s logical.Storage, na
InternalData: map[string]interface{}{
"secret_type": "creds",
"username": d.Username,
"role": name,
},
},
})
Expand Down Expand Up @@ -543,7 +589,8 @@ GRANT CONNECT ON DATABASE "postgres" TO "{{name}}";
`

var testBlockStatementRoleSlice = []string{
`DO $$
`
DO $$
BEGIN
IF NOT EXISTS (SELECT * FROM pg_catalog.pg_roles WHERE rolname='foo-role') THEN
CREATE ROLE "foo-role";
Expand All @@ -556,9 +603,18 @@ BEGIN
GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA foo TO "foo-role";
END IF;
END
$$`,
$$
`,
`CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';`,
`GRANT "foo-role" TO "{{name}}";`,
`ALTER ROLE "{{name}}" SET search_path = foo;`,
`GRANT CONNECT ON DATABASE "postgres" TO "{{name}}";`,
}

const defaultRevocationSQL = `
REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA public FROM {{name}};
REVOKE ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public FROM {{name}};
REVOKE USAGE ON SCHEMA public FROM {{name}};

DROP ROLE IF EXISTS {{name}};
`
1 change: 1 addition & 0 deletions builtin/logical/postgresql/path_role_create.go
Expand Up @@ -140,6 +140,7 @@ func (b *backend) pathRoleCreateRead(
"password": password,
}, map[string]interface{}{
"username": username,
"role": name,
})
resp.Secret.TTL = lease.Lease
return resp, nil
Expand Down
27 changes: 21 additions & 6 deletions builtin/logical/postgresql/path_roles.go
Expand Up @@ -26,15 +26,20 @@ func pathRoles(b *backend) *framework.Path {
return &framework.Path{
Pattern: "roles/" + framework.GenericNameRegex("name"),
Fields: map[string]*framework.FieldSchema{
"name": &framework.FieldSchema{
"name": {
Type: framework.TypeString,
Description: "Name of the role.",
},

"sql": &framework.FieldSchema{
"sql": {
Type: framework.TypeString,
Description: "SQL string to create a user. See help for more info.",
},

"revocation_sql": {
Type: framework.TypeString,
Description: "SQL string to revoke a user. This is in beta; use with caution.",
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
Expand Down Expand Up @@ -85,11 +90,19 @@ func (b *backend) pathRoleRead(
return nil, nil
}

return &logical.Response{
resp := &logical.Response{
Data: map[string]interface{}{
"sql": role.SQL,
},
}, nil
}

// TODO: This is separate because this is in beta in 0.6.2, so we don't
// want it to show up in the general case.
if role.RevocationSQL != "" {
resp.Data["revocation_sql"] = role.RevocationSQL
}

return resp, nil
}

func (b *backend) pathRoleList(
Expand Down Expand Up @@ -134,7 +147,8 @@ func (b *backend) pathRoleCreate(

// Store it
entry, err := logical.StorageEntryJSON("role/"+name, &roleEntry{
SQL: sql,
SQL: sql,
RevocationSQL: data.Get("revocation_sql").(string),
})
if err != nil {
return nil, err
Expand All @@ -147,7 +161,8 @@ func (b *backend) pathRoleCreate(
}

type roleEntry struct {
SQL string `json:"sql"`
SQL string `json:"sql" mapstructure:"sql" structs:"sql"`
RevocationSQL string `json:"revocation_sql" mapstructure:"revocation_sql" structs:"revocation_sql"`
}

const pathRoleHelpSyn = `
Expand Down