Support accessing non-tenanted/instanced tables with tenanted/instanced roles

Author: krishnamiriyala

pkg/authorizer/authorizer_test.go

@@ -47,7 +47,7 @@ func testGettingOrgFromContext(t *testing.T, authorizer Authorizer) {
 
 	ctx = metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ORGID, PEPSI))
 	orgId, err := authorizer.GetOrgFromContext(ctx)
-	assert.Nil(err)
+	assert.NoError(err)
 	assert.Equal(PEPSI, orgId)
 }
 
@@ -63,41 +63,51 @@ func TestGettingMatchingDbRoleWithMetadataBasedAuthorizer(t *testing.T) {
 
 	gCtx := authorizer.GetDefaultOrgAdminContext()
 	dbRole, err := authorizer.GetMatchingDbRole(gCtx, "appUser", "app")
-	assert.Nil(err)
+	assert.NoError(err)
 	assert.Equal(dbrole.TENANT_WRITER, dbRole)
 
 	aCtx := metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ORGID, PEPSI, METADATA_KEY_ROLE, METADATA_ROLE_SERVICE_ADMIN))
 	dbRole, err = authorizer.GetMatchingDbRole(aCtx, "appUser", "app")
-	assert.Nil(err)
+	assert.NoError(err)
 	assert.Equal(dbrole.WRITER, dbRole)
 
 	uCtx := metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ORGID, PEPSI, METADATA_KEY_ROLE, METADATA_ROLE_SERVICE_AUDITOR))
 	dbRole, err = authorizer.GetMatchingDbRole(uCtx, "appUser", "app")
-	assert.Nil(err)
+	assert.NoError(err)
 	assert.Equal(dbrole.READER, dbRole)
 
 	wCtx := metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ORGID, PEPSI, METADATA_KEY_ROLE, METADATA_ROLE_ADMIN))
 	dbRole, err = authorizer.GetMatchingDbRole(wCtx, "appUser", "app")
-	assert.Nil(err)
+	assert.NoError(err)
 	assert.Equal(dbrole.TENANT_WRITER, dbRole)
 
 	rCtx := metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ORGID, PEPSI, METADATA_KEY_ROLE, METADATA_ROLE_AUDITOR))
 	dbRole, err = authorizer.GetMatchingDbRole(rCtx, "appUser", "app")
-	assert.Nil(err)
+	assert.NoError(err)
 	assert.Equal(dbrole.TENANT_READER, dbRole)
 }
 
 /*
 Checks if updating role mapping in MetadataBasedAuthorizer works.
 */
 func TestConfiguringMetadataBasedAuthorizer(t *testing.T) {
-	authorizer := &MetadataBasedAuthorizer{}
+	assert := assert.New(t)
 
+	authorizer := &MetadataBasedAuthorizer{}
 	newRoleMapping := map[string]map[string]dbrole.DbRole{
 		"app": {
-			"SERVICE_ADMIN": dbrole.WRITER,
+			"SERVICE_ADMIN": dbrole.READER,
 		},
 	}
-
 	authorizer.Configure("app", newRoleMapping["app"])
+
+	ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ROLE, "SERVICE_ADMIN"))
+	dbRole, err := authorizer.GetMatchingDbRole(ctx, "app")
+	assert.NoError(err)
+	assert.Equal(dbrole.READER, dbRole)
+
+	ctx = metadata.NewIncomingContext(context.Background(), metadata.Pairs(METADATA_KEY_ROLE, "SERVICE_OP"))
+	dbRole, err = authorizer.GetMatchingDbRole(ctx, "app")
+	assert.NoError(err)
+	assert.Equal(dbrole.TENANT_READER, dbRole)
 }

pkg/datastore/database.go

