refactor: remove team name from workspace permissions (#656)

internal/authz.go

@@ -47,7 +47,6 @@ type WorkspacePolicy struct {
 
 // WorkspacePermission binds a role to a team.
 type WorkspacePermission struct {
-	Team   string // team name
 	TeamID string
 	Role   rbac.Role
 }

internal/http/html/static/templates/content/workspace_edit.tmpl

@@ -201,15 +201,15 @@
             <td class="p-2">admin</td>
           </tr>
           <!-- iterate through existing role assignments -->
-          {{ range .Policy.Permissions }}
-            {{ if eq .Team "owners" }}
+          {{ range .Assigned }}
+            {{ if eq .Team.Name "owners" }}
               {{ continue }}
             {{ end }}
-            <tr class="border-b" id="permissions-{{ .Team }}">
-              <td class="p-2"><a href="{{ teamPath .TeamID }}">{{ .Team }}</a></td>
+            <tr class="border-b" id="permissions-{{ .Team.Name }}">
+              <td class="p-2"><a href="{{ teamPath .Team.ID }}">{{ .Team.Name }}</a></td>
               <td class="p-2">
                 <form class="" action="{{ setPermissionWorkspacePath $.Workspace.ID }}" method="POST">
-                  <input name="team_name" value="{{ .Team }}" type="hidden">
+                  <input name="team_id" value="{{ .Team.ID }}" type="hidden">
                   <select name="role" id="role-select">
                     {{ $currentRole := .Role.String }}
                     {{ range $.Roles }}
@@ -221,7 +221,7 @@
               </td>
               <td>
                 <form class="" action="{{ unsetPermissionWorkspacePath $.Workspace.ID }}" method="POST">
-                  <input name="team_name" value="{{ .Team }}" type="hidden">
+                  <input name="team_id" value="{{ .Team.ID }}" type="hidden">
                   <button class="btn-danger">Remove</button>
                 </form>
               </td>
@@ -230,10 +230,10 @@
           <tr class="border-b">
             <form id="permissions-add-form" class="horizontal-form" action="{{ setPermissionWorkspacePath .Workspace.ID }}" method="POST"></form>
             <td class="p-2">
-              <select form="permissions-add-form" name="team_name" id="permissions-add-select-team">
+              <select form="permissions-add-form" name="team_id" id="permissions-add-select-team">
                 <option value="">--team--</option>
                 {{ range .Unassigned }}
-                  <option value="{{ .Name }}">{{ .Name }}</option>
+                  <option value="{{ .ID }}">{{ .Name }}</option>
                 {{ end }}
               </select>
             </td>

internal/integration/organization_min_permissions_test.go

@@ -27,7 +27,7 @@ func TestIntegration_MinimumPermissions(t *testing.T) {
 
 	// Assign read role to guests team. Guests now receive a minimum set of
 	// permissions across the workspace's organization.
-	err = svc.SetPermission(ctx, ws.ID, guests.Name, rbac.WorkspaceReadRole)
+	err = svc.SetPermission(ctx, ws.ID, guests.ID, rbac.WorkspaceReadRole)
 	require.NoError(t, err)
 
 	// Guest should be able to get org

internal/integration/plan_permission_e2e_test.go

@@ -29,7 +29,7 @@ func TestIntegration_PlanPermission(t *testing.T) {
 	// engineer's team.
 	browser.Run(t, ctx, chromedp.Tasks{
 		createWorkspace(t, svc.Hostname(), org.Name, "my-test-workspace"),
-		addWorkspacePermission(t, svc.Hostname(), org.Name, "my-test-workspace", team.Name, "plan"),
+		addWorkspacePermission(t, svc.Hostname(), org.Name, "my-test-workspace", team.ID, "plan"),
 	})
 
 	// As engineer, run terraform init, and plan. This should succeed because

internal/integration/ui_helpers_test.go

@@ -177,7 +177,7 @@ func screenshot(t *testing.T, docPath ...string) chromedp.ActionFunc {
 
 // addWorkspacePermission adds a workspace permission via the UI, assigning
 // a role to a team.
-func addWorkspacePermission(t *testing.T, hostname, org, workspaceName, team, role string) chromedp.Tasks {
+func addWorkspacePermission(t *testing.T, hostname, org, workspaceName, teamID, role string) chromedp.Tasks {
 	t.Helper()
 
 	return chromedp.Tasks{
@@ -191,7 +191,7 @@ func addWorkspacePermission(t *testing.T, hostname, org, workspaceName, team, ro
 		matchText(t, "#permissions-owners td:last-child", "admin", chromedp.ByQuery),
 		// assign role to team
 		chromedp.SetValue(`//select[@id="permissions-add-select-role"]`, role),
-		chromedp.SetValue(`//select[@id="permissions-add-select-team"]`, team),
+		chromedp.SetValue(`//select[@id="permissions-add-select-team"]`, teamID),
 		// scroll to bottom so that permissions are visible in screenshot
 		chromedp.ActionFunc(func(ctx context.Context) error {
 			_, exp, err := runtime.Evaluate(`window.scrollTo(0,document.body.scrollHeight);`).Do(ctx)

internal/integration/web_test.go

@@ -47,7 +47,7 @@ func TestWeb(t *testing.T) {
 			matchText(t, "//div[@role='alert']", "team permissions updated"),
 		},
 		// add write permission on workspace to devops team
-		addWorkspacePermission(t, daemon.Hostname(), org.Name, "my-workspace", "devops", "write"),
+		addWorkspacePermission(t, daemon.Hostname(), org.Name, "my-workspace", team.ID, "write"),
 		// list users
 		chromedp.Tasks{
 			// go to org

internal/integration/workspace_permissions_service_test.go

@@ -18,17 +18,17 @@ func TestIntegration_WorkspacePermissionsService(t *testing.T) {
 	t.Run("set permission", func(t *testing.T) {
 		ws := svc.createWorkspace(t, ctx, org)
 		team := svc.createTeam(t, ctx, org)
-		err := svc.SetPermission(ctx, ws.ID, team.Name, rbac.WorkspacePlanRole)
+		err := svc.SetPermission(ctx, ws.ID, team.ID, rbac.WorkspacePlanRole)
 		require.NoError(t, err)
 	})
 
 	t.Run("unset permission", func(t *testing.T) {
 		ws := svc.createWorkspace(t, ctx, org)
 		team := svc.createTeam(t, ctx, org)
-		err := svc.SetPermission(ctx, ws.ID, team.Name, rbac.WorkspacePlanRole)
+		err := svc.SetPermission(ctx, ws.ID, team.ID, rbac.WorkspacePlanRole)
 		require.NoError(t, err)
 
-		err = svc.UnsetPermission(ctx, ws.ID, team.Name)
+		err = svc.UnsetPermission(ctx, ws.ID, team.ID)
 		require.NoError(t, err)
 
 		policy, err := svc.GetPolicy(ctx, ws.ID)
@@ -41,11 +41,11 @@ func TestIntegration_WorkspacePermissionsService(t *testing.T) {
 		scum := svc.createTeam(t, ctx, org)
 		skates := svc.createTeam(t, ctx, org)
 		cherries := svc.createTeam(t, ctx, org)
-		err := svc.SetPermission(ctx, ws.ID, scum.Name, rbac.WorkspaceAdminRole)
+		err := svc.SetPermission(ctx, ws.ID, scum.ID, rbac.WorkspaceAdminRole)
 		require.NoError(t, err)
-		err = svc.SetPermission(ctx, ws.ID, skates.Name, rbac.WorkspaceReadRole)
+		err = svc.SetPermission(ctx, ws.ID, skates.ID, rbac.WorkspaceReadRole)
 		require.NoError(t, err)
-		err = svc.SetPermission(ctx, ws.ID, cherries.Name, rbac.WorkspacePlanRole)
+		err = svc.SetPermission(ctx, ws.ID, cherries.ID, rbac.WorkspacePlanRole)
 		require.NoError(t, err)
 
 		got, err := svc.GetPolicy(ctx, ws.ID)
@@ -55,17 +55,14 @@ func TestIntegration_WorkspacePermissionsService(t *testing.T) {
 		assert.Equal(t, ws.ID, got.WorkspaceID)
 		assert.Equal(t, 3, len(got.Permissions))
 		assert.Contains(t, got.Permissions, internal.WorkspacePermission{
-			Team:   scum.Name,
 			TeamID: scum.ID,
 			Role:   rbac.WorkspaceAdminRole,
 		})
 		assert.Contains(t, got.Permissions, internal.WorkspacePermission{
-			Team:   skates.Name,
 			TeamID: skates.ID,
 			Role:   rbac.WorkspaceReadRole,
 		})
 		assert.Contains(t, got.Permissions, internal.WorkspacePermission{
-			Team:   cherries.Name,
 			TeamID: cherries.ID,
 			Role:   rbac.WorkspacePlanRole,
 		})

internal/integration/workspace_test.go

@@ -327,8 +327,8 @@ func TestWorkspace(t *testing.T) {
 
 		team1 := svc.createTeam(t, ctx, org)
 		team2 := svc.createTeam(t, ctx, org)
-		_ = svc.SetPermission(ctx, ws1.ID, team1.Name, rbac.WorkspacePlanRole)
-		_ = svc.SetPermission(ctx, ws2.ID, team2.Name, rbac.WorkspacePlanRole)
+		_ = svc.SetPermission(ctx, ws1.ID, team1.ID, rbac.WorkspacePlanRole)
+		_ = svc.SetPermission(ctx, ws2.ID, team2.ID, rbac.WorkspacePlanRole)
 		user1 := svc.createUser(t, user.WithTeams(team1, team2))
 		user2 := svc.createUser(t)
 

internal/integration/write_permission_e2e_test.go

@@ -28,7 +28,7 @@ func TestWritePermissionE2E(t *testing.T) {
 	// engineer's team.
 	browser.Run(t, ctx, chromedp.Tasks{
 		createWorkspace(t, svc.Hostname(), org.Name, "my-test-workspace"),
-		addWorkspacePermission(t, svc.Hostname(), org.Name, "my-test-workspace", team.Name, "write"),
+		addWorkspacePermission(t, svc.Hostname(), org.Name, "my-test-workspace", team.ID, "write"),
 	})
 
 	// As engineer, run terraform init

internal/sql/queries/workspace_permission.sql

@@ -3,31 +3,19 @@ INSERT INTO workspace_permissions (
     workspace_id,
     team_id,
     role
-) SELECT w.workspace_id, t.team_id, pggen.arg('role')
-    FROM teams t
-    JOIN organizations o ON t.organization_name = o.name
-    JOIN workspaces w ON w.organization_name = o.name
-    WHERE t.name = pggen.arg('team_name')
-    AND w.workspace_id = pggen.arg('workspace_id')
-ON CONFLICT (workspace_id, team_id) DO UPDATE SET role = pggen.arg('role')
-;
+) VALUES (
+    pggen.arg('workspace_id'),
+    pggen.arg('team_id'),
+    pggen.arg('role')
+) ON CONFLICT (workspace_id, team_id) DO UPDATE SET role = pggen.arg('role');
 
 -- name: FindWorkspacePermissionsByWorkspaceID :many
-SELECT
-    wp.role,
-    (t.*)::"teams" AS team
-FROM workspace_permissions wp
-JOIN teams t USING (team_id)
-WHERE wp.workspace_id = pggen.arg('workspace_id')
-;
+SELECT *
+FROM workspace_permissions
+WHERE workspace_id = pggen.arg('workspace_id');
 
 -- name: DeleteWorkspacePermissionByID :exec
 DELETE
-FROM workspace_permissions wp
-USING workspaces w, teams t
-WHERE wp.team_id = t.team_id
-AND wp.workspace_id = pggen.arg('workspace_id')
-AND w.workspace_id = wp.workspace_id
-AND w.organization_name = t.organization_name
-AND t.name = pggen.arg('team_name')
-;
+FROM workspace_permissions
+WHERE workspace_id = pggen.arg('workspace_id')
+AND team_id = pggen.arg('team_id');

internal/team/team.go

@@ -174,7 +174,7 @@ func (t *Team) CanAccessWorkspace(action rbac.Action, policy internal.WorkspaceP
 		return false
 	}
 	for _, perm := range policy.Permissions {
-		if t.Name == perm.Team {
+		if t.ID == perm.TeamID {
 			return perm.Role.IsAllowed(action)
 		}
 	}

internal/workspace/permissions_db.go

@@ -10,10 +10,10 @@ import (
 	"github.com/leg100/otf/internal/sql/pggen"
 )
 
-func (db *pgdb) SetWorkspacePermission(ctx context.Context, workspaceID, team string, role rbac.Role) error {
+func (db *pgdb) SetWorkspacePermission(ctx context.Context, workspaceID, teamID string, role rbac.Role) error {
 	_, err := db.Conn(ctx).UpsertWorkspacePermission(ctx, pggen.UpsertWorkspacePermissionParams{
 		WorkspaceID: sql.String(workspaceID),
-		TeamName:    sql.String(team),
+		TeamID:      sql.String(teamID),
 		Role:        sql.String(role.String()),
 	})
 	if err != nil {
@@ -54,8 +54,7 @@ func (db *pgdb) GetWorkspacePolicy(ctx context.Context, workspaceID string) (int
 			return internal.WorkspacePolicy{}, err
 		}
 		policy.Permissions = append(policy.Permissions, internal.WorkspacePermission{
-			Team:   perm.Team.Name.String,
-			TeamID: perm.Team.TeamID.String,
+			TeamID: perm.TeamID.String,
 			Role:   role,
 		})
 	}