Skip to content

test(playwright): add nightly SSO login spec starting#27164

Merged
siddhant1 merged 37 commits intomainfrom
test/playwright/sso-login-okta
Apr 17, 2026
Merged

test(playwright): add nightly SSO login spec starting#27164
siddhant1 merged 37 commits intomainfrom
test/playwright/sso-login-okta

Conversation

@siddhant1
Copy link
Copy Markdown
Member

@siddhant1 siddhant1 commented Apr 8, 2026

Docs: https://www.notion.so/collate-inc/SSO-LOGIN-NIGHTLY-33db4572302480f5b8dcda6e9949969c

Nightly end-to-end Playwright suite that drives a real login round-trip through an external IdP and verifies the resulting OpenMetadata session. Okta in phase 1.

Changes

  • playwright/e2e/Auth/SSOLogin.spec.ts — asserts the SSO button renders, completes login + self-signup, verifies the JWT via /api/v1/users/loggedInUser.
  • playwright/utils/ssoAuth.tsfetchSecurityConfig / applyProviderConfig / restoreSecurityConfig. Snapshot the server config, merge the provider override, restore the exact snapshot in afterAll.
  • playwright/utils/sso-providers/{index,okta}.tsProviderHelper interface + Okta widget driver. Public OAuth identifiers live in an OKTA_TENANT constant.
  • .github/workflows/playwright-sso-login-nightly.yml — single-file workflow, matrix-driven, one job per provider. Credentials resolved via vars[format(...)] / secrets[format(...)] against the <PROVIDER>_SSO_* convention.
  • playwright.config.ts — new sso-auth project, single worker, excluded from the default chromium run.

Adding a new provider

  1. Add sso-providers/<provider>.ts + case in sso-providers/index.ts.
  2. Add <PROVIDER>_SSO_USERNAME (var) + <PROVIDER>_SSO_PASSWORD (secret) to the test environment.
  3. Add the provider to the dispatch options and the matrix array in the workflow.

Test plan

  • Local run against the Okta test tenant — green (~15s)
  • Capture/restore verified by back-to-back runs
  • yarn lint:playwright + tsc --noEmit clean
  • CI dispatch against the branch — run 24180522486

Extends Playwright coverage end-to-end for SSO login flows. Today's SSO
coverage (Features/SSOConfiguration.spec.ts) only asserts the config
form UI. This adds a new suite that configures OpenMetadata to an
external identity provider, drives a real login through the provider's
hosted UI, and validates the resulting session against the OM API.

Phase 1 ships Okta only (integrator-9351624.okta.com). Additional
providers (Auth0, Azure, Cognito, SAML, Google) plug into the same
dispatcher by adding a ProviderHelper implementation.

## What's new

- playwright/e2e/Auth/SSOLogin.spec.ts — two-test suite tagged @sso
  1. Asserts the SSO sign-in button renders on /signin with the correct
     brand label and that the basic-auth form is not shown.
  2. Clicks the button, drives the provider's login widget, follows the
     OAuth callback, completes first-run self-signup when needed,
     lands on /my-data, then verifies the JWT by calling
     GET /api/v1/users/loggedInUser and asserting the returned email
     matches SSO_USERNAME.

- playwright/utils/ssoAuth.ts — provider-agnostic orchestration:
  applyProviderConfig (PUT /api/v1/system/security/config),
  restoreBasicAuth, buildAuthContextFromJwt, verifyLoggedInUserMatches.
  Composes existing getApiContext/getAuthContext/getToken helpers — no
  token extraction or HTTP plumbing is reimplemented.

- playwright/utils/sso-providers/{index,okta}.ts — ProviderHelper
  interface plus the Okta Identity Engine widget driver. Defaults the
  dev tenant values from the committed openmetadata.yaml snippet so the
  spec only needs SSO_USERNAME/SSO_PASSWORD to run locally.

- playwright/constant/ssoAuth.ts — env var key constants,
  PROVIDER_BUTTON_TEXT map, and the BASIC_AUTH_CONFIG payload used for
  cleanup.

