Skip to content

Commit 4565c8d

Browse files
jhrozekjhrozek
committed
Expose SPIFFE client-auth registration through the operator CRDs
The SPIFFE client-authentication runtime model (trust domains + static workload-to-client associations) already exists in pkg/authserver, but was only reachable by hand-authoring an auth-server RunConfig file -- there was no way to declare it through VirtualMCPServer or MCPExternalAuthConfig. Add SPIFFETrustDomains and InboundGrants.SPIFFEClientAuth to the shared EmbeddedAuthServerConfig CRD type, with CEL admission validation for everything derivable from the object's own spec (paired configuration, no duplicate names/trust domains/client IDs/principal patterns, full cross-referencing between domains and clients, method-subset enforcement, principal/trust-domain consistency, and client-ID hygiene). Wire the new fields through to authserver.RunConfig via the same converter pattern used for DelegateClients/TrustedIssuers, and extend the existing reconcile-time revalidation (validateDelegateClientsAndTrustedIssuers) to cover SPIFFE trust domains too. Cross-field checks that need reconcile-time-derived values (AllowedAudiences, ScopesSupported -- neither is CRD-exposed) are left to that reconcile-time path rather than a premature admission-time Go check: an earlier attempt at that check passed nil for both, which the runtime validator treats as "validate against nothing" rather than "skip", silently rejecting any resources or custom scopes entry. A syntactically valid SPIFFE config is admitted by CEL but still fails reconciliation with a terminal error until RunConfig's "not yet enforced" placeholder gate is lifted by a future PR -- no live SVID verification exists yet, so this is expected, not a regression. Signed-off-by: Jakub Hrozek <jakub@stacklok.com>
1 parent 9aa58ce commit 4565c8d

12 files changed

Lines changed: 3698 additions & 31 deletions

‎cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go‎

Lines changed: 320 additions & 9 deletions
Large diffs are not rendered by default.

