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() })