- playwright.config.ts — new 'sso-auth' project matching
  playwright/e2e/Auth/**/*.spec.ts with its own serial workers, and
  '**/Auth/**' added to the chromium project's testIgnore so these
  tests never run in the default suite.

## How provider switching works

beforeAll logs in as admin via basic auth, captures the admin JWT via
getToken(page) BEFORE the swap, then PUTs the Okta config. The admin
JWT survives the provider swap because OM's internal JWKS stays in
publicKeyUrls and the admin user's isAdmin flag is persisted in the DB.
afterAll rebuilds an API context from that JWT and restores basic auth,
making the spec fully idempotent — the same OM instance can run the
suite repeatedly without any manual cleanup.

## Running locally

    export SSO_PROVIDER_TYPE=okta
    export SSO_USERNAME='<okta-test-user>'
    export SSO_PASSWORD='<okta-test-password>'
    npx playwright test playwright/e2e/Auth/SSOLogin.spec.ts \
      --project=sso-auth --workers=1

Verified end-to-end against integrator-9351624.okta.com — both tests
pass in ~12s on an already-provisioned user, ~14s on first-run
self-signup. Cleanup leaves the server in basic-auth mode.

## Notes for reviewers

- The existing .github/workflows/playwright-sso-tests.yml already wires
  up the CI matrix and secret names; this change intentionally does
  NOT enable the cron schedule. That lands in a follow-up once one
  provider is stable for a few nightly runs.
- OKTA_SSO_CLIENT_ID / OKTA_SSO_DOMAIN / OKTA_SSO_PRINCIPAL_DOMAIN env
  vars can override the baked-in dev tenant defaults if a different
  Okta tenant is used in CI.
@siddhant1 siddhant1 requested a review from a team as a code owner April 8, 2026 10:11
Copilot AI review requested due to automatic review settings April 8, 2026 10:11
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@siddhant1 siddhant1 added UI UI specific issues safe to test Add this label to run secure Github workflows on PRs labels Apr 8, 2026
Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/ssoAuth.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/constant/ssoAuth.ts Outdated
Adds .github/workflows/playwright-sso-login-nightly.yml, a standalone
workflow that runs the new SSOLogin spec nightly at 03:00 UTC instead
of piggy-backing on playwright-sso-tests.yml.

The existing playwright-sso-tests.yml is left untouched — it still
covers the SSO configuration form UI via SSOConfiguration.spec.ts and
its matrix/secrets wiring is unchanged. The new workflow complements
it with a real end-to-end login round-trip:

- Schedule: cron '0 3 * * *'
- Provider matrix: okta only for Phase 1 (extended as helpers ship)
- Invokes playwright/e2e/Auth/SSOLogin.spec.ts under the new
  sso-auth Playwright project with workers=1
- Wires provider credentials via secrets with the existing
  {PROVIDER}_SSO_USERNAME / {PROVIDER}_SSO_PASSWORD convention plus
  optional OKTA_SSO_CLIENT_ID / OKTA_SSO_DOMAIN /
  OKTA_SSO_PRINCIPAL_DOMAIN overrides
- Uses the shared setup-openmetadata-test-environment composite
  action, PostgreSQL, ingestion disabled — matching the existing SSO
  tests workflow
- Uploads the HTML report as an artifact on every run and cleans up
  the docker stack in a final always-run step
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Playwright E2E suite that exercises real SSO login via an external IdP (starting with Okta), including server-side auth config swaps and JWT validation against the OpenMetadata API.

Changes:

  • Added Auth/SSOLogin.spec.ts to perform end-to-end SSO login and validate /api/v1/users/loggedInUser.
  • Introduced provider-agnostic SSO orchestration helpers plus an Okta provider implementation.
  • Updated Playwright projects to isolate SSO auth tests from the default chromium suite.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
