Make sure we don't break the role editor for some preset roles, add support for GH organizations (#51608)

* Add more fields to the role editor

* Review

* Make sure we don't break the role editor for some preset roles

Adds a test that verifies that the built-in roles, as pulled from our Go
source codes, can be unambiguously translated to the standard editor
model. Also adds one currently missing field (`github_organizations`).

* Fix type-check

* Also remove the now redundant comments in presets.go

* Post-merge fix

* Review

* Increase the test timeout

* Increase the test timeout even more

* Attempt to work around the problem with compiler output

* Switch from testPathIgnorePatterns to watchPathIgnorePatterns

* Add newline to end of Makefile

* Wrap preset role tests into a separate describe block

* Change of approach: check in the generated file

* Revert an accidental change

* One more update to the preset roles

* Final touches

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

Makefile

@@ -1843,3 +1843,8 @@ create-github-release:
 .PHONY: go-mod-tidy-all
 go-mod-tidy-all:
 	find . -type "f" -name "go.mod" -execdir go mod tidy \;
+
+.PHONY: dump-preset-roles
+dump-preset-roles:
+	GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG) go run ./build.assets/dump-preset-roles/main.go
+	pnpm test web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.test.ts

build.assets/dump-preset-roles/main.go

@@ -0,0 +1,63 @@
+// Teleport
+// Copyright (C) 2025 Gravitational, Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+// A tool that dumps preset roles in a JSON file that will later be used in
+// TypeScript tests to make sure that the standard role editor can
+// unambiguously represent a preset role.
+
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"log"
+	"os"
+
+	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/services"
+)
+
+const filePath = "gen/preset-roles.json"
+
+func main() {
+	access := services.NewPresetAccessRole()
+	editor := services.NewPresetEditorRole()
+	auditor := services.NewPresetAuditorRole()
+
+	rolesByName := map[string]types.Role{
+		access.GetName():  access,
+		editor.GetName():  editor,
+		auditor.GetName(): auditor,
+	}
+
+	for _, r := range rolesByName {
+		err := services.CheckAndSetDefaults(r)
+		if err != nil {
+			log.Fatalf("Could not set default values: %s", err)
+		}
+	}
+
+	rolesJSON, err := json.Marshal(rolesByName)
+	if err != nil {
+		log.Fatalf("Could not marshal preset roles as JSON: %s", err)
+	}
+
+	if err = os.WriteFile(filePath, rolesJSON, 0744); err != nil {
+		log.Fatalf("Could not write JSON for preset roles: %s", err)
+	}
+
+	fmt.Printf("Successfully recreated %s\n", filePath)
+}

lib/services/presets_test.go

@@ -20,6 +20,8 @@ package services
 
 import (
 	"context"
+	"encoding/json"
+	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -782,3 +784,49 @@ func TestAddRoleDefaults(t *testing.T) {
 		})
 	}
 }
+
+func TestPresetRolesDumped(t *testing.T) {
+	// This test ensures that the most recent version of selected preset roles
+	// has been correctly dumped to a generated JSON file. We use a generated
+	// file, because it's simpler to load it from a TypeScript test this way,
+	// rather than calling a Go binary.
+
+	// First, get the required roles, as defined in our codebase, and set their
+	// defaults.
+	access := NewPresetAccessRole()
+	editor := NewPresetEditorRole()
+	auditor := NewPresetAuditorRole()
+	rolesByName := map[string]types.Role{
+		access.GetName():  access,
+		editor.GetName():  editor,
+		auditor.GetName(): auditor,
+	}
+	for _, r := range rolesByName {
+		err := CheckAndSetDefaults(r)
+		require.NoError(t, err)
+	}
+
+	// Next, dump them all to JSON and parse them back again. This step is
+	// necessary, because unmarshaling isn't precisely the opposite of
+	// marshaling, and comparing raw roles to their unmarshaled counterparts
+	// still lead to some discrepancies. We can't also directly compare JSON
+	// blobs, as it's hard to reason whether this process is entirely
+	// deterministic.
+	bytes, err := json.Marshal(rolesByName)
+	require.NoError(t, err)
+	var recreatedRolesByName map[string]types.RoleV6
+	err = json.Unmarshal(bytes, &recreatedRolesByName)
+	require.NoError(t, err)
+
+	// Read the roles defined in the generated file.
+	bytes, err = os.ReadFile("../../gen/preset-roles.json")
+	require.NoError(t, err)
+	var rolesFromFile map[string]types.RoleV6
+	err = json.Unmarshal(bytes, &rolesFromFile)
+	require.NoError(t, err)
+
+	// Finally, compare the roles.
+	require.Equal(t, rolesFromFile, recreatedRolesByName,
+		"The dumped preset roles differ from their representation in code. Please run:\n"+
+			"make dump-preset-roles")
+}

