Skip to content

fix(e2e): use non-admin user for orchestrator RBAC tests#4532

Open
gustavolira wants to merge 12 commits intoredhat-developer:mainfrom
gustavolira:fix/orchestrator-rbac-tests-use-non-admin-user
Open

fix(e2e): use non-admin user for orchestrator RBAC tests#4532
gustavolira wants to merge 12 commits intoredhat-developer:mainfrom
gustavolira:fix/orchestrator-rbac-tests-use-non-admin-user

Conversation

@gustavolira
Copy link
Copy Markdown
Member

@gustavolira gustavolira commented Apr 2, 2026

Summary

  • Orchestrator RBAC tests were using rhdh-qe (configured as RBAC admin in app-config-rhdh-rbac.yaml), which bypasses all permission checks — deny/read-only tests were failing or passing without actually validating RBAC enforcement
  • Switch 5 test blocks to assign roles to rhdh-qe-2 (non-admin) and login as that user for UI verification, while keeping rhdh-qe for API operations (create/delete roles)
  • Fix scaffolder task completion detection in orchestrator-entity-rbac.spec.ts to use output links ("View in catalog" / "Open workflow run") instead of text matching ("Completed/succeeded/finished")

Test plan

  • Verify orchestrator RBAC deny tests pass with rhdh-qe-2
  • Verify orchestrator RBAC read-only tests correctly detect disabled Run button
  • Verify orchestrator entity-rbac tests detect task completion via output links
  • Verify no regression in orchestrator RBAC positive tests (Global Workflow Access)

🤖 Generated with Claude Code

@openshift-ci openshift-ci bot requested review from kadel and rostalan April 2, 2026 14:51
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Apr 2, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Test Flakiness

The new completion check relies on exact accessible link names (View in catalog / Open workflow run). This can be brittle if the UI text changes, is localized, or differs across environments. Consider using a more stable selector (e.g., data-testid) or a regex-based role name match, and ensure the assertions clearly distinguish success vs. failure states.

 
// Wait for the scaffolder task to complete. The "Start Over" button appears
// on both success and failure, so we include it to avoid a 120s timeout if
// the task fails fast.
const viewInCatalog = page.getByRole("link", {
  name: "View in catalog",
});
const openWorkflowRun = page.getByRole("link", {
  name: "Open workflow run",
});
const startOver = page.getByRole("button", { name: "Start Over" });

await expect(
  viewInCatalog.or(openWorkflowRun).or(startOver),
).toBeVisible({
  timeout: 120000,
});

// Verify the task actually succeeded. The output links "View in catalog"
// and "Open workflow run" are defined in the greeting_w_component.yaml
// template and are only rendered when all scaffolder steps pass.
// If the workflow was denied (e.g. missing RBAC permissions), the step
// fails and only "Start Over" is shown — this assertion catches that.
await expect(viewInCatalog.or(openWorkflowRun)).toBeVisible({
  timeout: 10000,
});
Assertion Logic

The viewInCatalog.or(openWorkflowRun).or(startOver) wait may succeed early due to Start Over showing quickly, and the subsequent 10s success assertion might be too tight under slower rendering/backends. It may be worth asserting on an explicit failure indicator when only Start Over is present, or increasing robustness by waiting for either success links or a known error state before deciding the outcome.

 
await expect(
  viewInCatalog.or(openWorkflowRun).or(startOver),
).toBeVisible({
  timeout: 120000,
});

// Verify the task actually succeeded. The output links "View in catalog"
// and "Open workflow run" are defined in the greeting_w_component.yaml
// template and are only rendered when all scaffolder steps pass.
// If the workflow was denied (e.g. missing RBAC permissions), the step
// fails and only "Start Over" is shown — this assertion catches that.
await expect(viewInCatalog.or(openWorkflowRun)).toBeVisible({
  timeout: 10000,
});
Auth Reliability

