From 3952aa9af83a50491d48e971e679efc65a120e1e Mon Sep 17 00:00:00 2001 From: Allison Larson Date: Fri, 28 Nov 2025 15:47:09 -0800 Subject: [PATCH 1/3] wip: use temp cap/oidc branch --- go.mod | 1 + go.sum | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 69a1ff6f14e..666ceb0702b 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.25.3 // Pinned dependencies are noted in github.com/hashicorp/nomad/issues/11826. replace ( github.com/Microsoft/go-winio => github.com/endocrimes/go-winio v0.4.13-0.20190628114223-fb47a8b41948 + github.com/hashicorp/cap => github.com/allisonlarson/cap v0.0.0-20251128192950-7ada9938af25 github.com/hashicorp/hcl => github.com/hashicorp/hcl v1.0.1-nomad-1 ) diff --git a/go.sum b/go.sum index 007a414305a..10994a6c772 100644 --- a/go.sum +++ b/go.sum @@ -109,6 +109,8 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= +github.com/allisonlarson/cap v0.0.0-20251128192950-7ada9938af25 h1:atbEO0ax7BXODSqywPFdra76CfUHIQqtq3QiACcGdyw= +github.com/allisonlarson/cap v0.0.0-20251128192950-7ada9938af25/go.mod h1:HKbv27kfps+wONFNyNTHpAQmU/DCjjDuB5HF6mFsqPQ= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/apparentlymart/go-cidr v1.0.1 h1:NmIwLZ/KdsjIUlhf+/Np40atNXm/+lZ5txfTJ/SpF+U= github.com/apparentlymart/go-cidr v1.0.1/go.mod h1:EBcsNrHc3zQeuaeCeCtQruQm+n9/YjEn/vI25Lg7Gwc= @@ -415,8 +417,6 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.2 h1:8Tjv8EJ+pM1xP8mK6egEbD1OgnV github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.2/go.mod h1:pkJQ2tZHJ0aFOVEEot6oZmaVEZcRme73eIFmhiVuRWs= github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.65 h1:81+kWbE1yErFBMjME0I5k3x3kojjKsWtPYHEAutoPow= github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.65/go.mod h1:WtMzv9T++tfWVea+qB2MXoaqxw33S8bpJslzUike2mQ= -github.com/hashicorp/cap v0.11.0 h1:tnMNgIWEdbmyx0fulrlLPNHowsprg34xFWflOEB3t1s= -github.com/hashicorp/cap v0.11.0/go.mod h1:HKbv27kfps+wONFNyNTHpAQmU/DCjjDuB5HF6mFsqPQ= github.com/hashicorp/cli v1.1.7 h1:/fZJ+hNdwfTSfsxMBa9WWMlfjUZbX8/LnUxgAd7lCVU= github.com/hashicorp/cli v1.1.7/go.mod h1:e6Mfpga9OCT1vqzFuoGZiiF/KaG9CbUfO5s3ghU3YgU= github.com/hashicorp/consul-template v0.41.3 h1:kBV74WN+UBl7TL3tzXGXU4AiGug4teUrAGO3vnnz+DI= From 44ba6067a862bb3dd969dd2af799ac674c3e8d73 Mon Sep 17 00:00:00 2001 From: Allison Larson Date: Fri, 28 Nov 2025 15:47:52 -0800 Subject: [PATCH 2/3] add iss check to oidc auth resp flow --- api/acl.go | 2 + command/login.go | 1 + lib/auth/oidc/server.go | 1 + nomad/acl_endpoint.go | 14 +++++ nomad/acl_endpoint_test.go | 123 +++++++++++++++++++++++++++++++++++++ nomad/structs/acl.go | 1 + 6 files changed, 142 insertions(+) diff --git a/api/acl.go b/api/acl.go index edaa6a5a668..ed2b0e52f86 100644 --- a/api/acl.go +++ b/api/acl.go @@ -1232,6 +1232,8 @@ type ACLOIDCCompleteAuthRequest struct { State string Code string + Iss string + // RedirectURI is the URL that authorization should redirect to. This is a // required parameter. RedirectURI string diff --git a/command/login.go b/command/login.go index 39eff066896..d9433ce010e 100644 --- a/command/login.go +++ b/command/login.go @@ -256,6 +256,7 @@ func (l *LoginCommand) loginOIDC(ctx context.Context, client *api.Client) (*api. ClientNonce: callbackServer.Nonce(), Code: req.Code, State: req.State, + Iss: req.Iss, } token, _, err := client.ACLAuth().CompleteAuth(&cbArgs, nil) diff --git a/lib/auth/oidc/server.go b/lib/auth/oidc/server.go index 159217d34d6..bc3ce4d04d9 100644 --- a/lib/auth/oidc/server.go +++ b/lib/auth/oidc/server.go @@ -87,6 +87,7 @@ func (s *CallbackServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { State: q.Get("state"), ClientNonce: s.clientNonce, Code: q.Get("code"), + Iss: q.Get("iss"), } // Send our result. We don't block here because the channel should be diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index 9baeb35911c..fd3ebed9621 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -2767,6 +2767,20 @@ func (a *ACL) OIDCCompleteAuth( return fmt.Errorf("failed to generate OIDC provider: %v", err) } + // Check if the OIDC provider requires the `iss` parameter to be + // validated + providerMetadata := struct { + AuthorizationResponseIssParameterSupported bool `json:"authorization_response_iss_parameter_supported"` + }{} + if err := oidcProvider.Claims(&providerMetadata); err != nil { + return fmt.Errorf("failed to retrieve OIDC provider metadata: %v", err) + } + if providerMetadata.AuthorizationResponseIssParameterSupported { + if args.Iss == "" || args.Iss != authMethod.Config.OIDCDiscoveryURL { + return errors.New("access denied: invalid issuer in callback") + } + } + // Retrieve the request generated in OIDCAuthURL() oidcReq := a.oidcRequestCache.LoadAndDelete(args.ClientNonce) // I am so done with this NONCENSE if oidcReq == nil { diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go index 5d582d32d3c..9fd1486ddcf 100644 --- a/nomad/acl_endpoint_test.go +++ b/nomad/acl_endpoint_test.go @@ -4080,6 +4080,129 @@ func TestACL_OIDCCompleteAuth(t *testing.T) { }) } +// TestACL_OIDCCompleteAuth_IssEnforcedProvider tests that when an OIDC provider +// enforces the authorization_response_iss_parameter_supported configuration, the +// `iss` parameter is properly validated. +func TestACL_OIDCCompleteAuth_IssEnforcedProvider(t *testing.T) { + ci.Parallel(t) + + // setup the ACL server with verbose logging + var buf bytes.Buffer + testServer, _, testServerCleanupFn := TestACLServer(t, func(c *Config) { + c.Logger = hclog.NewInterceptLogger(&hclog.LoggerOptions{ + Level: hclog.Debug, + Output: io.MultiWriter(&buf, os.Stderr), + }) + }) + defer testServerCleanupFn() + codec := rpcClient(t, testServer) + testutil.WaitForLeader(t, testServer.RPC) + + // setup the test OIDC provider that requires iss parameter + oidcTestProvider := capOIDC.StartTestProvider(t) + defer oidcTestProvider.Stop() + oidcTestProvider.SetAllowedRedirectURIs([]string{"http://127.0.0.1:4649/oidc/callback"}) + oidcTestProvider.SetExpectedAuthNonce("fsSPuaodKevKfDU3IeXa") + oidcTestProvider.SetExpectedAuthCode("codeABC") + oidcTestProvider.SetCustomAudience("mock") + oidcTestProvider.SetCustomClaims(map[string]interface{}{ + "azp": "mock", + "http://nomad.internal/policies": []string{"engineering"}, + "http://nomad.internal/roles": []string{"engineering"}, + }) + oidcTestProvider.SetAdditionalConfiguration(map[string]interface{}{ + "authorization_response_iss_parameter_supported": true, + }) + + // setup the OIDC auth method with our test values + mockedAuthMethod := mock.ACLOIDCAuthMethod() + mockedAuthMethod.Config.BoundAudiences = []string{"mock"} + mockedAuthMethod.Config.AllowedRedirectURIs = []string{"http://127.0.0.1:4649/oidc/callback"} + mockedAuthMethod.Config.OIDCDiscoveryURL = oidcTestProvider.Addr() + mockedAuthMethod.Config.SigningAlgs = []string{"ES256"} + mockedAuthMethod.Config.DiscoveryCaPem = []string{oidcTestProvider.CACert()} + mockedAuthMethod.Config.ClaimMappings = map[string]string{} + mockedAuthMethod.Config.ListClaimMappings = map[string]string{ + "http://nomad.internal/roles": "roles", + "http://nomad.internal/policies": "policies", + } + + must.NoError(t, testServer.fsm.State().UpsertACLAuthMethods(10, []*structs.ACLAuthMethod{mockedAuthMethod})) + + // upsert and bind ACL policy and role for use in tests + mockACLPolicy := mock.ACLPolicy() + must.NoError(t, testServer.fsm.State().UpsertACLPolicies( + structs.MsgTypeTestSetup, 20, []*structs.ACLPolicy{mockACLPolicy})) + + mockACLRole := mock.ACLRole() + mockACLRole.Policies = []*structs.ACLRolePolicyLink{{Name: mockACLPolicy.Name}} + must.NoError(t, testServer.fsm.State().UpsertACLRoles( + structs.MsgTypeTestSetup, 30, []*structs.ACLRole{mockACLRole}, true)) + + mockBindingRule := mock.ACLBindingRule() + mockBindingRule.AuthMethod = mockedAuthMethod.Name + mockBindingRule.BindType = structs.ACLBindingRuleBindTypePolicy + mockBindingRule.Selector = "engineering in list.policies" + mockBindingRule.BindName = mockACLPolicy.Name + + must.NoError(t, testServer.fsm.State().UpsertACLBindingRules( + 40, []*structs.ACLBindingRule{mockBindingRule}, true)) + + // test a missing iss parameter + missingIssReq := structs.ACLOIDCCompleteAuthRequest{ + AuthMethodName: mockedAuthMethod.Name, + RedirectURI: mockedAuthMethod.Config.AllowedRedirectURIs[0], + ClientNonce: "fsSPuaodKevKfDU3IeXa", + State: "st_someweirdstateid", + Code: "codeABC", + WriteRequest: structs.WriteRequest{Region: DefaultRegion}, + } + // Pretend that OIDCAuthURL was called as a separate request. + cacheOIDCRequest(t, testServer.oidcRequestCache, missingIssReq) + + var missingIssResp structs.ACLLoginResponse + err := msgpackrpc.CallWithCodec(codec, structs.ACLOIDCCompleteAuthRPCMethod, &missingIssReq, &missingIssResp) + must.Error(t, err) + must.ErrorContains(t, err, "access denied: invalid issuer") + + // test an incorrect iss parameter + incorrectIssReq := structs.ACLOIDCCompleteAuthRequest{ + AuthMethodName: mockedAuthMethod.Name, + RedirectURI: mockedAuthMethod.Config.AllowedRedirectURIs[0], + ClientNonce: "fsSPuaodKevKfDU3IeXa", + State: "st_someweirdstateid", + Code: "codeABC", + Iss: "incorrect-issuer", + WriteRequest: structs.WriteRequest{Region: DefaultRegion}, + } + // Pretend that OIDCAuthURL was called as a separate request. + cacheOIDCRequest(t, testServer.oidcRequestCache, incorrectIssReq) + + var incorrectIssResp structs.ACLLoginResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLOIDCCompleteAuthRPCMethod, &incorrectIssReq, &incorrectIssResp) + must.Error(t, err) + must.ErrorContains(t, err, "access denied: invalid issuer") + + // test a valid iss parameter + req := structs.ACLOIDCCompleteAuthRequest{ + AuthMethodName: mockedAuthMethod.Name, + RedirectURI: mockedAuthMethod.Config.AllowedRedirectURIs[0], + ClientNonce: "fsSPuaodKevKfDU3IeXa", + State: "st_someweirdstateid", + Code: "codeABC", + Iss: oidcTestProvider.Addr(), + WriteRequest: structs.WriteRequest{Region: DefaultRegion}, + } + // Pretend that OIDCAuthURL was called as a separate request. + cacheOIDCRequest(t, testServer.oidcRequestCache, req) + + var resp structs.ACLLoginResponse + err = msgpackrpc.CallWithCodec(codec, structs.ACLOIDCCompleteAuthRPCMethod, &req, &resp) + must.NoError(t, err) + must.NotNil(t, resp.ACLToken) + must.Eq(t, structs.ACLClientToken, resp.ACLToken.Type) +} + // mockSerializer implements the capOIDC.JWTSerializer interface, // which is used to provide a client assertion JWT. type mockSerializer struct { diff --git a/nomad/structs/acl.go b/nomad/structs/acl.go index 7b95163f5e0..db7e94bf889 100644 --- a/nomad/structs/acl.go +++ b/nomad/structs/acl.go @@ -2350,6 +2350,7 @@ type ACLOIDCCompleteAuthRequest struct { ClientNonce string State string Code string + Iss string // RedirectURI is the URL that authorization should redirect to. This is a // required parameter. From c33c59783ba01fe2b3812e5c17dfd95b82b49e60 Mon Sep 17 00:00:00 2001 From: Allison Larson Date: Tue, 2 Dec 2025 14:14:23 -0800 Subject: [PATCH 3/3] update error message --- nomad/acl_endpoint.go | 2 +- nomad/acl_endpoint_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index fd3ebed9621..b1fe48533e8 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -2777,7 +2777,7 @@ func (a *ACL) OIDCCompleteAuth( } if providerMetadata.AuthorizationResponseIssParameterSupported { if args.Iss == "" || args.Iss != authMethod.Config.OIDCDiscoveryURL { - return errors.New("access denied: invalid issuer in callback") + return errors.New("invalid or missing issuer parameter in callback") } } diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go index 9fd1486ddcf..8a6b01db473 100644 --- a/nomad/acl_endpoint_test.go +++ b/nomad/acl_endpoint_test.go @@ -4163,7 +4163,7 @@ func TestACL_OIDCCompleteAuth_IssEnforcedProvider(t *testing.T) { var missingIssResp structs.ACLLoginResponse err := msgpackrpc.CallWithCodec(codec, structs.ACLOIDCCompleteAuthRPCMethod, &missingIssReq, &missingIssResp) must.Error(t, err) - must.ErrorContains(t, err, "access denied: invalid issuer") + must.ErrorContains(t, err, "invalid or missing issuer parameter") // test an incorrect iss parameter incorrectIssReq := structs.ACLOIDCCompleteAuthRequest{ @@ -4181,7 +4181,7 @@ func TestACL_OIDCCompleteAuth_IssEnforcedProvider(t *testing.T) { var incorrectIssResp structs.ACLLoginResponse err = msgpackrpc.CallWithCodec(codec, structs.ACLOIDCCompleteAuthRPCMethod, &incorrectIssReq, &incorrectIssResp) must.Error(t, err) - must.ErrorContains(t, err, "access denied: invalid issuer") + must.ErrorContains(t, err, "invalid or missing issuer parameter") // test a valid iss parameter req := structs.ACLOIDCCompleteAuthRequest{