Add tests all the way up through the endpoints to ensure duplicate src/destination is supported and so ultimately deny/allow nesting works.

Also adds a sanity check test for `api.Agent().ConnectAuthorize()` and a fix for a trivial bug in it.

agent/agent_endpoint_test.go

@@ -2293,6 +2293,84 @@ func TestAgentConnectAuthorize_deny(t *testing.T) {
 	assert.Contains(obj.Reason, "Matched")
 }
 
+func TestAgentConnectAuthorize_denyWildcard(t *testing.T) {
+	t.Parallel()
+
+	assert := assert.New(t)
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	target := "db"
+
+	// Create some intentions
+	{
+		// Deny wildcard to DB
+		req := structs.IntentionRequest{
+			Datacenter: "dc1",
+			Op:         structs.IntentionOpCreate,
+			Intention:  structs.TestIntention(t),
+		}
+		req.Intention.SourceNS = structs.IntentionDefaultNamespace
+		req.Intention.SourceName = "*"
+		req.Intention.DestinationNS = structs.IntentionDefaultNamespace
+		req.Intention.DestinationName = target
+		req.Intention.Action = structs.IntentionActionDeny
+
+		var reply string
+		assert.Nil(a.RPC("Intention.Apply", &req, &reply))
+	}
+	{
+		// Allow web to DB
+		req := structs.IntentionRequest{
+			Datacenter: "dc1",
+			Op:         structs.IntentionOpCreate,
+			Intention:  structs.TestIntention(t),
+		}
+		req.Intention.SourceNS = structs.IntentionDefaultNamespace
+		req.Intention.SourceName = "web"
+		req.Intention.DestinationNS = structs.IntentionDefaultNamespace
+		req.Intention.DestinationName = target
+		req.Intention.Action = structs.IntentionActionAllow
+
+		var reply string
+		assert.Nil(a.RPC("Intention.Apply", &req, &reply))
+	}
+
+	// Web should be allowed
+	{
+		args := &structs.ConnectAuthorizeRequest{
+			Target:        target,
+			ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(),
+		}
+		req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
+		resp := httptest.NewRecorder()
+		respRaw, err := a.srv.AgentConnectAuthorize(resp, req)
+		assert.Nil(err)
+		assert.Equal(200, resp.Code)
+
+		obj := respRaw.(*connectAuthorizeResp)
+		assert.True(obj.Authorized)
+		assert.Contains(obj.Reason, "Matched")
+	}
+
+	// API should be denied
+	{
+		args := &structs.ConnectAuthorizeRequest{
+			Target:        target,
+			ClientCertURI: connect.TestSpiffeIDService(t, "api").URI().String(),
+		}
+		req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args))
+		resp := httptest.NewRecorder()
+		respRaw, err := a.srv.AgentConnectAuthorize(resp, req)
+		assert.Nil(err)
+		assert.Equal(200, resp.Code)
+
+		obj := respRaw.(*connectAuthorizeResp)
+		assert.False(obj.Authorized)
+		assert.Contains(obj.Reason, "Matched")
+	}
+}
+
 // Test that authorize fails without service:write for the target service.
 func TestAgentConnectAuthorize_serviceWrite(t *testing.T) {
 	t.Parallel()

agent/consul/intention_endpoint_test.go

@@ -830,12 +830,13 @@ func TestIntentionMatch_good(t *testing.T) {
 	// Create some records
 	{
 		insert := [][]string{
-			{"foo", "*"},
-			{"foo", "bar"},
-			{"foo", "baz"}, // shouldn't match
-			{"bar", "bar"}, // shouldn't match
-			{"bar", "*"},   // shouldn't match
-			{"*", "*"},
+			{"foo", "*", "foo", "*"},
+			{"foo", "*", "foo", "bar"},
+			{"foo", "*", "foo", "baz"}, // shouldn't match
+			{"foo", "*", "bar", "bar"}, // shouldn't match
+			{"foo", "*", "bar", "*"},   // shouldn't match
+			{"foo", "*", "*", "*"},
+			{"bar", "*", "foo", "bar"}, // duplicate destination different source
 		}
 
 		for _, v := range insert {
@@ -843,10 +844,10 @@ func TestIntentionMatch_good(t *testing.T) {
 				Datacenter: "dc1",
 				Op:         structs.IntentionOpCreate,
 				Intention: &structs.Intention{
-					SourceNS:        "default",
-					SourceName:      "test",
-					DestinationNS:   v[0],
-					DestinationName: v[1],
+					SourceNS:        v[0],
+					SourceName:      v[1],
+					DestinationNS:   v[2],
+					DestinationName: v[3],
 					Action:          structs.IntentionActionAllow,
 				},
 			}
@@ -874,10 +875,20 @@ func TestIntentionMatch_good(t *testing.T) {
 	assert.Nil(msgpackrpc.CallWithCodec(codec, "Intention.Match", req, &resp))
 	assert.Len(resp.Matches, 1)
 
-	expected := [][]string{{"foo", "bar"}, {"foo", "*"}, {"*", "*"}}
+	expected := [][]string{
+		{"bar", "*", "foo", "bar"},
+		{"foo", "*", "foo", "bar"},
+		{"foo", "*", "foo", "*"},
+		{"foo", "*", "*", "*"},
+	}
 	var actual [][]string
 	for _, ixn := range resp.Matches[0] {
-		actual = append(actual, []string{ixn.DestinationNS, ixn.DestinationName})
+		actual = append(actual, []string{
+			ixn.SourceNS,
+			ixn.SourceName,
+			ixn.DestinationNS,
+			ixn.DestinationName,
+		})
 	}
 	assert.Equal(expected, actual)
 }

agent/intentions_endpoint_test.go

@@ -74,12 +74,13 @@ func TestIntentionsMatch_basic(t *testing.T) {
 	// Create some intentions
 	{
 		insert := [][]string{
-			{"foo", "*"},
-			{"foo", "bar"},
-			{"foo", "baz"}, // shouldn't match
-			{"bar", "bar"}, // shouldn't match
-			{"bar", "*"},   // shouldn't match
-			{"*", "*"},
+			{"foo", "*", "foo", "*"},
+			{"foo", "*", "foo", "bar"},
+			{"foo", "*", "foo", "baz"}, // shouldn't match
+			{"foo", "*", "bar", "bar"}, // shouldn't match
+			{"foo", "*", "bar", "*"},   // shouldn't match
+			{"foo", "*", "*", "*"},
+			{"bar", "*", "foo", "bar"}, // duplicate destination different source
 		}
 
 		for _, v := range insert {
@@ -88,8 +89,10 @@ func TestIntentionsMatch_basic(t *testing.T) {
 				Op:         structs.IntentionOpCreate,
 				Intention:  structs.TestIntention(t),
 			}
-			ixn.Intention.DestinationNS = v[0]
-			ixn.Intention.DestinationName = v[1]
+			ixn.Intention.SourceNS = v[0]
+			ixn.Intention.SourceName = v[1]
+			ixn.Intention.DestinationNS = v[2]
+			ixn.Intention.DestinationName = v[3]
 
 			// Create
 			var reply string
@@ -108,9 +111,19 @@ func TestIntentionsMatch_basic(t *testing.T) {
 	assert.Len(value, 1)
 
 	var actual [][]string
-	expected := [][]string{{"foo", "bar"}, {"foo", "*"}, {"*", "*"}}
+	expected := [][]string{
+		{"bar", "*", "foo", "bar"},
+		{"foo", "*", "foo", "bar"},
+		{"foo", "*", "foo", "*"},
+		{"foo", "*", "*", "*"},
+	}
 	for _, ixn := range value["foo/bar"] {
-		actual = append(actual, []string{ixn.DestinationNS, ixn.DestinationName})
+		actual = append(actual, []string{
+			ixn.SourceNS,
+			ixn.SourceName,
+			ixn.DestinationNS,
+			ixn.DestinationName,
+		})
 	}
 
 	assert.Equal(expected, actual)

api/agent.go

@@ -530,7 +530,7 @@ func (a *Agent) ConnectAuthorize(auth *AgentAuthorizeParams) (*AgentAuthorize, e
 	if err != nil {
 		return nil, err
 	}
-	resp.Body.Close()
+	defer resp.Body.Close()
 
 	var out AgentAuthorize
 	if err := decodeBody(resp, &out); err != nil {

api/agent_test.go

@@ -996,3 +996,26 @@ func TestAPI_AgentConnectCARoots_empty(t *testing.T) {
 	require.Equal(uint64(0), meta.LastIndex)
 	require.Len(list.Roots, 0)
 }
+
+// TODO(banks): once we have CA stuff setup properly we can probably make this
+// much more complete. This is just a sanity check that the agent code basically
+// works.
+func TestAPI_AgentConnectAuthorize(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	agent := c.Agent()
+	params := &AgentAuthorizeParams{
+		Target:           "foo",
+		ClientCertSerial: "fake",
+		// Importing connect.TestSpiffeIDService creates an import cycle
+		ClientCertURI: "spiffe://123.consul/ns/default/dc/ny1/svc/web",
+	}
+	auth, err := agent.ConnectAuthorize(params)
+	require.Nil(err)
+	require.True(auth.Authorized)
+	require.Equal(auth.Reason, "ACLs disabled, access is allowed by default")
+}