@@ -622,15 +622,15 @@ func (db *relationalDb) RegisterHelper(_ context.Context, roleMapping map[string
 // This function considers multi-tenancy and multi-instance configurations to determine
 // the users relevant to the given table.
 func (db *relationalDb) GetDbUsers(record Record, tableName string) []dbUserSpec {
-	users := getDbUsers(tableName, false, false)
-	if IsMultiTenanted(record, tableName) {
-		users = append(users, getDbUsers(tableName, true, false)...)
-		if IsMultiInstanced(record, tableName, db.instancer != nil) {
-			users = append(users, getDbUsers(tableName, true, true)...)
-		}
-	} else if IsMultiInstanced(record, tableName, db.instancer != nil) {
-		users = append(users, getDbUsers(tableName, false, true)...)
-	}
+	tableTenanted := IsMultiTenanted(record, tableName)
+	tableInstanced := IsMultiInstanced(record, tableName, db.instancer != nil)
+	// Create all users but with conditions set based on whether the role and the table configuration
+	// Tenant role without tenant column would not set the condition for tenant
+	// Insntacer role without instance column would not set the condition for instancer
+	users := getDbUsers(tableName, false, false, false, false)
+	users = append(users, getDbUsers(tableName, true, false, tableTenanted, false)...)
+	users = append(users, getDbUsers(tableName, false, true, false, tableInstanced)...)
+	users = append(users, getDbUsers(tableName, true, true, tableTenanted, tableInstanced)...)
 	return users
 }
 

pkg/datastore/datastore_test.go

@@ -729,6 +729,6 @@ func TestTransactions(t *testing.T) {
 	assert.NoError(err)
 
 	testSingleTableTransactions(t, ds, ps)
-	// testMultiTableTransactions(t, ds, ps)
-	// testMultiProtoTransactions(t, ds, ps)
+	testMultiTableTransactions(t, ds, ps)
+	testMultiProtoTransactions(t, ds, ps)
 }

pkg/datastore/db_user.go

@@ -73,7 +73,7 @@ func (spec dbUserSpec) String() string {
 }
 
 func getDbUser(dbRole dbrole.DbRole) dbUserSpec {
-	for _, spec := range getAllDbUsers("ANY") {
+	for _, spec := range getAllDbUsers() {
 		if spec.username == dbRole {
 			return spec
 		}
@@ -86,35 +86,40 @@ Generates specifications of 2 DB users:
 - user with read-only access (to specific org/instance)
 - user with read-write access (to specific org/instance)
 All the users have additional conditions to restrict access to records
-belonging to specific instance, if withInstanceIdCheck is set.
+belonging to specific instance, if instancerRole is set.
 */
-func getDbUsers(tableName string, withTenantIdCheck, withInstanceIdCheck bool) []dbUserSpec {
+func getDbUsers(tableName string, tenantRole, instancerRole bool, isTableTenanted, isTableInstanced bool) []dbUserSpec {
 	readerCommands := []string{"SELECT"}
 	writerCommands := []string{"SELECT", "INSERT", "UPDATE", "DELETE"}
 
 	tenantCond := COLUMN_ORGID + " = current_setting('" + DbConfigOrgId + "')"
 	instanceCond := COLUMN_INSTANCEID + " = current_setting('" + DbConfigInstanceId + "')"
 	tenantInstanceCond := tenantCond + " AND " + instanceCond
 
-	cond := "true"
 	rwUser := dbrole.WRITER
 	rUser := dbrole.READER
-
 	switch {
-	case withInstanceIdCheck && withTenantIdCheck:
-		cond = tenantInstanceCond
+	case tenantRole && instancerRole:
 		rwUser = dbrole.TENANT_INSTANCE_WRITER
 		rUser = dbrole.TENANT_INSTANCE_READER
-	case withTenantIdCheck:
-		cond = tenantCond
+	case tenantRole:
 		rwUser = dbrole.TENANT_WRITER
 		rUser = dbrole.TENANT_READER
-	case withInstanceIdCheck:
-		cond = instanceCond
+	case instancerRole:
 		rwUser = dbrole.INSTANCE_WRITER
 		rUser = dbrole.INSTANCE_READER
 	}
 
+	cond := "true"
+	switch {
+	case isTableInstanced && isTableTenanted:
+		cond = tenantInstanceCond
+	case isTableTenanted:
+		cond = tenantCond
+	case isTableInstanced:
+		cond = instanceCond
+	}
+
 	writer := dbUserSpec{
 		username:         rwUser,
 		commands:         writerCommands,
@@ -134,8 +139,8 @@ func getDbUsers(tableName string, withTenantIdCheck, withInstanceIdCheck bool) [
 		dbUsers[i].password = getPassword(string(dbUsers[i].username))
 		dbUsers[i].policyName = getRlsPolicyName(string(dbUsers[i].username), tableName)
 	}
-	TRACE("Returning DB user specs for table %q:\n\twithTenantIdCheck - %t\n\twithInstanceIdCheck -  %t\n\tdbUsers - %+v\n",
-		tableName, withTenantIdCheck, withInstanceIdCheck, dbUsers)
+	TRACE("Returning DB user specs for table %q:\n\t[roleT=%s, roleI=%s, tableT=%s, tableI=%s]\n\tdbUsers - %+v\n",
+		tableName, tenantRole, instancerRole, isTableTenanted, isTableInstanced, dbUsers)
 	return dbUsers
 }
 
@@ -147,11 +152,12 @@ func getPassword(username string) string {
 	return strconv.FormatInt(int64(h.Sum32()), 16)
 }
 
-func getAllDbUsers(tableName string) []dbUserSpec {
+func getAllDbUsers() []dbUserSpec {
+	tableName := "ANY" // The returned user specs are for creating users only not policies
 	allDbUsers := make([]dbUserSpec, 0)
-	allDbUsers = append(allDbUsers, getDbUsers(tableName, false, false)...)
-	allDbUsers = append(allDbUsers, getDbUsers(tableName, false, true)...)
-	allDbUsers = append(allDbUsers, getDbUsers(tableName, true, false)...)
-	allDbUsers = append(allDbUsers, getDbUsers(tableName, true, true)...)
+	allDbUsers = append(allDbUsers, getDbUsers(tableName, false, false, true, true)...)
+	allDbUsers = append(allDbUsers, getDbUsers(tableName, false, true, false, true)...)
+	allDbUsers = append(allDbUsers, getDbUsers(tableName, true, false, true, false)...)
+	allDbUsers = append(allDbUsers, getDbUsers(tableName, true, true, true, true)...)
 	return allDbUsers
 }

pkg/datastore/helper.go

@@ -134,7 +134,7 @@ func FromConfig(l *logrus.Entry, a authorizer.Authorizer, instancer authorizer.I
 			}
 
 			// Create Users when the MAIN connection to DB is established
-			for _, dbUserSpec := range getAllDbUsers("ANY") {
+			for _, dbUserSpec := range getAllDbUsers() {
 				stmt := getCreateUserStmt(string(dbUserSpec.username), dbUserSpec.password)
 				if tx := db.gormDBMap[dbrole.MAIN].Exec(stmt); tx.Error != nil {
 					err = ErrExecutingSqlStmt.Wrap(tx.Error).WithValue(SQL_STMT, stmt).WithValue(DB_NAME, db.dbName)

pkg/datastore/sql_struct.go

@@ -252,7 +252,7 @@ func GetTableName(x interface{}) (tableName string) {
 
 // Generates RLS-policy name based on database role/user and table name.
 func getRlsPolicyName(username string, tableName string) string {
-	policyName := strings.ToLower(username + "_" + tableName + "_v2")
+	policyName := strings.ToLower(username + "_" + tableName + "_policy_v2")
 	policyName = strings.ReplaceAll(policyName, "\"", "")
 	return policyName
 }

pkg/datastore/transaction_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/vmware-labs/multi-tenant-persistence-for-saas/pkg/datastore"
 	"github.com/vmware-labs/multi-tenant-persistence-for-saas/pkg/protostore"
 	. "github.com/vmware-labs/multi-tenant-persistence-for-saas/test"
+	"github.com/vmware-labs/multi-tenant-persistence-for-saas/test/pb"
 )
 
 func testSingleTableTransactions(t *testing.T, ds datastore.DataStore, ps protostore.ProtoStore) {
@@ -118,7 +119,6 @@ func testSingleTableTransactions(t *testing.T, ds datastore.DataStore, ps protos
 	assert.NoError(tx.Error)
 }
 
-/* FIXME(miriyalak): Enable test with non conflicting role bindings
 func testMultiTableTransactions(t *testing.T, ds datastore.DataStore, ps protostore.ProtoStore) {
 	t.Helper()
 	assert := assert.New(t)
@@ -329,4 +329,3 @@ func testMultiProtoTransactions(t *testing.T, ds datastore.DataStore, ps protost
 	assert.NoError(err)
 	t.Log("Purging pb.Disk after soft delete succeeded")
 }
-*/