‎cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,159 @@ func TestMCPExternalAuthConfig_validateEmbeddedAuthServer(t *testing.T) {
779779
expectErr: true,
780780
errMsg: "actor_matcher",
781781
},
782+
{
783+
// validateEmbeddedAuthServer deliberately runs no Go-level
784+
// permission-shaped SPIFFE pre-check (resources/scopes) at this
785+
// layer — same precedent as DelegateClients: a meaningful
786+
// resources/scopes check needs AllowedAudiences/ScopesSupported,
787+
// which only exist once derived at reconcile time. (It does run
788+
// the admission-time-safe bundle-URL and principal-overlap
789+
// checks below — see validateSPIFFEBundleEndpoints and
790+
// validateSPIFFEPrincipalPatternOverlap tests.) A resources
791+
// entry and a non-default scope must therefore pass here even
792+
// though they'd need revalidating once those derived values are
793+
// known (see
794+
// TestBuildAuthServerRunConfigInvalidSPIFFEIsTypedAndNotYetEnforced
795+
// in controllerutil for the reconcile-time revalidation this
796+
// relies on). CEL (spiffe_cel_test.go) covers structural
797+
// correctness (trust-domain refs, method subsets, etc.) at
798+
// admission.
799+
name: "spiffe client with resources and custom scope - valid at this layer",
800+
config: &MCPExternalAuthConfig{
801+
Spec: MCPExternalAuthConfigSpec{
802+
Type: ExternalAuthTypeEmbeddedAuthServer,
803+
EmbeddedAuthServer: &EmbeddedAuthServerConfig{
804+
Issuer: "https://auth.example.com",
805+
UpstreamProviders: []UpstreamProviderConfig{{
806+
Name: "github",
807+
Type: UpstreamProviderTypeOIDC,
808+
OIDCConfig: &OIDCUpstreamConfig{IssuerURL: "https://github.com", ClientID: "client-id"},
809+
}},
810+
SPIFFETrustDomains: []SPIFFETrustDomainConfig{{
811+
Name: "example", TrustDomain: "example.org",
812+
Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509},
813+
BundleSource: SPIFFEBundleSourceConfig{
814+
Type: SPIFFEBundleSourceTypeWorkloadAPI,
815+
WorkloadAPI: &SPIFFEWorkloadAPIBundleSourceConfig{},
816+
},
817+
}},
818+
InboundGrants: &InboundGrantsConfig{
819+
SPIFFEClientAuth: []SPIFFEClientConfig{{
820+
TrustDomainRef: "example",
821+
PrincipalPattern: "spiffe://example.org/ns/default/agent",
822+
ClientID: "spiffe-client",
823+
Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509},
824+
// Not in allowed_audiences and not a default
825+
// scope — would be rejected if the removed
826+
// admission-time pre-check fabricated a nil
827+
// allowlist/default-scopes set instead of
828+
// deferring to reconcile time.
829+
Resources: []string{"https://backend.example.com"},
830+
Audiences: []string{"https://mcp.example.com"},
831+
Scopes: []string{"custom:scope"},
832+
}},
833+
},
834+
},
835+
},
836+
},
837+
expectErr: false,
838+
},
839+
{
840+
name: "spiffe bundle-endpoint URL rejects non-https scheme",
841+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("http://bundle.example.com"),
842+
expectErr: true,
843+
errMsg: "must be an absolute HTTPS URL",
844+
},
845+
{
846+
name: "spiffe bundle-endpoint URL rejects userinfo",
847+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://user:pass@bundle.example.com"),
848+
expectErr: true,
849+
errMsg: "must not contain credentials",
850+
},
851+
{
852+
name: "spiffe bundle-endpoint URL rejects a query string",
853+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://bundle.example.com?x=1"),
854+
expectErr: true,
855+
errMsg: "must not contain credentials",
856+
},
857+
{
858+
name: "spiffe bundle-endpoint URL rejects a fragment",
859+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://bundle.example.com/bundle#frag"),
860+
expectErr: true,
861+
errMsg: "must not contain credentials",
862+
},
863+
{
864+
name: "spiffe bundle-endpoint URL rejects an IP-literal host",
865+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://192.0.2.1"),
866+
expectErr: true,
867+
errMsg: "must not contain credentials",
868+
},
869+
{
870+
name: "spiffe bundle-endpoint URL rejects the localhost host",
871+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://localhost"),
872+
expectErr: true,
873+
errMsg: "must not contain credentials",
874+
},
875+
{
876+
name: "spiffe bundle-endpoint URL rejects the 127.0.0.1 loopback host",
877+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://127.0.0.1"),
878+
expectErr: true,
879+
errMsg: "must not contain credentials",
880+
},
881+
{
882+
name: "spiffe bundle-endpoint URL rejects the [::1] loopback host",
883+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://[::1]"),
884+
expectErr: true,
885+
errMsg: "must not contain credentials",
886+
},
887+
{
888+
name: "spiffe bundle-endpoint URL accepts a valid https URL",
889+
config: mustEmbeddedAuthServerConfigWithBundleEndpoint("https://bundle.example.com"),
890+
expectErr: false,
891+
},
892+
{
893+
name: "spiffe principal patterns reject an identical duplicate pair",
894+
config: mustEmbeddedAuthServerConfigWithPrincipalPatterns(
895+
"spiffe://example.org/ns/default/agent", "spiffe://example.org/ns/default/agent",
896+
),
897+
expectErr: true,
898+
errMsg: "overlaps",
899+
},
900+
{
901+
name: "spiffe principal patterns reject a wildcard overlapping a concrete principal",
902+
config: mustEmbeddedAuthServerConfigWithPrincipalPatterns(
903+
"spiffe://example.org/agent/*", "spiffe://example.org/agent/one",
904+
),
905+
expectErr: true,
906+
errMsg: "overlaps",
907+
},
908+
{
909+
name: "spiffe principal patterns reject two overlapping wildcards",
910+
config: mustEmbeddedAuthServerConfigWithPrincipalPatterns(
911+
"spiffe://example.org/agent/*", "spiffe://example.org/agent/one/*",
912+
),
913+
expectErr: true,
914+
errMsg: "overlaps",
915+
},
916+
{
917+
name: "spiffe principal patterns accept a genuinely non-overlapping pair",
918+
config: mustEmbeddedAuthServerConfigWithPrincipalPatterns(
919+
"spiffe://example.org/agent/*", "spiffe://example.org/other/*",
920+
),
921+
expectErr: false,
922+
},
923+
{
924+
// Regression case: when only the SECOND entry in a pair is
925+
// malformed, the error must name index 1, not index 0 — a
926+
// pairwise loop that always blames the first entry in the pair
927+
// it happens to be comparing would get this wrong.
928+
name: "spiffe principal patterns blame the correct index when the second entry is malformed",
929+
config: mustEmbeddedAuthServerConfigWithPrincipalPatterns(
930+
"spiffe://example.org/agent/one", "spiffe://example.org/agent~2",
931+
),
932+
expectErr: true,
933+
errMsg: "spiffeClientAuth[1].principalPattern",
934+
},
782935
}
783936