openmetadata-ui/src/main/resources/ui/playwright/utils/ssoAuth.ts Adds helpers to apply/restore auth config and verify logged-in user via API.
openmetadata-ui/src/main/resources/ui/playwright/utils/sso-providers/index.ts Defines provider interface + dispatcher for SSO providers.
openmetadata-ui/src/main/resources/ui/playwright/utils/sso-providers/okta.ts Implements Okta config payload builder and hosted login widget automation.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Auth/SSOLogin.spec.ts New serial E2E test suite covering SSO button + full login + JWT verification.
openmetadata-ui/src/main/resources/ui/playwright/constant/ssoAuth.ts Adds env var constants and a basic-auth cleanup payload.
openmetadata-ui/src/main/resources/ui/playwright.config.ts Adds sso-auth project and excludes Auth specs from default chromium runs.

Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/sso-providers/okta.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/sso-providers/okta.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/constant/ssoAuth.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/constant/ssoAuth.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright.config.ts
Comment thread .github/workflows/playwright-sso-login-nightly.yml Fixed
Comment thread .github/workflows/playwright-sso-login-nightly.yml Fixed
Comment thread .github/workflows/playwright-sso-login-nightly.yml Outdated
Siddhant added 2 commits April 8, 2026 15:48
- verifyLoggedInUserMatches now asserts directly on the lowercased
  email field instead of building a candidate array and feeding it a
  long stringified failure message. The assertion failure already
  shows expected vs received, so the wrapper string was just noise.

- Drop buildAuthContextFromJwt — it was a one-line wrapper around
  getAuthContext. The spec calls getAuthContext directly now.