Multiple tests newly log in via process.env.GH_USER2_ID / process.env.GH_USER2_PASS. Validate that these env vars are required/validated in the test harness (clear error if missing) and that session state is isolated between tests (e.g., ensure prior admin login/cookies don’t leak into these checks). Also consider reducing duplication by centralizing the non-admin login step for the relevant describe blocks to avoid inconsistent future changes.

 
  test("Test global orchestrator workflow read-only access - Run button disabled", async () => {
    // Login as non-admin user to verify RBAC enforcement
    await common.loginAsKeycloakUser(
      process.env.GH_USER2_ID,
      process.env.GH_USER2_PASS,
    );
    await uiHelper.goToPageUrl("/orchestrator");
    await uiHelper.verifyHeading("Workflows");

    const orchestrator = new Orchestrator(page);
    await orchestrator.selectGreetingWorkflowItem();

    // Verify we're on the greeting workflow page
    await expect(
      page.getByRole("heading", { name: "Greeting workflow" }),
    ).toBeVisible();

    // Verify the Run button is either not visible or disabled (read-only access)
    const runButton = page.getByRole("button", { name: "Run" });

    // For read-only access, the button should either not exist or be disabled
    const buttonCount = await runButton.count();

    // Test that either button doesn't exist OR it's disabled
    // eslint-disable-next-line playwright/no-conditional-in-test
    if (buttonCount === 0) {
      // Button doesn't exist - this is valid for read-only access
      // eslint-disable-next-line playwright/no-conditional-expect
      expect(buttonCount).toBe(0);
    } else {
      // Button exists - it should be disabled
      // eslint-disable-next-line playwright/no-conditional-expect
      await expect(runButton).toBeDisabled();
    }
  });

  test.afterAll(async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    try {
      const remainingPoliciesResponse = await rbacApi.getPoliciesByRole(
        "default/workflowReadonly",
      );

      const remainingPolicies = await Response.removeMetadataFromResponse(
        remainingPoliciesResponse,
      );

      const deleteRemainingPolicies = await rbacApi.deletePolicy(
        "default/workflowReadonly",
        remainingPolicies as Policy[],
      );

      const deleteRole = await rbacApi.deleteRole("default/workflowReadonly");

      expect(deleteRemainingPolicies.ok()).toBeTruthy();
      expect(deleteRole.ok()).toBeTruthy();
    } catch (error) {
      console.error("Error during cleanup in afterAll:", error);
    }
  });
});

test.describe
  .serial("Test Orchestrator RBAC: Global Workflow Denied Access", () => {
  test.describe.configure({ retries: 0 });
  let common: Common;
  let uiHelper: UIhelper;
  let page: Page;
  let apiToken: string;

  test.beforeAll(async ({ browser }, testInfo) => {
    page = (await setupBrowser(browser, testInfo)).page;

    uiHelper = new UIhelper(page);
    common = new Common(page);

    await common.loginAsKeycloakUser();
    apiToken = await RhdhAuthApiHack.getToken(page);
  });

  test.beforeEach(async ({}, testInfo) => {
    console.log(
      `beforeEach: Attempting setup for ${testInfo.title}, retry: ${testInfo.retry}`,
    );
  });

  test("Create role with global orchestrator.workflow denied permissions", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);
    const members = ["user:default/rhdh-qe-2"];

    const orchestratorDeniedRole = {
      memberReferences: members,
      name: "role:default/workflowDenied",
    };

    const orchestratorDeniedPolicies = [
      {
        entityReference: "role:default/workflowDenied",
        permission: "orchestrator.workflow",
        policy: "read",
        effect: "deny",
      },
      {
        entityReference: "role:default/workflowDenied",
        permission: "orchestrator.workflow.use",
        policy: "update",
        effect: "deny",
      },
    ];

    const rolePostResponse = await rbacApi.createRoles(
      orchestratorDeniedRole,
    );
    const policyPostResponse = await rbacApi.createPolicies(
      orchestratorDeniedPolicies,
    );

    expect(rolePostResponse.ok()).toBeTruthy();
    expect(policyPostResponse.ok()).toBeTruthy();
  });

  test("Verify denied role exists via API", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    const rolesResponse = await rbacApi.getRoles();
    expect(rolesResponse.ok()).toBeTruthy();

    const roles = await rolesResponse.json();
    const workflowRole = roles.find(
      (role: { name: string; memberReferences: string[] }) =>
        role.name === "role:default/workflowDenied",
    );
    expect(workflowRole).toBeDefined();
    expect(workflowRole?.memberReferences).toContain("user:default/rhdh-qe-2");

    const policiesResponse = await rbacApi.getPoliciesByRole(
      "default/workflowDenied",
    );
    expect(policiesResponse.ok()).toBeTruthy();

    const policies = await policiesResponse.json();
    expect(policies).toHaveLength(2);

    const denyReadPolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow" &&
        policy.policy === "read",
    );
    const denyUpdatePolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.use" &&
        policy.policy === "update",
    );

    expect(denyReadPolicy).toBeDefined();
    expect(denyUpdatePolicy).toBeDefined();
    expect(denyReadPolicy.effect).toBe("deny");
    expect(denyUpdatePolicy.effect).toBe("deny");
  });

  test("Test global orchestrator workflow denied access - no workflows visible", async () => {
    // Login as non-admin user to verify RBAC enforcement
    await common.loginAsKeycloakUser(
      process.env.GH_USER2_ID,
      process.env.GH_USER2_PASS,
    );
    await uiHelper.goToPageUrl("/orchestrator");
    await uiHelper.verifyHeading("Workflows");

    // With denied access, the workflows table should be empty or show no results
    await uiHelper.verifyTableIsEmpty();

    // Alternatively, verify that the Greeting workflow link is not visible
    const greetingWorkflowLink = page.getByRole("link", {
      name: "Greeting workflow",
    });
    await expect(greetingWorkflowLink).toHaveCount(0);
  });

  test.afterAll(async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    try {
      const remainingPoliciesResponse = await rbacApi.getPoliciesByRole(
        "default/workflowDenied",
      );

      const remainingPolicies = await Response.removeMetadataFromResponse(
        remainingPoliciesResponse,
      );

      const deleteRemainingPolicies = await rbacApi.deletePolicy(
        "default/workflowDenied",
        remainingPolicies as Policy[],
      );

      const deleteRole = await rbacApi.deleteRole("default/workflowDenied");

      expect(deleteRemainingPolicies.ok()).toBeTruthy();
      expect(deleteRole.ok()).toBeTruthy();
    } catch (error) {
      console.error("Error during cleanup in afterAll:", error);
    }
  });
});