784937
for _, tt := range tests {
@@ -796,6 +949,79 @@ func TestMCPExternalAuthConfig_validateEmbeddedAuthServer(t *testing.T) {
796949
}
797950
}
798951

952+
// mustEmbeddedAuthServerConfigWithBundleEndpoint builds a minimal valid
953+
// MCPExternalAuthConfig with a single spiffeTrustDomains entry whose
954+
// bundleSource is a bundle_endpoint with the given URL (always using the
955+
// https_web profile), for exercising validateSPIFFEBundleEndpoints in
956+
// isolation.
957+
func mustEmbeddedAuthServerConfigWithBundleEndpoint(url string) *MCPExternalAuthConfig {
958+
return &MCPExternalAuthConfig{
959+
Spec: MCPExternalAuthConfigSpec{
960+
Type: ExternalAuthTypeEmbeddedAuthServer,
961+
EmbeddedAuthServer: &EmbeddedAuthServerConfig{
962+
Issuer: "https://auth.example.com",
963+
UpstreamProviders: []UpstreamProviderConfig{{
964+
Name: "github",
965+
Type: UpstreamProviderTypeOIDC,
966+
OIDCConfig: &OIDCUpstreamConfig{IssuerURL: "https://github.com", ClientID: "client-id"},
967+
}},
968+
SPIFFETrustDomains: []SPIFFETrustDomainConfig{{
969+
Name: "example", TrustDomain: "example.org",
970+
Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509},
971+
BundleSource: SPIFFEBundleSourceConfig{
972+
Type: SPIFFEBundleSourceTypeEndpoint,
973+
Endpoint: &SPIFFEBundleEndpointSourceConfig{
974+
URL: url, Profile: SPIFFEBundleEndpointProfileHTTPSWeb,
975+
},
976+
},
977+
}},
978+
},
979+
},
980+
}
981+
}
982+
983+
// mustEmbeddedAuthServerConfigWithPrincipalPatterns builds a minimal valid
984+
// MCPExternalAuthConfig with two spiffeClientAuth entries using the given
985+
// principal patterns, for exercising validateSPIFFEPrincipalPatternOverlap
986+
// in isolation.
987+
func mustEmbeddedAuthServerConfigWithPrincipalPatterns(first, second string) *MCPExternalAuthConfig {
988+
return &MCPExternalAuthConfig{
989+
Spec: MCPExternalAuthConfigSpec{
990+
Type: ExternalAuthTypeEmbeddedAuthServer,
991+
EmbeddedAuthServer: &EmbeddedAuthServerConfig{
992+
Issuer: "https://auth.example.com",
993+
UpstreamProviders: []UpstreamProviderConfig{{
994+
Name: "github",
995+
Type: UpstreamProviderTypeOIDC,
996+
OIDCConfig: &OIDCUpstreamConfig{IssuerURL: "https://github.com", ClientID: "client-id"},
997+
}},
998+
SPIFFETrustDomains: []SPIFFETrustDomainConfig{{
999+
Name: "example", TrustDomain: "example.org",
1000+
Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509},
1001+
BundleSource: SPIFFEBundleSourceConfig{
1002+
Type: SPIFFEBundleSourceTypeWorkloadAPI,
1003+
WorkloadAPI: &SPIFFEWorkloadAPIBundleSourceConfig{},
1004+
},
1005+
}},
1006+
InboundGrants: &InboundGrantsConfig{
1007+
SPIFFEClientAuth: []SPIFFEClientConfig{
1008+
{
1009+
TrustDomainRef: "example", PrincipalPattern: first, ClientID: "spiffe-client-1",
1010+
Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509},
1011+
Audiences: []string{"https://mcp.example.com"}, Scopes: []string{"openid"},
1012+
},
1013+
{
1014+
TrustDomainRef: "example", PrincipalPattern: second, ClientID: "spiffe-client-2",
1015+
Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509},
1016+
Audiences: []string{"https://mcp.example.com"}, Scopes: []string{"openid"},
1017+
},
1018+
},
1019+
},
1020+
},
1021+
},
1022+
}
1023+
}
1024+
7991025
func TestMCPExternalAuthConfig_ZeroUpstreamAlternatives(t *testing.T) {
8001026
t.Parallel()
8011027

‎cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 125 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)