web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.test.tsx

@@ -27,6 +27,7 @@ import { RoleVersion } from 'teleport/services/resources';
 import {
   AppAccessSection,
   DatabaseAccessSection,
+  GitHubOrganizationAccessSection,
   KubernetesAccessSection,
   ServerAccessSection,
   WindowsDesktopAccessSection,
@@ -35,6 +36,7 @@ import {
   AppAccess,
   DatabaseAccess,
   defaultRoleVersion,
+  GitHubOrganizationAccess,
   KubernetesAccess,
   newResourceAccess,
   ServerAccess,
@@ -42,6 +44,7 @@ import {
 } from './standardmodel';
 import { StatefulSection } from './StatefulSection';
 import {
+  GitHubOrganizationAccessValidationResult,
   ResourceAccessValidationResult,
   validateResourceAccess,
 } from './validation';
@@ -518,6 +521,46 @@ describe('WindowsDesktopAccessSection', () => {
   });
 });
 
+describe('GitHubOrganizationAccessSection', () => {
+  const setup = () => {
+    const onChange = jest.fn();
+    let validator: Validator;
+    render(
+      <StatefulSection<
+        GitHubOrganizationAccess,
+        GitHubOrganizationAccessValidationResult
+      >
+        component={GitHubOrganizationAccessSection}
+        defaultValue={newResourceAccess('git_server', defaultRoleVersion)}
+        onChange={onChange}
+        validatorRef={v => {
+          validator = v;
+        }}
+        validate={validateResourceAccess}
+      />
+    );
+    return { user: userEvent.setup(), onChange, validator };
+  };
+
+  test('editing', async () => {
+    const { onChange } = setup();
+    await selectEvent.create(
+      screen.getByLabelText('Organization Names'),
+      'illuminati',
+      {
+        createOptionText: 'Organization: illuminati',
+      }
+    );
+    expect(onChange).toHaveBeenLastCalledWith({
+      kind: 'git_server',
+      organizations: [
+        expect.objectContaining({ value: '{{internal.github_orgs}}' }),
+        expect.objectContaining({ label: 'illuminati', value: 'illuminati' }),
+      ],
+    } as GitHubOrganizationAccess);
+  });
+});
+
 const reactSelectValueContainer = (input: HTMLInputElement) =>
   // eslint-disable-next-line testing-library/no-node-access
   input.closest('.react-select__value-container');

web/packages/teleport/src/Roles/RoleEditor/StandardEditor/Resources.tsx