test.describe
  .serial("Test Orchestrator RBAC: Individual Workflow Denied Access", () => {
  test.describe.configure({ retries: 0 });
  let common: Common;
  let uiHelper: UIhelper;
  let page: Page;
  let apiToken: string;

  test.beforeAll(async ({ browser }, testInfo) => {
    page = (await setupBrowser(browser, testInfo)).page;

    uiHelper = new UIhelper(page);
    common = new Common(page);

    await common.loginAsKeycloakUser();
    apiToken = await RhdhAuthApiHack.getToken(page);
  });

  test.beforeEach(async ({}, testInfo) => {
    console.log(
      `beforeEach: Attempting setup for ${testInfo.title}, retry: ${testInfo.retry}`,
    );
  });

  test("Create role with greeting workflow denied permissions", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);
    const members = ["user:default/rhdh-qe-2"];

    const greetingDeniedRole = {
      memberReferences: members,
      name: "role:default/workflowGreetingDenied",
    };

    const greetingDeniedPolicies = [
      {
        entityReference: "role:default/workflowGreetingDenied",
        permission: "orchestrator.workflow.greeting",
        policy: "read",
        effect: "deny",
      },
      {
        entityReference: "role:default/workflowGreetingDenied",
        permission: "orchestrator.workflow.use.greeting",
        policy: "update",
        effect: "deny",
      },
    ];

    const rolePostResponse = await rbacApi.createRoles(greetingDeniedRole);
    const policyPostResponse = await rbacApi.createPolicies(
      greetingDeniedPolicies,
    );

    expect(rolePostResponse.ok()).toBeTruthy();
    expect(policyPostResponse.ok()).toBeTruthy();
  });

  test("Verify greeting workflow denied role exists via API", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    const rolesResponse = await rbacApi.getRoles();
    expect(rolesResponse.ok()).toBeTruthy();

    const roles = await rolesResponse.json();
    const workflowRole = roles.find(
      (role: { name: string; memberReferences: string[] }) =>
        role.name === "role:default/workflowGreetingDenied",
    );
    expect(workflowRole).toBeDefined();
    expect(workflowRole?.memberReferences).toContain("user:default/rhdh-qe-2");

    const policiesResponse = await rbacApi.getPoliciesByRole(
      "default/workflowGreetingDenied",
    );
    expect(policiesResponse.ok()).toBeTruthy();

    const policies = await policiesResponse.json();
    expect(policies).toHaveLength(2);

    const denyReadPolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.greeting" &&
        policy.policy === "read",
    );
    const denyUpdatePolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.use.greeting" &&
        policy.policy === "update",
    );

    expect(denyReadPolicy).toBeDefined();
    expect(denyUpdatePolicy).toBeDefined();
    expect(denyReadPolicy.effect).toBe("deny");
    expect(denyUpdatePolicy.effect).toBe("deny");
  });

  test("Test individual workflow denied access - no workflows visible", async () => {
    // Login as non-admin user to verify RBAC enforcement
    await common.loginAsKeycloakUser(
      process.env.GH_USER2_ID,
      process.env.GH_USER2_PASS,
    );
    await uiHelper.goToPageUrl("/orchestrator");
    await uiHelper.verifyHeading("Workflows");

    // Verify that the Greeting workflow link is NOT visible (denied)
    const greetingWorkflowLink = page.getByRole("link", {
      name: "Greeting workflow",
    });
    await expect(greetingWorkflowLink).toHaveCount(0);

    // Verify that User Onboarding workflow is also NOT visible (no global permissions)
    const userOnboardingLink = page.getByRole("link", {
      name: "User Onboarding",
    });
    await expect(userOnboardingLink).toHaveCount(0);

    // Verify workflows table is empty (no workflows visible due to individual deny + no global allow)
    await uiHelper.verifyTableIsEmpty();
  });

  test.afterAll(async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    try {
      const remainingPoliciesResponse = await rbacApi.getPoliciesByRole(
        "default/workflowGreetingDenied",
      );

      const remainingPolicies = await Response.removeMetadataFromResponse(
        remainingPoliciesResponse,
      );

      const deleteRemainingPolicies = await rbacApi.deletePolicy(
        "default/workflowGreetingDenied",
        remainingPolicies as Policy[],
      );

      const deleteRole = await rbacApi.deleteRole(
        "default/workflowGreetingDenied",
      );

      expect(deleteRemainingPolicies.ok()).toBeTruthy();
      expect(deleteRole.ok()).toBeTruthy();
    } catch (error) {
      console.error("Error during cleanup in afterAll:", error);
    }
  });
});

