Postgres revocation sql, beta mode (#1972)

builtin/logical/mysql/backend_test.go

@@ -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,
 		}
 	}
 
@@ -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';
 `

builtin/logical/mysql/path_roles.go

@@ -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.",
 			},
@@ -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
 }
@@ -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),
@@ -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"`

builtin/logical/mysql/secret_creds.go

@@ -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}}'@'%'
 `
@@ -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,
@@ -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{}
@@ -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

builtin/logical/postgresql/backend_test.go

@@ -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,
@@ -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,
@@ -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,
 					},
 				},
 			})
@@ -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";
@@ -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}};
+`

builtin/logical/postgresql/path_role_create.go

@@ -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

builtin/logical/postgresql/path_roles.go

@@ -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{
@@ -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(
@@ -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
@@ -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 = `