oidc: support PKCE and client assertion / private key JWT (#25231)

PKCE is enabled by default for new/updated auth methods.
 * ref: https://oauth.net/2/pkce/

Client assertions are an optional, more secure replacement for client secrets
 * ref: https://oauth.net/private-key-jwt/

a change to the existing flow, even without these new options,
is that the oidc.Req is retained on the Nomad server (leader)
in between auth-url and complete-auth calls.

and some fields in auth method config are now more strictly required.

command/acl_auth_method_create_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/command/agent"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/shoenig/test"
 	"github.com/shoenig/test/must"
 )
 
@@ -67,9 +68,10 @@ func TestACLAuthMethodCreateCommand_Run(t *testing.T) {
 	args := []string{
 		"-address=" + url, "-token=" + rootACLToken.SecretID, "-name=acl-auth-method-cli-test",
 		"-type=OIDC", "-token-locality=global", "-default=true", "-max-token-ttl=3600s",
-		"-config={\"OIDCDiscoveryURL\":\"http://example.com\", \"ExpirationLeeway\": \"1h\"}",
+		`-config={"OIDCDiscoveryURL":"http://example.com", "OIDCClientID": "example-id", "BoundAudiences": ["example-aud"], "ExpirationLeeway": "1h"}`,
 	}
-	must.Eq(t, 0, cmd.Run(args))
+	test.Eq(t, 0, cmd.Run(args))
+	test.Eq(t, "", ui.ErrorWriter.String())
 	s := ui.OutputWriter.String()
 	must.StrContains(t, s, "acl-auth-method-cli-test")
 
@@ -81,7 +83,11 @@ func TestACLAuthMethodCreateCommand_Run(t *testing.T) {
 	defer os.Remove(configFile.Name())
 	must.Nil(t, err)
 
-	conf := map[string]interface{}{"OIDCDiscoveryURL": "http://example.com"}
+	conf := map[string]interface{}{
+		"OIDCDiscoveryURL": "http://example.com",
+		"OIDCClientID":     "example-id",
+		"BoundAudiences":   []string{"example-aud"},
+	}
 	jsonData, err := json.Marshal(conf)
 	must.Nil(t, err)
 
@@ -93,7 +99,8 @@ func TestACLAuthMethodCreateCommand_Run(t *testing.T) {
 		"-type=OIDC", "-token-locality=global", "-default=false", "-max-token-ttl=3600s",
 		fmt.Sprintf("-config=@%s", configFile.Name()),
 	}
-	must.Eq(t, 0, cmd.Run(args))
+	test.Eq(t, 0, cmd.Run(args))
+	test.Eq(t, "", ui.ErrorWriter.String())
 	s = ui.OutputWriter.String()
 	must.StrContains(t, s, "acl-auth-method-cli-test")
 

command/acl_auth_method_update_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/nomad/command/agent"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/shoenig/test"
 	"github.com/shoenig/test/must"
 )
 
@@ -64,6 +65,8 @@ func TestACLAuthMethodUpdateCommand_Run(t *testing.T) {
 		TokenLocality: "local",
 		Config: &structs.ACLAuthMethodConfig{
 			OIDCDiscoveryURL: "http://example.com",
+			OIDCClientID:     "example-id",
+			BoundAudiences:   []string{"example-aud"},
 		},
 	}
 	method.SetHash()
@@ -80,7 +83,8 @@ func TestACLAuthMethodUpdateCommand_Run(t *testing.T) {
 	// Update the token locality
 	code = cmd.Run([]string{
 		"-address=" + url, "-token=" + rootACLToken.SecretID, "-token-locality=global", method.Name})
-	must.Zero(t, code)
+	test.Zero(t, code)
+	test.Eq(t, "", ui.ErrorWriter.String())
 	s := ui.OutputWriter.String()
 	must.StrContains(t, s, method.Name)
 
@@ -92,7 +96,11 @@ func TestACLAuthMethodUpdateCommand_Run(t *testing.T) {
 	defer os.Remove(configFile.Name())
 	must.Nil(t, err)
 
-	conf := map[string]interface{}{"OIDCDiscoveryURL": "http://example.com"}
+	conf := map[string]interface{}{
+		"OIDCDiscoveryURL": "http://example.com",
+		"OIDCClientID":     "example-id",
+		"BoundAudiences":   []string{"example-aud"},
+	}
 	jsonData, err := json.Marshal(conf)
 	must.Nil(t, err)
 
@@ -105,7 +113,8 @@ func TestACLAuthMethodUpdateCommand_Run(t *testing.T) {
 		fmt.Sprintf("-config=@%s", configFile.Name()),
 		method.Name,
 	})
-	must.Zero(t, code)
+	test.Zero(t, code)
+	test.Eq(t, "", ui.ErrorWriter.String())
 	s = ui.OutputWriter.String()
 	must.StrContains(t, s, method.Name)
 

command/agent/acl_endpoint_test.go

@@ -1823,7 +1823,6 @@ func TestHTTPServer_ACLOIDCCompleteAuthRequest(t *testing.T) {
 				oidcTestProvider.SetExpectedAuthNonce("fpSPuaodKevKfDU3IeXa")
 				oidcTestProvider.SetExpectedAuthCode("codeABC")
 				oidcTestProvider.SetCustomAudience("mock")
-				oidcTestProvider.SetExpectedState("st_someweirdstateid")
 				oidcTestProvider.SetCustomClaims(map[string]interface{}{
 					"azp":                            "mock",
 					"http://nomad.internal/policies": []string{"engineering"},
@@ -1834,7 +1833,7 @@ func TestHTTPServer_ACLOIDCCompleteAuthRequest(t *testing.T) {
 				requestBody := structs.ACLOIDCCompleteAuthRequest{
 					AuthMethodName: mockedAuthMethod.Name,
 					ClientNonce:    "fpSPuaodKevKfDU3IeXa",
-					State:          "st_someweirdstateid",
+					State:          "overwrite me",
 					Code:           "codeABC",
 					RedirectURI:    mockedAuthMethod.Config.AllowedRedirectURIs[0],
 					WriteRequest: structs.WriteRequest{
@@ -1842,6 +1841,10 @@ func TestHTTPServer_ACLOIDCCompleteAuthRequest(t *testing.T) {
 					},
 				}
 
+				// Request Auth URL first, as a user would. This primes the
+				// request cache on the server, returns the expected state.
+				requestBody.State = requestAuthState(t, testAgent.Server, mockedAuthMethod, requestBody.ClientNonce)
+
 				// Build the HTTP request.
 				req, err := http.NewRequest(http.MethodPost, "/v1/acl/oidc/complete-auth", encodeReq(&requestBody))
 				must.NoError(t, err)
@@ -1877,6 +1880,10 @@ func TestHTTPServer_ACLOIDCCompleteAuthRequest(t *testing.T) {
 				must.NoError(t, testAgent.server.State().UpsertACLBindingRules(
 					40, []*structs.ACLBindingRule{mockBindingRule1, mockBindingRule2}, true))
 
+				// Request Auth URL first, as a user would. This primes the
+				// request cache on the server, returns the expected state.
+				requestBody.State = requestAuthState(t, testAgent.Server, mockedAuthMethod, requestBody.ClientNonce)
+
 				// Build the HTTP request.
 				req, err = http.NewRequest(http.MethodPost, "/v1/acl/oidc/complete-auth", encodeReq(&requestBody))
 				must.NoError(t, err)
@@ -2036,3 +2043,22 @@ func TestHTTPServer_ACLLoginRequest(t *testing.T) {
 		})
 	}
 }
+
+// requestAuthState hits the oidc/auth-url endpoint, as a user would during
+// normal login, before a subsequent call to oidc/complete-auth. Returns the
+// "state" generated by the server, to be used in ACLOIDCCompleteAuthRequest.
+func requestAuthState(t *testing.T, server *HTTPServer, authMethod *structs.ACLAuthMethod, nonce string) string {
+	t.Helper()
+	urlReq, err := http.NewRequest(http.MethodPost, "/v1/acl/oidc/auth-url", encodeReq(&structs.ACLOIDCAuthURLRequest{
+		AuthMethodName: authMethod.Name,
+		RedirectURI:    authMethod.Config.AllowedRedirectURIs[0],
+		ClientNonce:    nonce,
+		WriteRequest:   structs.WriteRequest{Region: "global"},
+	}))
+	must.NoError(t, err)
+	authURLResp, err := server.ACLOIDCAuthURLRequest(httptest.NewRecorder(), urlReq)
+	must.NoError(t, err)
+	u, err := url.Parse(authURLResp.(structs.ACLOIDCAuthURLResponse).AuthURL)
+	must.NoError(t, err)
+	return u.Query().Get("state")
+}