Enable optimistic locking support for OIDC connectors (#33458)

Updates tctl edit and the web ui to start using optimistic locking.
The functionality to support optimistic locking already existed, the
APIs used by both clients were updated to use create/update instead
of upsert so that optimistic locking could be enforced. Most of the
changes introduced are tests to ensure that tctl edit, tctl create
behave as expected.

Note: the web ui changes are include in the e ref update.

Contributes to #30416.

e

@@ -1 +1 @@
-Subproject commit 53fee236e7d0b46baf2c35885b14df4fc3d80e9e
+Subproject commit f427336bc0a3617d9d3bf167baa0a08f7a237a30

tool/tctl/common/edit_command_test.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/backend"
+	"github.com/gravitational/teleport/lib/modules"
 	"github.com/gravitational/teleport/lib/tbot/testhelpers"
 	"github.com/gravitational/teleport/lib/utils"
 )
@@ -85,5 +86,65 @@ func TestEditGithubConnector(t *testing.T) {
 	// since the created revision is stale.
 	_, err = runEditCommand(t, fc, []string{"edit", "connector/github"}, withEditor(editor))
 	assert.Error(t, err, "stale connector was allowed to be updated")
-	require.Error(t, backend.ErrIncorrectRevision, err, "expected an incorrect revision error got %T", err)
+	require.ErrorIs(t, err, backend.ErrIncorrectRevision, "expected an incorrect revision error got %T", err)
+}
+
+func TestEditOIDConnector(t *testing.T) {
+	modules.SetTestModules(t, &modules.TestModules{
+		TestBuildType: modules.BuildEnterprise,
+		TestFeatures:  modules.Features{OIDC: true},
+	})
+	ctx := context.Background()
+	log := utils.NewLoggerForTests()
+	fc, fds := testhelpers.DefaultConfig(t)
+	_ = testhelpers.MakeAndRunTestAuthServer(t, log, fc, fds)
+	rootClient := testhelpers.MakeDefaultAuthClient(t, log, fc)
+	expected, err := types.NewOIDCConnector("oidc", types.OIDCConnectorSpecV3{
+		ClientID:     "12345",
+		ClientSecret: "678910",
+		RedirectURLs: []string{"https://proxy.example.com/v1/webapi/github/callback"},
+		Display:      "OIDC",
+		ClaimsToRoles: []types.ClaimMapping{
+			{
+				Claim: "test",
+				Value: "test",
+				Roles: []string{"access", "editor", "auditor"},
+			},
+		},
+	})
+	require.NoError(t, err, "creating initial connector resource")
+	created, err := rootClient.CreateOIDCConnector(ctx, expected.(*types.OIDCConnectorV3))
+	require.NoError(t, err, "persisting initial connector resource")
+
+	editor := func(name string) error {
+		f, err := os.Create(name)
+		if err != nil {
+			return trace.Wrap(err, "opening file to edit")
+		}
+
+		expected.SetRevision(created.GetRevision())
+		expected.SetClientID("abcdef")
+
+		collection := &connectorsCollection{oidc: []types.OIDCConnector{expected}}
+		return trace.NewAggregate(writeYAML(collection, f), f.Close())
+
+	}
+
+	// Edit the connector and validate that the expected field is updated.
+	_, err = runEditCommand(t, fc, []string{"edit", "connector/oidc"}, withEditor(editor))
+	require.NoError(t, err, "expected editing oidc connector to succeed")
+
+	actual, err := rootClient.GetOIDCConnector(ctx, expected.GetName(), false)
+	require.NoError(t, err, "retrieving oidc connector after edit")
+	require.Empty(t, cmp.Diff(created, actual, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision", "Namespace"),
+		cmpopts.IgnoreFields(types.OIDCConnectorSpecV3{}, "ClientID", "ClientSecret"),
+	))
+	require.NotEqual(t, created.GetClientID(), actual.GetClientID(), "client id should have been modified by edit")
+	require.Equal(t, expected.GetClientID(), actual.GetClientID(), "client id should match the retrieved connector")
+
+	// Try editing the connector a second time. This time the revisions will not match
+	// since the created revision is stale.
+	_, err = runEditCommand(t, fc, []string{"edit", "connector/oidc"}, withEditor(editor))
+	assert.Error(t, err, "stale connector was allowed to be updated")
+	require.ErrorIs(t, err, backend.ErrIncorrectRevision, "expected an incorrect revision error got %T", err)
 }

tool/tctl/common/resource_command.go

