Remove announced deprecated code allowing ports in target addrs (#4265)

We made it an error in the API to do this a while back; this cleans up
the logic to disallow it for existing targets too.

CHANGELOG.md

@@ -10,6 +10,11 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
   removed. Since 0.13.0, unless the `use_deprecated_kms_auth_method` value was
   set on the worker config, the new `kms` mechanism was already being used; this
   is simply no longer an available option.
+* Per the notes in Boundary 0.12.0 and 0.14.0, it is now an error if an address
+  on a host or target contains a port. As of this release, this restriction also
+  affects existing addresses (not just creation/updating via the API) so any
+  existing addresses containing a port will not be able to be used as part of a
+  target's session authorization call.
 
 ### New and Improved
 

internal/daemon/controller/handlers/targets/target_service.go

@@ -935,22 +935,16 @@ func (s Service) AuthorizeSession(ctx context.Context, req *pbs.AuthorizeSession
 	}
 
 	// Ensure we don't have a port from the address, which would be unexpected
-	// FIXME: We've decided to hold off on making this an error until 0.14. In
-	// the meantime, ignore any port coming from the host address.
-	hostWithoutPort, _, err := net.SplitHostPort(h)
+	_, _, err = net.SplitHostPort(h)
 	switch {
 	case err != nil && strings.Contains(err.Error(), globals.MissingPortErrStr):
 		// This is what we expect
 	case err != nil:
 		return nil, errors.Wrap(ctx, err, op, errors.WithMsg("error when parsing the chosen endpoint host address"))
 	case err == nil:
-		h = hostWithoutPort
-		// Use below logic for 0.14:
-		/*
-			return nil, handlers.ApiErrorWithCodeAndMessage(
-				codes.FailedPrecondition,
-				"Address specified for use unexpectedly contains a port.")
-		*/
+		return nil, handlers.ApiErrorWithCodeAndMessage(
+			codes.FailedPrecondition,
+			"Address specified for use unexpectedly contains a port.")
 	}
 
 	// Generate the endpoint URL

internal/daemon/controller/handlers/targets/tcp/target_service_test.go

@@ -2916,7 +2916,6 @@ func TestAuthorizeSession(t *testing.T) {
 	shsWithPort := static.TestSets(t, conn, hcWithPort.GetPublicId(), 1)[0]
 	_ = static.TestSetMembers(t, conn, shsWithPort.GetPublicId(), []*static.Host{hWithPort})
 	hWithPortBareAddress := hWithPort.GetAddress()
-	hWithPort.Address = fmt.Sprintf("%s:54321", hWithPort.GetAddress())
 	hWithPort, _, err = staticRepo.UpdateHost(ctx, hcWithPort.GetProjectId(), hWithPort, hWithPort.GetVersion(), []string{"address"})
 	require.NoError(t, err)
 
@@ -3822,7 +3821,6 @@ func TestAuthorizeSession_Errors(t *testing.T) {
 			HostSourceIds: []string{hs.GetPublicId()},
 		})
 		require.NoError(t, err)
-		h.Address = fmt.Sprintf("%s:54321", h.GetAddress())
 		repo, err := staticHostRepoFn()
 		require.NoError(t, err)
 		_, _, err = repo.UpdateHost(ctx, hc.GetProjectId(), h, h.GetVersion(), []string{"address"})
@@ -3931,7 +3929,6 @@ func TestAuthorizeSession_Errors(t *testing.T) {
 			name:            "no worker",
 			setup:           []func(tcpTarget target.Target) uint32{hostExists, libraryExists},
 			useTargetId:     true,
-			wantErr:         true,
 			wantErrContains: "No workers are available to handle this session",
 		},
 		{
@@ -3943,7 +3940,6 @@ func TestAuthorizeSession_Errors(t *testing.T) {
 			name:            "no target",
 			setup:           []func(tcpTarget target.Target) uint32{workerExists, hostExists, libraryExists},
 			useTargetId:     false,
-			wantErr:         true,
 			wantErrContains: "Resource not found",
 		},
 		{
@@ -3951,25 +3947,37 @@ func TestAuthorizeSession_Errors(t *testing.T) {
 			setup:       []func(tcpTarget target.Target) uint32{workerExists, hostWithoutPort, libraryExists},
 			useTargetId: true,
 		},
+		{
+			name: "host port",
+			setup: []func(tcpTarget target.Target) uint32{
+				workerExists, func(tcpTarget target.Target) uint32 {
+					tcpTarget.SetAddress("127.0.0.1:22")
+					repo, err := repoFn()
+					require.NoError(t, err)
+					tcpTarget, _, err = repo.UpdateTarget(ctx, tcpTarget, tcpTarget.GetVersion(), []string{"address"})
+					require.NoError(t, err)
+					return tcpTarget.GetVersion()
+				},
+			},
+			wantErrContains: "Address specified for use unexpectedly contains a port",
+			useTargetId:     true,
+		},
 		{
 			name:            "no hosts",
 			setup:           []func(tcpTarget target.Target) uint32{workerExists, hostSetNoHostExists, libraryExists},
 			useTargetId:     true,
-			wantErr:         true,
 			wantErrContains: "No host sources or address found for given target",
 		},
 		{
 			name:            "bad library configuration",
 			setup:           []func(tcpTarget target.Target) uint32{workerExists, hostExists, misConfiguredlibraryExists},
 			useTargetId:     true,
-			wantErr:         true,
 			wantErrContains: "external system issue: error #3014: Error making API request",
 		},
 		{
 			name:            "expired token library",
 			setup:           []func(tcpTarget target.Target) uint32{workerExists, hostExists, expiredTokenLibrary},
 			useTargetId:     true,
-			wantErr:         true,
 			wantErrContains: "vault.newClient: invalid configuration",
 		},
 	}
@@ -3990,7 +3998,7 @@ func TestAuthorizeSession_Errors(t *testing.T) {
 			res, err := s.AuthorizeSession(ctx, &pbs.AuthorizeSessionRequest{
 				Id: id,
 			})
-			if tc.wantErr {
+			if tc.wantErrContains != "" {
 				require.Error(t, err)
 				require.Nil(t, res)
 				require.ErrorContains(t, err, tc.wantErrContains)

internal/target/repository.go

@@ -659,9 +659,9 @@ func (r *Repository) UpdateTarget(ctx context.Context, target Target, version ui
 		return nil, db.NoRowsAffected, errors.New(ctx, errors.EmptyFieldMask, op, "empty field mask")
 	}
 
-	// The Address field is not apart of the target schema in the database.
-	// It is apart of a different table called target_address, which is why
-	// the Address field must be filtered out of the dbMask & nullFields slices.
+	// The Address field is not a part of the target schema in the database. It
+	// is a part of a different table called target_address, which is why the
+	// Address field must be filtered out of the dbMask & nullFields slices.
 	var updateAddress, deleteAddress bool
 	var filteredDbMask, filteredNullFields []string
 	for _, f := range dbMask {