- Extract OM_BASE_URL from PLAYWRIGHT_TEST_BASE_URL (with the same
  http://localhost:8585 default as playwright.config.ts) and export
  it from constant/ssoAuth.ts. okta.ts and BASIC_AUTH_CONFIG both
  consume it, so callbackUrl, the OM JWKS entry in publicKeyUrls, and
  the basic-auth restore payload all match the test target — including
  CI runs against non-default hosts.

- Drop PROVIDER_BUTTON_TEXT. It was exported but never imported; the
  ProviderHelper.expectedButtonText field is the only source of truth
  for the SSO sign-in button label and the spec already reads from it.

- Restore the OM convention adminPrincipals: ['admin'] in the Okta
  config (matches conf/openmetadata.yaml's AUTHORIZER_ADMIN_PRINCIPALS
  default). The previous code was granting admin to whichever IdP user
  ran the suite — verifyLoggedInUserMatches only needs an authenticated
  session, not admin, so the elevation was unnecessary. This also drops
  the now-unused requireEnv on SSO_USERNAME inside okta.ts; the spec
  itself still gates on the env var via test.skip.

- Set workers: 1 on the sso-auth Playwright project. fullyParallel:
  false alone wasn't enough — the global workers: 3 on CI could still
  fan out across multiple Auth/**/*.spec.ts files in the future. The
  explicit limit enforces full isolation as more provider specs land.
Copilot AI review requested due to automatic review settings April 8, 2026 10:22
Replaces the dynamic secret lookup

    secrets[format('{0}_SSO_USERNAME', upper(matrix.provider))]

with a static reference

    secrets.OKTA_SSO_USERNAME

CodeQL flagged the dynamic indexing because GitHub Actions can only
mask & scope secrets that are referenced statically. With a computed
key, the runner has no way to know which single secret is needed and
conservatively materializes EVERY org and repo secret into the step's
environment — even though the test only reads OKTA_SSO_*. Static
references let GitHub expose only the two credentials this step
actually uses.

Phase 1's matrix is okta-only so the change is two lines. The added
inline comment documents the convention for future providers: add a
sibling step gated by `if: matrix.provider == '<provider>'` with that
provider's static secret references — do not bring back the
secrets[format(...)] pattern.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/ssoAuth.ts
Comment thread openmetadata-ui/src/main/resources/ui/playwright/constant/ssoAuth.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/sso-providers/okta.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
63.83% (59800/93684) 43.66% (31344/71789) 46.71% (9418/20161)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread docker/local-sso/keycloak-saml/docker-compose.yml
Siddhant and others added 2 commits April 15, 2026 11:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment on lines +103 to +107
const signInButton = page.locator('.signin-button');

await expect(signInButton).toBeVisible();
await expect(signInButton).toContainText(helper.expectedButtonText);
await expect(page.getByTestId('email')).toHaveCount(0);
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selector for the SSO button is very broad (.signin-button) and could match non-button elements or multiple nodes. Other Auth specs in this repo use button.signin-button for this control. Tightening the locator here would reduce flakiness and align with existing patterns.

Copilot uses AI. Check for mistakes.
Comment thread openmetadata-ui/src/main/resources/ui/playwright/e2e/Auth/SSOLogin.spec.ts Outdated
Siddhant and others added 2 commits April 15, 2026 12:54
Address Copilot review comments on PR #27164:
- Use button.signin-button to match the pattern in SSOAuthentication.spec.ts.
- Await /api/v1/users/logout POST alongside the /signin navigation in
  the logout test to remove the race against the server response.

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

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comment thread openmetadata-ui/src/main/resources/ui/playwright/e2e/Auth/SSOLogin.spec.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright.config.ts
Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/ssoAuth.ts
Siddhant and others added 3 commits April 15, 2026 13:44
…ogin.spec.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread openmetadata-ui/src/main/resources/ui/playwright.config.ts
Comment thread .github/workflows/playwright-sso-login-nightly.yml Outdated
siddhant1 and others added 3 commits April 15, 2026 16:43
…e-saml

Route Keycloak credentials through the same `vars[format(...)]` /
`secrets[format(...)]` indirection as Okta via an `env_prefix` matrix
column, removing the hardcoded fixture literals from the workflow.
Password lookup falls back `vars || secrets` so fixture passwords can
live as vars while real provider secrets stay in secrets.

Also drop the keycloak-google-saml variant — same IdP and realm shape
as the Azure variant, so it adds CI cost without meaningful coverage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a per-provider Slack notification step mirroring the pattern used
by the postgresql/mysql nightly workflows — reuses the existing
`slack-cli.config.json` and `playwright-slack-report` CLI against the
`results.json` that the global JSON reporter already emits.

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

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread .github/workflows/playwright-sso-login-nightly.yml
Comment thread .github/workflows/playwright-sso-login-nightly.yml
siddhant1 and others added 2 commits April 15, 2026 16:55
OktaAuthenticator.logout clears tokens locally with no backend call, and
GenericAuthenticator (SAML) hits `GET /auth/logout` — neither triggers
the `POST /api/v1/users/logout` the test was waiting on. The listener
never matched, so `Promise.all` hung past the 180s test timeout even
though the page had already navigated to /signin.

Rely on `waitForURL('**/signin')` + the signin button assertion, which
are the actual cross-provider success signals.

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

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 16, 2026

Code Review 👍 Approved with suggestions 6 resolved / 7 findings

Adds nightly Okta SSO login test spec with comprehensive fixes for JWT context building, GitHub Actions expressions, test cleanup, and environment variable handling. Pin the jlumbroso/free-disk-space action to a specific release tag instead of @main to avoid unexpected behavior changes.

💡 Security: Unpinned third-party action uses mutable @main ref

📄 .github/workflows/playwright-sso-login-nightly.yml:55

The jlumbroso/free-disk-space action is referenced with @main (line 55), which is a mutable branch ref. A compromised or force-pushed main branch in that repo could inject arbitrary code into this workflow. All other actions in the file are pinned to version tags (@v4).

This is lower risk because the workflow only runs on a schedule or manual dispatch (no PR trigger), and the permissions: contents: read scope limits blast radius. Still worth pinning for supply-chain hygiene.

Suggested fix
Pin to a release tag or commit SHA:

- uses: jlumbroso/free-disk-space@v1.3.1
+ # or pin to SHA:
+ uses: jlumbroso/free-disk-space@<commit-sha>
✅ 6 resolved
Quality: buildAuthContextFromJwt is a trivial one-line wrapper

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/ssoAuth.ts:46-50
buildAuthContextFromJwt in ssoAuth.ts (lines 46-50) is a single-line passthrough to getAuthContext(jwt). It adds an extra layer of indirection without any additional logic, validation, or error handling. Consider importing and calling getAuthContext directly in the spec to reduce abstraction overhead.

Bug: upper() is not a valid GitHub Actions expression function

📄 .github/workflows/playwright-sso-login-nightly.yml:116-117
Lines 116-117 use upper(matrix.provider) inside secrets[format(…)] to build secret names like OKTA_SSO_USERNAME. However, upper() does not exist in the GitHub Actions expression language — the supported string functions are contains, startsWith, endsWith, format, join, toJSON, fromJSON, and hashFiles.

This will cause an expression evaluation error at runtime, meaning SSO_USERNAME and SSO_PASSWORD will never be populated, and every nightly run will fail.

Edge Case: afterAll cleanup skipped if getToken returns empty string

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Auth/SSOLogin.spec.ts:44-58 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Auth/SSOLogin.spec.ts:62-74
In beforeAll, getToken(page) is called before applyProviderConfig() (correct order). However, getToken() returns an empty string '' when the token isn't found (not undefined). In afterAll (line 63), the guard if (!adminJwt) correctly catches empty string since it's falsy, but this means if token extraction silently fails for any reason, applyProviderConfig will have already switched the server to SSO mode, and afterAll will skip the restore — leaving the server permanently stuck in SSO mode.

This is especially problematic in CI where the same OM instance may be reused across test runs. Consider capturing the JWT before applying the config and failing the setup explicitly if the token is missing, rather than proceeding with the config swap.

Edge Case: BASIC_AUTH_CONFIG hardcodes adminPrincipals: ['admin']

📄 openmetadata-ui/src/main/resources/ui/playwright/constant/ssoAuth.ts:33-47
The BASIC_AUTH_CONFIG constant in ssoAuth.ts (line 45) hardcodes adminPrincipals: ['admin']. If the target OM instance was originally configured with additional admin principals, restoring via this config will silently drop them. Consider reading the current config before the swap (in beforeAll) and storing the original values for a faithful restore, or at minimum documenting this assumption.

Edge Case: jq outputs literal "null" when env var key is missing

📄 .github/actions/sso-login-run/action.yml:102-103
When ${PREFIX}_SSO_USERNAME is not defined in the test environment's vars, jq -r '.[$k]' returns the literal string "null" (not empty). This sets SSO_USERNAME=null, causing the Playwright spec to silently use "null" as the expected email — the test would fail with a confusing assertion mismatch instead of a clear "variable not configured" error.

The same issue applies to the bulk export on lines 98-101: any expected var that's absent from vars would be exported as KEY=null.

...and 1 more resolved from earlier reviews

🤖 Prompt for agents
Code Review: Adds nightly Okta SSO login test spec with comprehensive fixes for JWT context building, GitHub Actions expressions, test cleanup, and environment variable handling. Pin the `jlumbroso/free-disk-space` action to a specific release tag instead of `@main` to avoid unexpected behavior changes.

1. 💡 Security: Unpinned third-party action uses mutable `@main` ref
   Files: .github/workflows/playwright-sso-login-nightly.yml:55

   The `jlumbroso/free-disk-space` action is referenced with `@main` (line 55), which is a mutable branch ref. A compromised or force-pushed `main` branch in that repo could inject arbitrary code into this workflow. All other actions in the file are pinned to version tags (`@v4`).
   
   This is lower risk because the workflow only runs on a schedule or manual dispatch (no PR trigger), and the `permissions: contents: read` scope limits blast radius. Still worth pinning for supply-chain hygiene.

   Suggested fix:
   Pin to a release tag or commit SHA:
   
   - uses: jlumbroso/free-disk-space@v1.3.1
   + # or pin to SHA:
   + uses: jlumbroso/free-disk-space@<commit-sha>

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

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

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants