fix: return OpaqueAccessDenied for NotFound remote cluster errors

integration/integration_test.go

@@ -2441,7 +2441,7 @@ func testInvalidLogins(t *testing.T, suite *integrationTestSuite) {
 	require.NoError(t, err)
 
 	err = tc.SSH(context.Background(), cmd, false)
-	require.ErrorIs(t, err, trace.NotFound("failed to dial target host\n\tcluster \"wrong-site\" is not found"))
+	require.ErrorIs(t, err, trace.NotFound("failed to dial target host\n\tlooking up remote cluster \"wrong-site\"\n\t\tnot found"))
 }
 
 // TestTwoClustersTunnel creates two teleport clusters: "a" and "b" and creates a

lib/auth/auth_with_roles.go

@@ -4947,7 +4947,7 @@ func (a *ServerWithRoles) GetRemoteCluster(ctx context.Context, clusterName stri
 	}
 	cluster, err := a.authServer.GetRemoteCluster(ctx, clusterName)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, utils.OpaqueAccessDenied(err)
 	}
 	if err := a.context.Checker.CheckAccessToRemoteCluster(cluster); err != nil {
 		return nil, utils.OpaqueAccessDenied(err)

lib/auth/presence/presencev1/service.go

@@ -126,7 +126,7 @@ func (s *Service) GetRemoteCluster(
 
 	rc, err := s.backend.GetRemoteCluster(ctx, req.Name)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, utils.OpaqueAccessDenied(err)
 	}
 
 	if err := authCtx.Checker.CheckAccessToRemoteCluster(rc); err != nil {

lib/auth/presence/presencev1/service_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/gravitational/trace"
+	"github.com/gravitational/trace/trail"
 	"github.com/jonboulle/clockwork"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/testing/protocmp"
@@ -143,7 +144,6 @@ func TestGetRemoteCluster(t *testing.T) {
 				Name: matchingRC.GetName(),
 			},
 			assertError: func(t require.TestingT, err error, i ...interface{}) {
-				// Opaque no permission presents as not found
 				require.True(t, trace.IsAccessDenied(err), "error should be access denied")
 			},
 		},
@@ -154,6 +154,7 @@ func TestGetRemoteCluster(t *testing.T) {
 				Name: notMatchingRC.GetName(),
 			},
 			assertError: func(t require.TestingT, err error, i ...interface{}) {
+				// Opaque no permission presents as not found
 				require.True(t, trace.IsNotFound(err), "error should be not found")
 			},
 		},
@@ -175,7 +176,7 @@ func TestGetRemoteCluster(t *testing.T) {
 				Name: "non-existent",
 			},
 			assertError: func(t require.TestingT, err error, i ...interface{}) {
-				require.True(t, trace.IsNotFound(err), "error should be bad parameter")
+				require.True(t, trace.IsNotFound(err), "error should be not found")
 			},
 		},
 	}
@@ -184,14 +185,33 @@ func TestGetRemoteCluster(t *testing.T) {
 			client, err := srv.NewClient(auth.TestUser(tt.user))
 			require.NoError(t, err)
 
-			bot, err := client.PresenceServiceClient().GetRemoteCluster(ctx, tt.req)
+			rc, err := client.PresenceServiceClient().GetRemoteCluster(ctx, tt.req)
 			tt.assertError(t, err)
 			if tt.want != nil {
-				// Check that the returned bot matches
-				require.Empty(t, cmp.Diff(tt.want, bot, protocmp.Transform()))
+				// Check that the returned remote cluster matches
+				require.Empty(t, cmp.Diff(tt.want, rc, protocmp.Transform()))
 			}
 		})
 	}
+
+	t.Run("doesnt exist and no permissions errors match", func(t *testing.T) {
+		client, err := srv.NewClient(auth.TestUser(user.GetName()))
+		require.NoError(t, err)
+
+		_, doesntExistError := client.PresenceServiceClient().GetRemoteCluster(ctx, &presencev1pb.GetRemoteClusterRequest{
+			Name: "non-existent",
+		})
+		require.Error(t, doesntExistError)
+		_, noPermissionsError := client.PresenceServiceClient().GetRemoteCluster(ctx, &presencev1pb.GetRemoteClusterRequest{
+			Name: notMatchingRC.GetName(),
+		})
+		require.Error(t, noPermissionsError)
+
+		require.Equal(t, doesntExistError.Error(), noPermissionsError.Error(),
+			"the error message returned when the rc doesn't exist or when the user has no permission to see it should be indistinguishable")
+		require.Equal(t, trail.ToGRPC(doesntExistError), trail.ToGRPC(noPermissionsError),
+			"the gRPC error returned when the rc doesn't exist or when the user has no permission to see it should be indistinguishable")
+	})
 }
 
 // TestListRemoteClusters is an integration test that uses a real gRPC

lib/proxy/router.go

@@ -224,7 +224,7 @@ func (r *Router) DialHost(ctx context.Context, clientSrcAddr, clientDstAddr net.
 	if clusterName != r.clusterName {
 		remoteSite, err := r.getRemoteCluster(ctx, clusterName, accessChecker)
 		if err != nil {
-			return nil, trace.Wrap(err)
+			return nil, trace.Wrap(err, "looking up remote cluster %q", clusterName)
 		}
 		site = remoteSite
 	}
@@ -365,12 +365,12 @@ func (r *Router) getRemoteCluster(ctx context.Context, clusterName string, check
 
 	site, err := r.siteGetter.GetSite(clusterName)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, utils.OpaqueAccessDenied(err)
 	}
 
 	rc, err := r.clusterGetter.GetRemoteCluster(ctx, clusterName)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, utils.OpaqueAccessDenied(err)
 	}
 
 	if err := checker.CheckAccessToRemoteCluster(rc); err != nil {

lib/reversetunnelclient/api_with_roles.go

@@ -94,14 +94,14 @@ func (t *TunnelWithRoles) GetSite(clusterName string) (RemoteSite, error) {
 	ctx := context.TODO()
 	cluster, err := t.tunnel.GetSite(clusterName)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, utils.OpaqueAccessDenied(err)
 	}
 	if t.localCluster == cluster.GetName() {
 		return cluster, nil
 	}
 	rc, err := t.access.GetRemoteCluster(ctx, clusterName)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, utils.OpaqueAccessDenied(err)
 	}
 	if err := t.accessChecker.CheckAccessToRemoteCluster(rc); err != nil {
 		return nil, utils.OpaqueAccessDenied(err)

lib/utils/utils.go

@@ -414,10 +414,11 @@ func IsCertExpiredError(err error) bool {
 	return strings.Contains(trace.Unwrap(err).Error(), "ssh: cert has expired")
 }
 
-// OpaqueAccessDenied returns a generic NotFound instead of AccessDenied
-// so as to avoid leaking the existence of secret resources.
+// OpaqueAccessDenied returns a generic [trace.NotFoundError] if [err] is a [trace.NotFoundError] or
+// a [trace.AccessDeniedError] so as to avoid leaking the existence of secret resources,
+// for other error types it returns the original error.
 func OpaqueAccessDenied(err error) error {
-	if trace.IsAccessDenied(err) {
+	if trace.IsNotFound(err) || trace.IsAccessDenied(err) {
 		return trace.NotFound("not found")
 	}
 	return trace.Wrap(err)