@@ -142,6 +142,7 @@ func (rc *ResourceCommand) Initialize(app *kingpin.Application, config *servicec
 	rc.UpdateHandlers = map[ResourceKind]ResourceCreateHandler{
 		types.KindUser:            rc.updateUser,
 		types.KindGithubConnector: rc.updateGithubConnector,
+		types.KindOIDCConnector:   rc.updateOIDCConnector,
 	}
 	rc.config = config
 
@@ -823,6 +824,20 @@ func (rc *ResourceCommand) createOIDCConnector(ctx context.Context, client auth.
 	return nil
 }
 
+// updateGithubConnector updates an existing OIDC connector.
+func (rc *ResourceCommand) updateOIDCConnector(ctx context.Context, client auth.ClientI, raw services.UnknownResource) error {
+	connector, err := services.UnmarshalOIDCConnector(raw.Raw)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
+	if _, err := client.UpdateOIDCConnector(ctx, connector); err != nil {
+		return trace.Wrap(err)
+	}
+	fmt.Printf("authentication connector %q has been updated\n", connector.GetName())
+	return nil
+}
+
 func (rc *ResourceCommand) createSAMLConnector(ctx context.Context, client auth.ClientI, raw services.UnknownResource) error {
 	// Create services.SAMLConnector from raw YAML to extract the connector name.
 	conn, err := services.UnmarshalSAMLConnector(raw.Raw)

tool/tctl/common/resource_command_test.go

@@ -44,6 +44,7 @@ import (
 	"github.com/gravitational/teleport/lib/config"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/fixtures"
+	"github.com/gravitational/teleport/lib/modules"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/tbot/testhelpers"
 	"github.com/gravitational/teleport/lib/utils"
@@ -1403,3 +1404,67 @@ version: v3`
 	_, err = runResourceCommand(t, fc, []string{"create", "-f", connectorYAMLPath})
 	require.NoError(t, err)
 }
+
+func TestCreateOIDCConnector(t *testing.T) {
+	modules.SetTestModules(t, &modules.TestModules{
+		TestBuildType: modules.BuildEnterprise,
+		TestFeatures:  modules.Features{OIDC: true},
+	})
+
+	fc, fds := testhelpers.DefaultConfig(t)
+	makeAndRunTestAuthServer(t, withFileConfig(fc), withFileDescriptors(fds), withFakeClock(clockwork.NewFakeClock()))
+
+	// Ensure there are no connectors to start
+	buf, err := runResourceCommand(t, fc, []string{"get", types.KindOIDCConnector, "--format=json"})
+	require.NoError(t, err)
+	connectors := mustDecodeJSON[[]*types.OIDCConnectorV3](t, buf)
+	require.Empty(t, connectors)
+
+	const connectorYAML = `kind: oidc
+version: v3
+metadata:
+  name: oidc
+spec:
+  redirect_url: "https://proxy.example.com/v1/webapi/oidc/callback"
+  client_id: "12345"
+  client_secret: "678910"
+  display: OIDC
+  scope: [roles]
+  claims_to_roles:
+    - {claim: "test", value: "test", roles: ["access", "editor", "auditor"]}`
+
+	// Create the connector
+	connectorYAMLPath := filepath.Join(t.TempDir(), "connector.yaml")
+	require.NoError(t, os.WriteFile(connectorYAMLPath, []byte(connectorYAML), 0644))
+	_, err = runResourceCommand(t, fc, []string{"create", connectorYAMLPath})
+	require.NoError(t, err)
+
+	// Fetch the connector
+	buf, err = runResourceCommand(t, fc, []string{"get", types.KindOIDCConnector, "--format=json"})
+	require.NoError(t, err)
+	connectors = mustDecodeJSON[[]*types.OIDCConnectorV3](t, buf)
+	require.Len(t, connectors, 1)
+
+	var expected types.OIDCConnectorV3
+	require.NoError(t, yaml.Unmarshal([]byte(connectorYAML), &expected))
+
+	require.Empty(t, cmp.Diff(
+		[]*types.OIDCConnectorV3{&expected},
+		connectors,
+		cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision", "Namespace"),
+		cmpopts.IgnoreFields(types.OIDCConnectorSpecV3{}, "ClientSecret"), // get retrieves the connector without secrets
+	))
+
+	// Explicitly change the revision and try creating the user with and without
+	// the force flag.
+	expected.SetRevision(uuid.NewString())
+	connectorBytes, err := services.MarshalOIDCConnector(&expected, services.PreserveResourceID())
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(connectorYAMLPath, connectorBytes, 0644))
+
+	_, err = runResourceCommand(t, fc, []string{"create", connectorYAMLPath})
+	require.True(t, trace.IsAlreadyExists(err))
+
+	_, err = runResourceCommand(t, fc, []string{"create", "-f", connectorYAMLPath})
+	require.NoError(t, err)
+}