Fix users being logged out on any role not found error (#57677)

* fix users being logged out on any role not found error

* CR

lib/auth/auth_with_roles.go

@@ -4747,7 +4747,14 @@ func (a *ServerWithRoles) GetRole(ctx context.Context, name string) (types.Role,
 	// that they hold.  This requirement is checked first to avoid
 	// misleading denial messages in the logs.
 	if slices.Contains(a.context.User.GetRoles(), name) {
-		return a.authServer.GetRole(ctx, name)
+		role, err := a.authServer.GetRole(ctx, name)
+		if err != nil && trace.IsNotFound(err) {
+			// Add the UserSessionRoleNotFoundErrorMsg message to indicate this role not found error was
+			// encountered while the user was looking up one of their own roles. At this point, the
+			// role not found error can only happen if a role in their session cert doesn't exist anymore.
+			return nil, trace.Wrap(err, services.UserSessionRoleNotFoundErrorMsg)
+		}
+		return role, trace.Wrap(err)
 	}
 
 	authErr := a.authorizeAction(types.KindRole, types.VerbRead)

lib/services/access_checker.go

@@ -350,6 +350,27 @@ func NewAccessChecker(info *AccessInfo, localCluster string, access RoleGetter)
 	}, nil
 }
 
+// NewAccessCheckerForUserSession is an alternative to NewAccessChecker that includes a UserSessionRoleNotFoundErrorMsg if
+// a role from the user's session is not found during the access check. This allows the Web UI to distinguish between
+// a user session role lookup error (which should prompt the user to re-login) vs. other role lookup
+// failures.
+func NewAccessCheckerForUserSession(info *AccessInfo, localCluster string, access RoleGetter) (AccessChecker, error) {
+	roleSet, err := FetchRoles(info.Roles, access, info.Traits)
+	if err != nil {
+		if trace.IsNotFound(err) {
+			// Add the UserSessionRoleNotFoundErrorMsg message to indicate this role not found error was encountered fetching
+			// the user's session roles. This can only happen if the user's session certificate contains a role that no longer exists.
+			return nil, trace.Wrap(err, UserSessionRoleNotFoundErrorMsg)
+		}
+		return nil, trace.Wrap(err)
+	}
+	return &accessChecker{
+		info:         info,
+		localCluster: localCluster,
+		RoleSet:      roleSet,
+	}, nil
+}
+
 // NewAccessCheckerWithRoleSet is similar to NewAccessChecker, but accepts the
 // full RoleSet rather than a RoleGetter.
 func NewAccessCheckerWithRoleSet(info *AccessInfo, localCluster string, roleSet RoleSet) AccessChecker {

lib/services/access_checker_test.go

@@ -19,9 +19,11 @@
 package services
 
 import (
+	"context"
 	"sort"
 	"testing"
 
+	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 
 	decisionpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/decision/v1alpha1"
@@ -1160,3 +1162,68 @@ func TestIdentityCenterAccountAccessRequestMatcher(t *testing.T) {
 		})
 	}
 }
+
+// TestUserSessionRoleNotFoundError ensures that role not found errors during user session access checks include UserSessionRoleNotFoundErrorMsg when appropriate,
+func TestUserSessionRoleNotFoundError(t *testing.T) {
+	// Create a mock RoleGetter that returns "role not found" error for a specific role
+	mockRoleGetter := &mockRoleGetter{
+		roles: map[string]types.Role{
+			"existing-role": newRole(func(rv *types.RoleV6) { rv.SetName("existing-role") }),
+		},
+	}
+
+	t.Run("NewAccessChecker with missing role does not add UserSessionRoleNotFoundErrorMsg", func(t *testing.T) {
+		accessInfo := &AccessInfo{
+			Roles: []string{"missing-role"},
+		}
+
+		_, err := NewAccessChecker(accessInfo, "cluster", mockRoleGetter)
+		require.Error(t, err)
+		require.True(t, trace.IsNotFound(err))
+		require.Contains(t, err.Error(), "role missing-role is not found")
+		require.NotContains(t, err.Error(), UserSessionRoleNotFoundErrorMsg)
+	})
+
+	t.Run("NewAccessCheckerForUserSession with missing role adds UserSessionRoleNotFoundErrorMsg", func(t *testing.T) {
+		accessInfo := &AccessInfo{
+			Roles: []string{"missing-role"},
+		}
+
+		_, err := NewAccessCheckerForUserSession(accessInfo, "cluster", mockRoleGetter)
+		require.Error(t, err)
+		require.True(t, trace.IsNotFound(err))
+		require.Contains(t, err.Error(), "role missing-role is not found")
+		require.Contains(t, err.Error(), UserSessionRoleNotFoundErrorMsg)
+	})
+
+	t.Run("NewAccessCheckerForUserSession with existing role succeeds", func(t *testing.T) {
+		accessInfo := &AccessInfo{
+			Roles: []string{"existing-role"},
+		}
+
+		checker, err := NewAccessCheckerForUserSession(accessInfo, "cluster", mockRoleGetter)
+		require.NoError(t, err)
+		require.NotNil(t, checker)
+	})
+}
+
+// mockRoleGetter implements RoleGetter for testing
+type mockRoleGetter struct {
+	roles map[string]types.Role
+}
+
+func (m *mockRoleGetter) GetRole(ctx context.Context, name string) (types.Role, error) {
+	if role, exists := m.roles[name]; exists {
+		return role, nil
+	}
+	// Return the same error format as the real implementation
+	return nil, trace.NotFound("role %v is not found", name)
+}
+
+func (m *mockRoleGetter) GetRoles(ctx context.Context) ([]types.Role, error) {
+	var roles []types.Role
+	for _, role := range m.roles {
+		roles = append(roles, role)
+	}
+	return roles, nil
+}