test.describe
  .serial("Test Orchestrator RBAC: Individual Workflow Read-Write Access", () => {
  test.describe.configure({ retries: 0 });
  let common: Common;
  let uiHelper: UIhelper;
  let page: Page;
  let apiToken: string;

  test.beforeAll(async ({ browser }, testInfo) => {
    page = (await setupBrowser(browser, testInfo)).page;

    uiHelper = new UIhelper(page);
    common = new Common(page);

    await common.loginAsKeycloakUser();
    apiToken = await RhdhAuthApiHack.getToken(page);
  });

  test.beforeEach(async ({}, testInfo) => {
    console.log(
      `beforeEach: Attempting setup for ${testInfo.title}, retry: ${testInfo.retry}`,
    );
  });

  test("Create role with greeting workflow read-write permissions", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);
    const members = ["user:default/rhdh-qe-2"];

    const greetingReadwriteRole = {
      memberReferences: members,
      name: "role:default/workflowGreetingReadwrite",
    };

    const greetingReadwritePolicies = [
      {
        entityReference: "role:default/workflowGreetingReadwrite",
        permission: "orchestrator.workflow.greeting",
        policy: "read",
        effect: "allow",
      },
      {
        entityReference: "role:default/workflowGreetingReadwrite",
        permission: "orchestrator.workflow.use.greeting",
        policy: "update",
        effect: "allow",
      },
    ];

    const rolePostResponse = await rbacApi.createRoles(greetingReadwriteRole);
    const policyPostResponse = await rbacApi.createPolicies(
      greetingReadwritePolicies,
    );

    expect(rolePostResponse.ok()).toBeTruthy();
    expect(policyPostResponse.ok()).toBeTruthy();
  });

  test("Verify greeting workflow read-write role exists via API", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    const rolesResponse = await rbacApi.getRoles();
    expect(rolesResponse.ok()).toBeTruthy();

    const roles = await rolesResponse.json();
    const workflowRole = roles.find(
      (role: { name: string; memberReferences: string[] }) =>
        role.name === "role:default/workflowGreetingReadwrite",
    );
    expect(workflowRole).toBeDefined();
    expect(workflowRole?.memberReferences).toContain("user:default/rhdh-qe-2");

    const policiesResponse = await rbacApi.getPoliciesByRole(
      "default/workflowGreetingReadwrite",
    );
    expect(policiesResponse.ok()).toBeTruthy();

    const policies = await policiesResponse.json();
    expect(policies).toHaveLength(2);

    const allowReadPolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.greeting" &&
        policy.policy === "read",
    );
    const allowUpdatePolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.use.greeting" &&
        policy.policy === "update",
    );

    expect(allowReadPolicy).toBeDefined();
    expect(allowUpdatePolicy).toBeDefined();
    expect(allowReadPolicy.effect).toBe("allow");
    expect(allowUpdatePolicy.effect).toBe("allow");
  });

  test("Test individual workflow read-write access - only Greeting workflow visible and runnable", async () => {
    // Login as non-admin user to verify RBAC enforcement
    await common.loginAsKeycloakUser(
      process.env.GH_USER2_ID,
      process.env.GH_USER2_PASS,
    );
    await uiHelper.goToPageUrl("/orchestrator");
    await uiHelper.verifyHeading("Workflows");

    // Verify that the Greeting workflow link IS visible (allowed)
    const greetingWorkflowLink = page.getByRole("link", {
      name: "Greeting workflow",
    });
    await expect(greetingWorkflowLink).toBeVisible();

    // Verify that User Onboarding workflow is NOT visible (no global permissions)
    const userOnboardingLink = page.getByRole("link", {
      name: "User Onboarding",
    });
    await expect(userOnboardingLink).toHaveCount(0);

    // Navigate to Greeting workflow and verify we can run it
    await greetingWorkflowLink.click();
    await expect(
      page.getByRole("heading", { name: "Greeting workflow" }),
    ).toBeVisible();

    const runButton = page.getByRole("button", { name: "Run" });
    await expect(runButton).toBeVisible();
    await expect(runButton).toBeEnabled();
    await runButton.click();
  });

  test.afterAll(async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    try {
      const remainingPoliciesResponse = await rbacApi.getPoliciesByRole(
        "default/workflowGreetingReadwrite",
      );

      const remainingPolicies = await Response.removeMetadataFromResponse(
        remainingPoliciesResponse,
      );

      const deleteRemainingPolicies = await rbacApi.deletePolicy(
        "default/workflowGreetingReadwrite",
        remainingPolicies as Policy[],
      );

      const deleteRole = await rbacApi.deleteRole(
        "default/workflowGreetingReadwrite",
      );

      expect(deleteRemainingPolicies.ok()).toBeTruthy();
      expect(deleteRole.ok()).toBeTruthy();
    } catch (error) {
      console.error("Error during cleanup in afterAll:", error);
    }
  });
});