@@ -41,6 +41,7 @@ import { SectionBox, SectionProps, SectionPropsWithDispatch } from './sections';
 import {
   AppAccess,
   DatabaseAccess,
+  GitHubOrganizationAccess,
   KubernetesAccess,
   kubernetesResourceKindOptions,
   KubernetesResourceModel,
@@ -54,6 +55,7 @@ import {
 import {
   AppAccessValidationResult,
   DatabaseAccessValidationResult,
+  GitHubOrganizationAccessValidationResult,
   KubernetesAccessValidationResult,
   KubernetesResourceValidationResult,
   ResourceAccessValidationResult,
@@ -140,6 +142,7 @@ const allResourceAccessKinds: ResourceAccessKind[] = [
   'app',
   'db',
   'windows_desktop',
+  'git_server',
 ];
 
 /** Maps resource access kind to UI component configuration. */
@@ -176,6 +179,11 @@ export const resourceAccessSections: Record<
     tooltip: 'Configures access to Windows desktops',
     component: WindowsDesktopAccessSection,
   },
+  git_server: {
+    title: 'GitHub Organizations',
+    tooltip: 'Configures access to GitHub organizations and their repositories',
+    component: GitHubOrganizationAccessSection,
+  },
 };
 
 /**
@@ -595,6 +603,32 @@ export function WindowsDesktopAccessSection({
   );
 }
 
+export function GitHubOrganizationAccessSection({
+  value,
+  isProcessing,
+  onChange,
+}: SectionProps<
+  GitHubOrganizationAccess,
+  GitHubOrganizationAccessValidationResult
+>) {
+  return (
+    <FieldSelectCreatable
+      isMulti
+      label="Organization Names"
+      toolTipContent="A list of GitHub organization names that this role is allowed to use"
+      placeholder="Type an organization name and press Enter"
+      isDisabled={isProcessing}
+      formatCreateLabel={label => `Organization: ${label}`}
+      components={{
+        DropdownIndicator: null,
+      }}
+      openMenuOnClick={false}
+      value={value.organizations}
+      onChange={organizations => onChange?.({ ...value, organizations })}
+    />
+  );
+}
+
 // TODO(bl-nero): This should ideally use tonal neutral 1 from the opposite
 // theme as background.
 const MarkInverse = styled(Mark)`

web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.test.tsx

@@ -63,6 +63,7 @@ test('adding and removing sections', async () => {
     'Applications',
     'Databases',
     'Windows Desktops',
+    'GitHub Organizations',
   ]);
 
   await user.click(screen.getByRole('menuitem', { name: 'Servers' }));
@@ -76,6 +77,7 @@ test('adding and removing sections', async () => {
     'Applications',
     'Databases',
     'Windows Desktops',
+    'GitHub Organizations',
   ]);
 
   await user.click(screen.getByRole('menuitem', { name: 'Kubernetes' }));

web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.test.ts

@@ -20,6 +20,7 @@ import { Label as UILabel } from 'teleport/components/LabelsInput/LabelsInput';
 import {
   CreateDBUserMode,
   CreateHostUserMode,
+  GitHubPermission,
   KubernetesResource,
   Labels,
   RequireMFAType,
@@ -31,6 +32,7 @@ import {
   SSHPortForwarding,
 } from 'teleport/services/resources';
 
+import presetRoles from '../../../../../../../gen/preset-roles.json';
 import {
   createDBUserModeOptionsMap,
   createHostUserModeOptionsMap,
@@ -252,6 +254,31 @@ describe.each<{ name: string; role: Role; model: RoleEditorModel }>([
     },
   },
 
+  {
+    name: 'GitHub organization',
+    role: {
+      ...minimalRole(),
+      spec: {
+        ...minimalRole().spec,
+        allow: {
+          github_permissions: [{ orgs: ['illuminati', 'reptilians'] }],
+        },
+      },
+    },
+    model: {
+      ...minimalRoleModel(),
+      resources: [
+        {
+          kind: 'git_server',
+          organizations: [
+            { label: 'illuminati', value: 'illuminati' },
+            { label: 'reptilians', value: 'reptilians' },
+          ],
+        },
+      ],
+    },
+  },
+
   {
     name: 'Options object',
     role: {
@@ -564,6 +591,31 @@ describe('roleToRoleEditorModel', () => {
       },
     },
 
+    {
+      name: 'unknown field in github_permissions',
+      role: {
+        ...minRole,
+        spec: {
+          ...minRole.spec,
+          allow: {
+            ...minRole.spec.allow,
+            github_permissions: [
+              { orgs: ['foo'], unknownField: 123 } as GitHubPermission,
+            ],
+          },
+        },
+      },
+      model: {
+        ...roleModelWithReset,
+        resources: [
+          {
+            kind: 'git_server',
+            organizations: [{ label: 'foo', value: 'foo' }],
+          },
+        ],
+      },
+    },
+
     {
       name: 'unknown fields in Rule',
       role: {
@@ -992,6 +1044,40 @@ describe('roleToRoleEditorModel', () => {
       ],
     } as RoleEditorModel);
   });
+
+  test('multiple github_permissions', () => {
+    expect(
+      roleToRoleEditorModel({
+        ...minimalRole(),
+        spec: {
+          ...minimalRole().spec,
+          allow: {
+            ...minimalRole().spec.allow,
+            github_permissions: [{ orgs: ['foo'] }, { orgs: ['bar'] }],
+          },
+        },
+      })
+    ).toEqual({
+      ...minimalRoleModel(),
+      resources: [
+        {
+          kind: 'git_server',
+          organizations: [
+            { label: 'foo', value: 'foo' },
+            { label: 'bar', value: 'bar' },
+          ],
+        },
+      ],
+    } as RoleEditorModel);
+  });
+
+  it.each(['access', 'editor', 'auditor'])(
+    'supports the preset "%s" role',
+    roleName => {
+      const { requiresReset } = roleToRoleEditorModel(presetRoles[roleName]);
+      expect(requiresReset).toBe(false);
+    }
+  );
 });
 
 test('labelsToModel', () => {

web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts

@@ -29,6 +29,7 @@ import {
 import {
   CreateDBUserMode,
   CreateHostUserMode,
+  GitHubPermission,
   KubernetesResourceKind,
   KubernetesVerb,
   RequireMFAType,
@@ -87,7 +88,8 @@ export type ResourceAccess =
   | ServerAccess
   | AppAccess
   | DatabaseAccess
-  | WindowsDesktopAccess;
+  | WindowsDesktopAccess
+  | GitHubOrganizationAccess;
 
 /**
  * A base for all resource section models. Contains a type discriminator field.
@@ -95,7 +97,7 @@ export type ResourceAccess =
 type ResourceAccessBase<T extends ResourceAccessKind> = {
   /**
    * Determines kind of resource that is accessed using this spec. Intended to
-   * be mostly consistent with UnifiedResources.kind, but that has no real
+   * be mostly consistent with UnifiedResourceKind, but that has no real
    * meaning on the server side; we needed some discriminator, so we picked
    * this one.
    */
@@ -107,7 +109,8 @@ export type ResourceAccessKind =
   | 'kube_cluster'
   | 'app'
   | 'db'
-  | 'windows_desktop';
+  | 'windows_desktop'
+  | 'git_server';
 
 /** Model for the Kubernetes resource section. */
 export type KubernetesAccess = ResourceAccessBase<'kube_cluster'> & {
@@ -274,6 +277,10 @@ export type WindowsDesktopAccess = ResourceAccessBase<'windows_desktop'> & {
   logins: readonly Option[];
 };
 
+export type GitHubOrganizationAccess = ResourceAccessBase<'git_server'> & {
+  organizations: readonly Option[];
+};
+
 export type RuleModel = {
   /** Autogenerated ID to be used with the `key` property. */
   id: string;
@@ -434,6 +441,11 @@ export function newResourceAccess(
   roleVersion: RoleVersion
 ): WindowsDesktopAccess;
 
+export function newResourceAccess(
+  kind: 'git_server',
+  roleVersion: RoleVersion
+): GitHubOrganizationAccess;
+
 export function newResourceAccess(
   kind: ResourceAccessKind,
   roleVersion: RoleVersion
@@ -482,6 +494,11 @@ export function newResourceAccess(
         labels: [],
         logins: [stringToOption('{{internal.windows_logins}}')],
       };
+    case 'git_server':
+      return {
+        kind: 'git_server',
+        organizations: [stringToOption('{{internal.github_orgs}}')],
+      };
     default:
       kind satisfies never;
   }
@@ -569,28 +586,37 @@ function roleConditionsToModel(
   roleVersion: RoleVersion
 ): Pick<RoleEditorModel, 'resources' | 'rules' | 'requiresReset'> {
   const {
+    // Server access
     node_labels,
     logins,
 
+    // Kubernetes access
     kubernetes_groups,
     kubernetes_labels,
     kubernetes_resources,
+    kubernetes_users,
 
+    // App access
     app_labels,
     aws_role_arns,
     azure_identities,
     gcp_service_accounts,
 
+    // Database access
     db_labels,
     db_names,
     db_users,
     db_roles,
     db_service_labels,
 
+    // Windows desktop access
     windows_desktop_labels,
     windows_desktop_logins,
-    kubernetes_users,
 
+    // GitHub organization access
+    github_permissions,
+
+    // Access rules
     rules,
 
     ...unsupportedConditions
@@ -690,6 +716,17 @@ function roleConditionsToModel(
     });
   }
 
+  const {
+    model: gitHubOrganizationsModel,
+    requiresReset: gitHubOrgsRequireReset,
+  } = gitHubOrganizationsToModel(github_permissions);
+  if (someNonEmpty(gitHubOrganizationsModel)) {
+    resources.push({
+      kind: 'git_server',
+      organizations: gitHubOrganizationsModel,
+    });
+  }
+
   const { model: rulesModel, requiresReset: rulesRequireReset } =
     rulesToModel(rules);
 
@@ -698,6 +735,7 @@ function roleConditionsToModel(
     rules: rulesModel,
     requiresReset:
       kubernetesResourcesRequireReset ||
+      gitHubOrgsRequireReset ||
       rulesRequireReset ||
       !isEmpty(unsupportedConditions),
   };
@@ -776,6 +814,34 @@ function kubernetesResourceToModel(
   };
 }
 
+/**
+ * Converts a {@link GitHubPermission} array to a list of organizations.
+ * Technically, there can be more than one `GitHubPermission` object, but we
+ * simply glue the underlying organization arrays into one. Note that the
+ * object's semantics may further be extended to the point where there's a
+ * difference between multiple such objects and one object containing all the
+ * organizations; however, since this would require adding additional fields to
+ * the `GitHubPermission` object (otherwise, such change wouldn't make sense),
+ * this function is protected anyway from attempting to interpret such an
+ * extended object, and it would return `requiresReset: true` anyway.
+ */
+function gitHubOrganizationsToModel(gitHubPermissions: GitHubPermission[]): {
+  model: Option[];
+  requiresReset: boolean;
+} {
+  const permissions = gitHubPermissions ?? [];
+  let requiresReset = false;
+  const model: Option[] = [];
+  for (const { orgs, ...rest } of permissions) {
+    if (!isEmpty(rest)) {
+      requiresReset = true;
+    }
+    model.push(...stringsToOptions(orgs));
+  }
+
+  return { model, requiresReset };
+}
+
 function rulesToModel(rules: Rule[]): {
   model: RuleModel[];
   requiresReset: boolean;
@@ -1025,6 +1091,12 @@ export function roleEditorModelToRole(roleModel: RoleEditorModel): Role {
         role.spec.allow.windows_desktop_logins = optionsToStrings(res.logins);
         break;
 
+      case 'git_server':
+        role.spec.allow.github_permissions = [
+          { orgs: optionsToStrings(res.organizations) },
+        ];
+        break;
+
       default:
         kind satisfies never;
     }

web/packages/teleport/src/Roles/RoleEditor/StandardEditor/validation.ts

@@ -162,6 +162,8 @@ export function validateResourceAccess(
       return runRules(resource, databaseAccessValidationRules);
     case 'windows_desktop':
       return runRules(resource, windowsDesktopAccessValidationRules);
+    case 'git_server':
+      return { valid: true };
     default:
       kind satisfies never;
   }
@@ -172,7 +174,8 @@ export type ResourceAccessValidationResult =
   | KubernetesAccessValidationResult
   | AppAccessValidationResult
   | DatabaseAccessValidationResult
-  | WindowsDesktopAccessValidationResult;
+  | WindowsDesktopAccessValidationResult
+  | GitHubOrganizationAccessValidationResult;
 
 const validKubernetesResource = (res: KubernetesResourceModel) => () => {
   const kind = validKubernetesKind(res.kind.value, res.roleVersion);
@@ -300,6 +303,8 @@ export type WindowsDesktopAccessValidationResult = RuleSetValidationResult<
   typeof windowsDesktopAccessValidationRules
 >;
 
+export type GitHubOrganizationAccessValidationResult = ValidationResult;
+
 export function validateAccessRuleList(
   rules: RuleModel[],
   previousRules: RuleModel[],

web/packages/teleport/src/services/resources/types.ts

@@ -96,6 +96,8 @@ export type RoleConditions = {
   windows_desktop_labels?: Labels;
   windows_desktop_logins?: string[];
 
+  github_permissions?: GitHubPermission[];
+
   rules?: Rule[];
 };
 
@@ -345,6 +347,10 @@ export type Verb =
   | 'update'
   | 'use';
 
+export type GitHubPermission = {
+  orgs?: string[];
+};
+
 /**
  * Teleport role options in full format, as returned from Teleport API. Note
  * that its fields follow the snake case convention to match the wire format.
@@ -372,7 +378,7 @@ export type RoleOptions = {
   ssh_port_forwarding?: SSHPortForwarding;
   port_forwarding?: boolean;
   record_session: {
-    default: SessionRecordingMode;
+    default?: SessionRecordingMode;
     ssh?: SessionRecordingMode;
     desktop: boolean;
   };