lib/services/role.go

@@ -3651,6 +3651,13 @@ const (
 	MFARequiredPerRole MFARequired = "per-role"
 )
 
+// UserSessionRoleNotFoundErrorMsg is added to "role not found" errors when they occur
+// during user session roles validation. This allows the Web UI to distinguish between
+// a user session role lookup error (which should prompt the user to re-login) vs. other role lookup
+// failures.
+// Keep in sync with teleport/src/services/api/api.ts(isUserSessionRoleNotFoundError)
+const UserSessionRoleNotFoundErrorMsg = "user session role not found"
+
 // UnmarshalRole unmarshals the Role resource from JSON.
 func UnmarshalRole(bytes []byte, opts ...MarshalOption) (types.Role, error) {
 	return UnmarshalRoleV6(bytes, opts...)

lib/web/sessions.go

@@ -515,7 +515,7 @@ func (c *SessionContext) GetUserAccessChecker() (services.AccessChecker, error)
 
 	accessInfo := services.AccessInfoFromLocalSSHIdentity(ident)
 
-	accessChecker, err := services.NewAccessChecker(accessInfo, c.cfg.RootClusterName, c.cfg.UnsafeCachedAuthClient)
+	accessChecker, err := services.NewAccessCheckerForUserSession(accessInfo, c.cfg.RootClusterName, c.cfg.UnsafeCachedAuthClient)
 	return accessChecker, trace.Wrap(err)
 }
 

web/packages/teleport/src/services/api/api.test.ts

@@ -21,7 +21,7 @@ import websession from '../websession';
 import api, {
   defaultRequestOptions,
   getAuthHeaders,
-  isRoleNotFoundError,
+  isUserSessionRoleNotFoundError,
   MFA_HEADER,
 } from './api';
 import { ApiError } from './parseError';
@@ -205,31 +205,45 @@ test('fetchJsonWithMfaAuthnRetry does not return any', () => {
   expect(true).toBe(true);
 });
 