test.describe
  .serial("Test Orchestrator RBAC: Individual Workflow Read-Only Access", () => {
  test.describe.configure({ retries: 0 });
  let common: Common;
  let uiHelper: UIhelper;
  let page: Page;
  let apiToken: string;

  test.beforeAll(async ({ browser }, testInfo) => {
    page = (await setupBrowser(browser, testInfo)).page;

    uiHelper = new UIhelper(page);
    common = new Common(page);

    await common.loginAsKeycloakUser();
    apiToken = await RhdhAuthApiHack.getToken(page);
  });

  test.beforeEach(async ({}, testInfo) => {
    console.log(
      `beforeEach: Attempting setup for ${testInfo.title}, retry: ${testInfo.retry}`,
    );
  });

  test("Create role with greeting workflow read-only permissions", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);
    const members = ["user:default/rhdh-qe-2"];

    const greetingReadonlyRole = {
      memberReferences: members,
      name: "role:default/workflowGreetingReadonly",
    };

    const greetingReadonlyPolicies = [
      {
        entityReference: "role:default/workflowGreetingReadonly",
        permission: "orchestrator.workflow.greeting",
        policy: "read",
        effect: "allow",
      },
      {
        entityReference: "role:default/workflowGreetingReadonly",
        permission: "orchestrator.workflow.use.greeting",
        policy: "update",
        effect: "deny",
      },
    ];

    const rolePostResponse = await rbacApi.createRoles(greetingReadonlyRole);
    const policyPostResponse = await rbacApi.createPolicies(
      greetingReadonlyPolicies,
    );

    expect(rolePostResponse.ok()).toBeTruthy();
    expect(policyPostResponse.ok()).toBeTruthy();
  });

  test("Verify greeting workflow read-only role exists via API", async () => {
    const rbacApi = await RhdhRbacApi.build(apiToken);

    const rolesResponse = await rbacApi.getRoles();
    expect(rolesResponse.ok()).toBeTruthy();

    const roles = await rolesResponse.json();
    const workflowRole = roles.find(
      (role: { name: string; memberReferences: string[] }) =>
        role.name === "role:default/workflowGreetingReadonly",
    );
    expect(workflowRole).toBeDefined();
    expect(workflowRole?.memberReferences).toContain("user:default/rhdh-qe-2");

    const policiesResponse = await rbacApi.getPoliciesByRole(
      "default/workflowGreetingReadonly",
    );
    expect(policiesResponse.ok()).toBeTruthy();

    const policies = await policiesResponse.json();
    expect(policies).toHaveLength(2);

    const allowReadPolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.greeting" &&
        policy.policy === "read",
    );
    const denyUpdatePolicy = policies.find(
      (policy: { permission: string; policy: string; effect: string }) =>
        policy.permission === "orchestrator.workflow.use.greeting" &&
        policy.policy === "update",
    );

    expect(allowReadPolicy).toBeDefined();
    expect(denyUpdatePolicy).toBeDefined();
    expect(allowReadPolicy.effect).toBe("allow");
    expect(denyUpdatePolicy.effect).toBe("deny");
  });

  test("Test individual workflow read-only access - only Greeting workflow visible, Run button disabled", async () => {
    // Login as non-admin user to verify RBAC enforcement
    await common.loginAsKeycloakUser(
      process.env.GH_USER2_ID,
      process.env.GH_USER2_PASS,
    );
    await uiHelper.goToPageUrl("/orchestrator");
📄 References
  1. No matching references available

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Apr 2, 2026

PR Type

Bug fix, Tests

Description

  • Switch orchestrator RBAC tests from admin user rhdh-qe to non-admin rhdh-qe-2 to properly validate permission enforcement

  • Add login calls for non-admin user in 5 UI verification test blocks to ensure RBAC rules are actually enforced

  • Fix scaffolder task completion detection to use output links instead of text matching for more reliable test execution

  • Update all role member references from rhdh-qe to rhdh-qe-2 across 5 test suites (read-only, denied, greeting workflows)

File Walkthrough
Relevant files
Bug fix
orchestrator-rbac.spec.ts
Switch RBAC tests to non-admin user for proper validation

e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-rbac.spec.ts

  • Replace all role member references from rhdh-qe (admin) to rhdh-qe-2
    (non-admin) across 5 test suites
  • Add loginAsKeycloakUser calls with GH_USER2_ID and GH_USER2_PASS
    credentials in 5 UI verification tests
  • Ensures RBAC enforcement is properly validated by testing with
    non-admin user instead of bypassing checks
 +35/-10 
orchestrator-entity-rbac.spec.ts
Fix task completion detection using output links                 

e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-entity-rbac.spec.ts

  • Replace text-based task completion detection
    (/Completed|succeeded|finished/i) with output link selectors
  • Add explicit verification that output links ("View in catalog" or
    "Open workflow run") are visible to confirm task success
  • Add detailed comments explaining that output links only render on
    successful scaffolder completion
  • Prevents false positives when tasks fail fast and only "Start Over"
    button appears
 +21/-5   

@gustavolira
Copy link
Copy Markdown
Member Author

gustavolira commented Apr 2, 2026

/test ?

@gustavolira
Copy link
Copy Markdown
Member Author

gustavolira commented Apr 2, 2026

/test e2e-ocp-helm-nightly

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Apr 2, 2026 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
✅ Clear session before login
Suggestion Impact:Added `await page.context().clearCookies();` before logging in as the non-admin user across multiple RBAC tests to clear the prior admin session (but did not add the suggested navigation to /auth/login).

code diff:

-      // Login as non-admin user to verify RBAC enforcement
+      // Clear admin session and login as non-admin user to verify RBAC enforcement
+      await page.context().clearCookies();
       await common.loginAsKeycloakUser(
         process.env.GH_USER2_ID,
         process.env.GH_USER2_PASS,
@@ -424,7 +425,8 @@
     });
 
     test("Test global orchestrator workflow denied access - no workflows visible", async () => {
-      // Login as non-admin user to verify RBAC enforcement
+      // Clear admin session and login as non-admin user to verify RBAC enforcement
+      await page.context().clearCookies();
       await common.loginAsKeycloakUser(
         process.env.GH_USER2_ID,
         process.env.GH_USER2_PASS,
@@ -566,7 +568,8 @@
     });
 
     test("Test individual workflow denied access - no workflows visible", async () => {
-      // Login as non-admin user to verify RBAC enforcement
+      // Clear admin session and login as non-admin user to verify RBAC enforcement
+      await page.context().clearCookies();
       await common.loginAsKeycloakUser(
         process.env.GH_USER2_ID,
         process.env.GH_USER2_PASS,
@@ -716,7 +719,8 @@
     });
 
     test("Test individual workflow read-write access - only Greeting workflow visible and runnable", async () => {
-      // Login as non-admin user to verify RBAC enforcement
+      // Clear admin session and login as non-admin user to verify RBAC enforcement
+      await page.context().clearCookies();
       await common.loginAsKeycloakUser(
         process.env.GH_USER2_ID,
         process.env.GH_USER2_PASS,
@@ -874,7 +878,8 @@
     });
 
     test("Test individual workflow read-only access - only Greeting workflow visible, Run button disabled", async () => {
-      // Login as non-admin user to verify RBAC enforcement
+      // Clear admin session and login as non-admin user to verify RBAC enforcement
+      await page.context().clearCookies();
       await common.loginAsKeycloakUser(
         process.env.GH_USER2_ID,

Clear cookies and navigate to the login page before logging in as a non-admin
user to ensure a clean session and prevent test flakiness.

e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-rbac.spec.ts [266-270]

 // Login as non-admin user to verify RBAC enforcement
+await page.context().clearCookies();
+await page.goto("/auth/login");
 await common.loginAsKeycloakUser(
   process.env.GH_USER2_ID,
   process.env.GH_USER2_PASS,
 );

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion improves test robustness by ensuring a clean session before logging in as a different user, which helps prevent potential test flakiness.

Low
Validate credential variables

Add a check to validate that GH_USER2_ID and GH_USER2_PASS environment variables
are defined before attempting login, failing fast if they are missing.

e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-rbac.spec.ts [266-270]

 // Login as non-admin user to verify RBAC enforcement
+if (!process.env.GH_USER2_ID || !process.env.GH_USER2_PASS) {
+  throw new Error("Environment variables GH_USER2_ID and GH_USER2_PASS must be set");
+}
 await common.loginAsKeycloakUser(
   process.env.GH_USER2_ID,
   process.env.GH_USER2_PASS,
 );
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: This is a good practice that improves test robustness and debuggability by adding a precondition check for required environment variables, allowing the test to fail fast with a clear error.

Low
General
Use dynamic user reference

Replace the hardcoded user identifier user:default/rhdh-qe-2 with a dynamic
reference using the process.env.GH_USER2_ID variable for better maintainability.

e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-rbac.spec.ts [193]

-const members = ["user:default/rhdh-qe-2"];
+const testUser = `user:default/${process.env.GH_USER2_ID}`;
+const members = [testUser];
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion improves maintainability by replacing a hardcoded user ID with a dynamic reference to an environment variable already used in the test.

Low
  • Update

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Image was built and published successfully. It is available at:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@gustavolira
Copy link
Copy Markdown
Member Author

gustavolira commented Apr 6, 2026

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@gustavolira
Copy link
Copy Markdown
Member Author

gustavolira commented Apr 6, 2026

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

gustavolira and others added 8 commits April 6, 2026 17:15
@gustavolira @claude
fix(e2e): use non-admin user for orchestrator RBAC tests
b41ed4f 
The orchestrator RBAC tests were using `rhdh-qe` which is configured as
an RBAC admin in app-config-rhdh-rbac.yaml. Admin users bypass all
permission checks, so deny/read-only tests were either failing or
passing for the wrong reason (not actually validating RBAC enforcement).

Changes:
- Switch 5 test blocks to use `rhdh-qe-2` (non-admin) for role
  membership and UI verification
- Admin `rhdh-qe` still handles API operations (create/delete roles)
- Fix scaffolder task completion detection in entity-rbac tests to use
  output links instead of text matching

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): fill template form fields before Create in entity-rbac tests
a9b1032 
The orchestrator-entity-rbac tests assumed the "Greeting Test Picker"
template had no input fields and clicked Create directly. The template
actually requires Language and Name fields to be filled, followed by
clicking Review before Create — matching the working pattern in
orchestrator-entity-workflows.spec.ts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): use exact locator for Next button and remove waitForLoadState
ed04524 
- Replace weak regex /Review/i with exact "Next" matching the actual
  stepper button text on Step 1
- Replace /Create/i regex with exact "Create" string
- Remove waitForLoadState("domcontentloaded") — Playwright assertions
  with auto-waiting handle this better

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): clear cookies before re-login as non-admin user
2344193 
loginAsKeycloakUser navigates to / and clicks "Sign In", but the page
already has an active session from beforeAll (rhdh-qe admin). Without
clearing cookies first, the Sign In button is not present and the login
times out.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): move clearCookies + non-admin login to beforeEach
efbddb4 
Move the session switch (clearCookies + loginAsKeycloakUser as
rhdh-qe-2) from each individual UI test into beforeEach, avoiding
repetition across the 5 affected test blocks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): navigate directly to template form URL
22c5e18 
The multi-step navigation (Catalog → entity page → Self-service →
Choose → template form) was unreliable — clickBtnInCard and
verifyHeading could match elements on the wrong page, causing the
test to end up on Home.

Replace with direct URL navigation to
/create/templates/default/greetingComponent, which is deterministic
and doesn't depend on intermediate page transitions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
style: fix prettier formatting
1fcc021 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): restore clickBtnInCard with flexible text verification
2081208 
Navigate to /create, click Choose on the template card, then verify
"Greeting Test Picker" text is present (instead of strict heading match
that could match card titles on the Self-service page).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gustavolira and others added 2 commits April 6, 2026 17:15
@gustavolira @claude
fix(e2e): reload page after clearCookies before re-login
5a30a45 
clearCookies alone doesn't invalidate the active page session — the
browser still shows the old authenticated content. Adding page.goto("/")
+ waitForLoadState("load") after clearCookies forces the browser to
reload without cookies, showing the Sign In page. This matches the
pattern already used in the working tests at lines 1235, 1290, 1455.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira @claude
fix(e2e): remove form field filling — template has no input fields
4ee1e63 
The greeting_w_component.yaml template goes straight to Review with
a Create button (no Language/Name fields). Confirmed via CI screenshot
showing Step 1 = Review with Back + Create buttons only.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira gustavolira force-pushed the fix/orchestrator-rbac-tests-use-non-admin-user branch from c6b7bb1 to 4ee1e63 Compare April 6, 2026 20:16
@gustavolira @claude
fix(e2e): restore catalog template verification test
b8ca736 
Restore the test that verifies the Greeting Test Picker template exists
in the Catalog, but simplified: navigate directly to
/catalog?filters[kind]=template and assert the template link is visible.
No fragile multi-step UI navigation (openSidebar, selectMuiBox, search).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build workflow finished with status: cancelled.

@gustavolira
Copy link
Copy Markdown
Member Author

gustavolira commented Apr 6, 2026

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Image was built and published successfully. It is available at:

@gustavolira @claude
fix(e2e): accept 409 Conflict as valid outcome for template execution
194b656 
The greeting_w_component template registers a catalog entity. On
repeated CI runs the entity already exists, causing a 409 Conflict.
This is expected — the workflow still executed successfully. Accept
409 Conflict alongside success output links as a valid result.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 6, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@gustavolira
Copy link
Copy Markdown
Member Author

gustavolira commented Apr 6, 2026

/test e2e-ocp-helm-nightly

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 7, 2026

@gustavolira: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm 194b656 link true /test e2e-ocp-helm
ci/prow/e2e-ocp-helm-nightly 194b656 link false /test e2e-ocp-helm-nightly

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kadel kadel Awaiting requested review from kadel

@rostalan rostalan Awaiting requested review from rostalan

Assignees

No one assigned

Labels

Bug fix Review effort 3/5 Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gustavolira