From 70135f3b9f2cfc33cf34438c399c9eb94d2eb544 Mon Sep 17 00:00:00 2001 From: Neil Trini Lasrado Date: Wed, 16 Sep 2026 21:00:26 +0530 Subject: [PATCH] Gate OIDC on a registered service rather than the Enterprise entitlement ServerWithRoles gated UpsertOIDCConnector, UpdateOIDCConnector, CreateOIDCConnector and CreateOIDCAuthRequest on the OIDC Enterprise entitlement, which is never enabled in an OSS build. Gate on whether an OIDCService has actually been registered via Server.SetOIDCService instead, so a self-hosted implementation can be wired in. This gates on capability rather than licence: the entitlement flag and lib/modules are left untouched, and the enterprise connector-management UI stays locked. Three tool/tctl/common suites set up OIDC connectors by enabling the entitlement and never register a service, so they regress under the new gate. Add authtest.StubOIDCService as a shared stub and register it in those fixtures. Their modulestest.SetTestModules setup is kept, since the SAML gate still reads the entitlement and those subtests run alongside the OIDC ones. Co-Authored-By: Claude Opus 5 --- lib/auth/auth_with_roles.go | 28 ++----- lib/auth/auth_with_roles_test.go | 92 +++++++++++++++++++++++ lib/auth/authtest/authtest.go | 21 ++++++ tool/tctl/common/admin_action_test.go | 6 ++ tool/tctl/common/edit_command_test.go | 6 ++ tool/tctl/common/resource_command_test.go | 6 ++ 6 files changed, 139 insertions(+), 20 deletions(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index efe42a5ee8af..cae42f13dcd9 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -4431,11 +4431,8 @@ func (a *ServerWithRoles) UpsertOIDCConnector(ctx context.Context, connector typ if err := a.authConnectorAction(types.KindOIDC, types.VerbUpdate); err != nil { return nil, trace.Wrap(err) } - if !modules.GetModules().Features().GetEntitlement(entitlements.OIDC).Enabled { - // TODO(zmb3): ideally we would wrap ErrRequiresEnterprise here, but - // we can't currently propagate wrapped errors across the gRPC boundary, - // and we want tctl to display a clean user-facing message in this case - return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") + if a.authServer.oidcAuthService == nil { + return nil, trace.AccessDenied("OIDC is not available: no OIDC service is registered") } // Support reused MFA for bulk tctl create requests. @@ -4452,11 +4449,8 @@ func (a *ServerWithRoles) UpdateOIDCConnector(ctx context.Context, connector typ if err := a.authConnectorAction(types.KindOIDC, types.VerbUpdate); err != nil { return nil, trace.Wrap(err) } - if !modules.GetModules().Features().GetEntitlement(entitlements.OIDC).Enabled { - // TODO(zmb3): ideally we would wrap ErrRequiresEnterprise here, but - // we can't currently propagate wrapped errors across the gRPC boundary, - // and we want tctl to display a clean user-facing message in this case - return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") + if a.authServer.oidcAuthService == nil { + return nil, trace.AccessDenied("OIDC is not available: no OIDC service is registered") } if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil { @@ -4472,11 +4466,8 @@ func (a *ServerWithRoles) CreateOIDCConnector(ctx context.Context, connector typ if err := a.authConnectorAction(types.KindOIDC, types.VerbCreate); err != nil { return nil, trace.Wrap(err) } - if !modules.GetModules().Features().GetEntitlement(entitlements.OIDC).Enabled { - // TODO(zmb3): ideally we would wrap ErrRequiresEnterprise here, but - // we can't currently propagate wrapped errors across the gRPC boundary, - // and we want tctl to display a clean user-facing message in this case - return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") + if a.authServer.oidcAuthService == nil { + return nil, trace.AccessDenied("OIDC is not available: no OIDC service is registered") } // Support reused MFA for bulk tctl create requests. @@ -4534,11 +4525,8 @@ func (a *ServerWithRoles) ListOIDCConnectors(ctx context.Context, limit int, sta } func (a *ServerWithRoles) CreateOIDCAuthRequest(ctx context.Context, req types.OIDCAuthRequest) (*types.OIDCAuthRequest, error) { - if !modules.GetModules().Features().GetEntitlement(entitlements.OIDC).Enabled { - // TODO(zmb3): ideally we would wrap ErrRequiresEnterprise here, but - // we can't currently propagate wrapped errors across the gRPC boundary, - // and we want tctl to display a clean user-facing message in this case - return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") + if a.authServer.oidcAuthService == nil { + return nil, trace.AccessDenied("OIDC is not available: no OIDC service is registered") } if err := a.authorizeAction(types.KindOIDCRequest, types.VerbCreate); err != nil { diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index 2dcb1743744d..a071418a6bcc 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -530,6 +530,98 @@ func TestGitHubConnectorNameTooLarge(t *testing.T) { require.ErrorContains(t, err, "exceeds maximum length") } +// TestOIDCGatedOnRegisteredService verifies that ServerWithRoles.CreateOIDCConnector, +// UpsertOIDCConnector, UpdateOIDCConnector and CreateOIDCAuthRequest are gated on +// whether an OIDC service is registered on the auth server (auth.Server.SetOIDCService), +// not on the (Enterprise-only) OIDC entitlement. Slice 0 of the Google-OIDC plan replaced +// the old `modules.GetModules().Features().GetEntitlement(entitlements.OIDC).Enabled` check +// with `a.authServer.oidcAuthService == nil`, since these code paths must work in OSS once +// an OIDC service implementation is registered (Slice 4), independent of licensing. +func TestOIDCGatedOnRegisteredService(t *testing.T) { + t.Parallel() + + srv, err := authtest.NewAuthServer(authtest.AuthServerConfig{Dir: t.TempDir()}) + require.NoError(t, err) + + authContext, err := srv.Authorizer.Authorize(authz.ContextWithUser(t.Context(), authtest.TestBuiltin(types.RoleAdmin).I)) + require.NoError(t, err) + + authWithRoles := auth.NewServerWithRoles( + srv.AuthServer, + new(eventstest.MockAuditLog), + *authContext, + ) + + newConnector := func(name string) types.OIDCConnector { + conn, err := types.NewOIDCConnector(name, types.OIDCConnectorSpecV3{ + ClientID: "example-client-id", + ClientSecret: "example-client-secret", + RedirectURLs: []string{"https://proxy.example.com/v1/webapi/oidc/callback"}, + Display: "OIDC", + ClaimsToRoles: []types.ClaimMapping{ + { + Claim: "test", + Value: "test", + Roles: []string{"access"}, + }, + }, + }) + require.NoError(t, err) + return conn + } + + authRequest := types.OIDCAuthRequest{ConnectorID: "does-not-exist", Type: constants.OIDC} + + // No OIDC service registered: all four gated methods must return AccessDenied, + // regardless of the OIDC entitlement (which is disabled by default in tests). + t.Run("no service registered", func(t *testing.T) { + _, err := authWithRoles.CreateOIDCConnector(t.Context(), newConnector("no-service-create")) + require.Error(t, err) + require.True(t, trace.IsAccessDenied(err), "expected access denied, got: %v", err) + + _, err = authWithRoles.UpsertOIDCConnector(t.Context(), newConnector("no-service-upsert")) + require.Error(t, err) + require.True(t, trace.IsAccessDenied(err), "expected access denied, got: %v", err) + + existing := newConnector("no-service-update") + _, err = authWithRoles.UpdateOIDCConnector(t.Context(), existing) + require.Error(t, err) + require.True(t, trace.IsAccessDenied(err), "expected access denied, got: %v", err) + + _, err = authWithRoles.CreateOIDCAuthRequest(t.Context(), authRequest) + require.Error(t, err) + require.True(t, trace.IsAccessDenied(err), "expected access denied, got: %v", err) + }) + + // Register a stub OIDC service: the gate must now pass through to the + // underlying implementation for all four methods. + srv.AuthServer.SetOIDCService(authtest.StubOIDCService{}) + + t.Run("service registered", func(t *testing.T) { + created, err := authWithRoles.CreateOIDCConnector(t.Context(), newConnector("service-create")) + require.NoError(t, err) + require.NotNil(t, created) + + upserted, err := authWithRoles.UpsertOIDCConnector(t.Context(), newConnector("service-upsert")) + require.NoError(t, err) + require.NotNil(t, upserted) + + // UpdateOIDCConnector requires the revision of the resource being + // updated to match, so update the connector CreateOIDCConnector just + // persisted rather than a freshly constructed one. + _, err = authWithRoles.UpdateOIDCConnector(t.Context(), created) + require.NoError(t, err) + + // CreateOIDCAuthRequest passes the gate and reaches the stub service, + // which unconditionally returns NotImplemented. Getting NotImplemented + // (rather than AccessDenied) proves the gate itself let the call through. + _, err = authWithRoles.CreateOIDCAuthRequest(t.Context(), authRequest) + require.Error(t, err) + require.False(t, trace.IsAccessDenied(err), "gate should have passed through, got: %v", err) + require.True(t, trace.IsNotImplemented(err), "expected the stub service's NotImplemented error, got: %v", err) + }) +} + func TestGithubAuthRequest(t *testing.T) { modulestest.SetTestModules(t, *modulestest.EnterpriseModules()) ctx := context.Background() diff --git a/lib/auth/authtest/authtest.go b/lib/auth/authtest/authtest.go index 3858281be087..6d86087a7fc7 100644 --- a/lib/auth/authtest/authtest.go +++ b/lib/auth/authtest/authtest.go @@ -24,6 +24,7 @@ import ( "crypto/x509" "fmt" "net" + "net/url" "strings" "testing" "time" @@ -1802,3 +1803,23 @@ func FlushCache(t *testing.T, clt Flusher) { } } } + +// StubOIDCService is a minimal auth.OIDCService implementation for tests that +// need to get past ServerWithRoles's "is an OIDC service registered" gate +// (see auth.Server.SetOIDCService) without exercising real OIDC behavior. +// Every method unconditionally returns trace.NotImplemented, so a caller +// that reaches the stub is unambiguously exercising the gate rather than +// accidentally succeeding for some unrelated reason. +type StubOIDCService struct{} + +func (StubOIDCService) CreateOIDCAuthRequest(ctx context.Context, req types.OIDCAuthRequest) (*types.OIDCAuthRequest, error) { + return nil, trace.NotImplemented("authtest.StubOIDCService") +} + +func (StubOIDCService) CreateOIDCAuthRequestForMFA(ctx context.Context, req types.OIDCAuthRequest) (*types.OIDCAuthRequest, error) { + return nil, trace.NotImplemented("authtest.StubOIDCService") +} + +func (StubOIDCService) ValidateOIDCAuthCallback(ctx context.Context, q url.Values) (*authclient.OIDCAuthResponse, error) { + return nil, trace.NotImplemented("authtest.StubOIDCService") +} diff --git a/tool/tctl/common/admin_action_test.go b/tool/tctl/common/admin_action_test.go index 17e8894db984..326f02a44d17 100644 --- a/tool/tctl/common/admin_action_test.go +++ b/tool/tctl/common/admin_action_test.go @@ -43,6 +43,7 @@ import ( "github.com/gravitational/teleport/entitlements" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/auth/authtest" "github.com/gravitational/teleport/lib/auth/mocku2f" "github.com/gravitational/teleport/lib/auth/storage" wancli "github.com/gravitational/teleport/lib/auth/webauthncli" @@ -1094,6 +1095,11 @@ func newAdminActionTestSuite(t *testing.T) *adminActionTestSuite { authAddr, err := process.AuthAddr() require.NoError(t, err) s.authServer = process.GetAuthServer() + // The OIDC connector CRUD RPCs are additionally gated on an OIDC service + // being registered on the auth server (independent of the entitlement + // set above); register a stub so the gate doesn't block this suite's + // OIDC connector admin-action tests. + s.authServer.SetOIDCService(authtest.StubOIDCService{}) // create admin role and user. username := "admin" diff --git a/tool/tctl/common/edit_command_test.go b/tool/tctl/common/edit_command_test.go index d0da1566c8db..7aa053d63948 100644 --- a/tool/tctl/common/edit_command_test.go +++ b/tool/tctl/common/edit_command_test.go @@ -47,6 +47,7 @@ import ( "github.com/gravitational/teleport/api/types/userprovisioning" "github.com/gravitational/teleport/entitlements" "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/auth/authtest" "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/modules" @@ -396,6 +397,11 @@ func TestEditEnterpriseResources(t *testing.T) { require.NoError(t, process.Close()) require.NoError(t, process.Wait()) }) + // The OIDC connector CRUD RPCs are additionally gated on an OIDC service + // being registered on the auth server (independent of the entitlement + // above); register a stub so the gate doesn't block this test from + // exercising `tctl edit` for an OIDC connector. + process.GetAuthServer().SetOIDCService(authtest.StubOIDCService{}) rootClient, err := testenv.NewDefaultAuthClient(process) require.NoError(t, err) t.Cleanup(func() { _ = rootClient.Close() }) diff --git a/tool/tctl/common/resource_command_test.go b/tool/tctl/common/resource_command_test.go index c85f3ab3ab8f..6ba2cf7ac9ab 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -54,6 +54,7 @@ import ( "github.com/gravitational/teleport/entitlements" "github.com/gravitational/teleport/integration/helpers" "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/auth/authtest" "github.com/gravitational/teleport/lib/config" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/fixtures" @@ -2825,6 +2826,11 @@ func TestCreateEnterpriseResources(t *testing.T) { require.NoError(t, process.Close()) require.NoError(t, process.Wait()) }) + // The OIDC connector CRUD RPCs are additionally gated on an OIDC service + // being registered on the auth server (independent of the entitlement + // above); register a stub so the gate doesn't block this test from + // exercising `tctl create` for an OIDC connector. + process.GetAuthServer().SetOIDCService(authtest.StubOIDCService{}) clt, err := testenv.NewDefaultAuthClient(process) require.NoError(t, err) t.Cleanup(func() { _ = clt.Close() })