-test('isRoleNotFoundError correctly identifies role not found errors', () => {
-  const errorMessage1 = 'role admin is not found';
-  expect(isRoleNotFoundError(errorMessage1)).toBe(true);
+test('isUserSessionRoleNotFoundError correctly identifies user session role not found errors', () => {
+  const userSessionRoleError = {
+    error: 'role admin is not found',
+    messages: ['user session role not found'],
+  };
+  expect(isUserSessionRoleNotFoundError(userSessionRoleError)).toBe(true);
 
-  const errorMessage2 = '    role test-role is not found ';
-  expect(isRoleNotFoundError(errorMessage2)).toBe(true);
+  const regularRoleError = { error: 'role admin is not found' };
+  expect(isUserSessionRoleNotFoundError(regularRoleError)).toBe(false);
 
-  const errorMessage3 = 'failed to list access lists';
-  expect(isRoleNotFoundError(errorMessage3)).toBe(false);
+  const regularRoleWithIrrelevantMessages = {
+    error: 'role admin is not found',
+    messages: ['some_other_message'],
+  };
+  expect(
+    isUserSessionRoleNotFoundError(regularRoleWithIrrelevantMessages)
+  ).toBe(false);
+
+  const nonRoleError = { error: 'failed to list access lists' };
+  expect(isUserSessionRoleNotFoundError(nonRoleError)).toBe(false);
 });
 
-describe('api.get handling of role not found errors', () => {
-  beforeEach(() => {
-    jest.spyOn(global, 'fetch').mockResolvedValue({
-      json: async () => ({ error: { message: 'role foo is not found' } }),
-      ok: false,
-      status: 404,
-    } as Response); // we don't care about response
-  });
-
+describe('handling of role not found errors', () => {
   afterEach(() => {
     jest.resetAllMocks();
   });
 
-  test('sign out on error', async () => {
+  test('sign out on user session role not found error', async () => {
+    jest.spyOn(global, 'fetch').mockResolvedValue({
+      json: async () => ({
+        error: {
+          message: 'role foo is not found',
+        },
+        messages: ['user session role not found'],
+      }),
+      ok: false,
+      status: 404,
+    } as Response);
+
     await api.get('/foobar');
     expect(mockedWebsession.logoutWithoutSlo).toHaveBeenCalledWith({
       rememberLocation: false,
@@ -237,10 +251,14 @@ describe('api.get handling of role not found errors', () => {
     });
   });
 
-  test("don't sign out on error", async () => {
-    await expect(
-      api.get('/foobar', null, null, { allowRoleNotFound: true })
-    ).rejects.toThrow(ApiError);
+  test("don't sign out on regular role not found error", async () => {
+    jest.spyOn(global, 'fetch').mockResolvedValue({
+      json: async () => ({ error: { message: 'role foo is not found' } }),
+      ok: false,
+      status: 404,
+    } as Response);
+
+    await expect(api.get('/foobar')).rejects.toThrow(ApiError);
     expect(mockedWebsession.logoutWithoutSlo).not.toHaveBeenCalled();
   });
 });

web/packages/teleport/src/services/api/api.ts

@@ -28,15 +28,6 @@ import parseError, { ApiError, parseProxyVersion } from './parseError';
 export const MFA_HEADER = 'Teleport-Mfa-Response';
 
 type RequestOptions = {
-  /**
-   * Usually, an HTTP/404 with a "role not found" message means that the user
-   * can't be authorized and needs to sign in again. In such case the API
-   * service immediately signs the user out. Setting this flag to `true`
-   * overrides this behavior and allows a "role not found" error to be
-   * propagated up the call stack.
-   */
-  allowRoleNotFound?: boolean;
-
   /**
    * If set to `true`, the API service will not attempt to retry after an MFA
    * challenge.
@@ -203,7 +194,7 @@ const api = {
   ): Promise<any> {
     try {
       const response = await api.fetch(url, customOptions, mfaResponse);
-      return await api.getJsonFromFetchResponse(response, options);
+      return await api.getJsonFromFetchResponse(response);
     } catch (err) {
       // Retry with MFA if we get an admin action MFA error.
       if (
@@ -213,15 +204,14 @@ const api = {
       ) {
         mfaResponse = await api.getAdminActionMfaResponse();
         const response = await api.fetch(url, customOptions, mfaResponse);
-        return await api.getJsonFromFetchResponse(response, options);
+        return await api.getJsonFromFetchResponse(response);
       } else {
         throw err;
       }
     }
   },
 
-  async getJsonFromFetchResponse(response: Response, options: RequestOptions) {
-    const { allowRoleNotFound = false } = options;
+  async getJsonFromFetchResponse(response: Response) {
     let json;
     try {
       json = await response.json();
@@ -238,9 +228,8 @@ const api = {
     }
 
     /** This error can occur in the edge case where a role in the user's certificate was deleted during their session. */
-    const isRoleNotFoundErr =
-      !allowRoleNotFound && isRoleNotFoundError(parseError(json));
-    if (isRoleNotFoundErr) {
+    const isUserSessionRoleNotFoundErr = isUserSessionRoleNotFoundError(json);
+    if (isUserSessionRoleNotFoundErr) {
       websession.logoutWithoutSlo({
         /* Don't remember location after login, since they may no longer have access to the page they were on. */
         rememberLocation: false,
@@ -394,10 +383,12 @@ export function isAdminActionRequiresMfaError(err: Error) {
   );
 }
 
-/** isRoleNotFoundError returns true if the error message is due to a role not being found. */
-export function isRoleNotFoundError(errMessage: string): boolean {
-  // This error message format should be kept in sync with the NotFound error message returned in lib/services/local/access.GetRole
-  return /role \S+ is not found/.test(errMessage);
+/** isUserSessionRoleNotFoundError returns true if the error is a role not found error encountered durings user session role validation */
+export function isUserSessionRoleNotFoundError(json: any): boolean {
+  // Keep in sync with lib/services/role.go(UserSessionRoleNotFoundError)
+  return (
+    !!json.error && !!json?.messages?.includes('user session role not found')
+  );
 }
 
 export default api;

web/packages/teleport/src/services/resources/resource.ts

@@ -174,10 +174,7 @@ class ResourceService {
       await api.get(
         cfg.getRoleUrl({ action: 'get', name }),
         undefined,
-        undefined,
-        {
-          allowRoleNotFound: true,
-        }
+        undefined
       )
     );
   }
@@ -252,9 +249,7 @@ export async function fetchRole(
   signal?: AbortSignal
 ): Promise<RoleResource> {
   return makeResource<'role'>(
-    await api.get(cfg.getRoleUrl({ action: 'get', name }), signal, undefined, {
-      allowRoleNotFound: true,
-    })
+    await api.get(cfg.getRoleUrl({ action: 'get', name }), signal, undefined)
   );
 }