Skip to content

feat(FR-2627): migrate EduAppLauncher sToken path to STokenLoginBoundary#6864

Merged
graphite-app[bot] merged 1 commit into
mainfrom
04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary
Apr 23, 2026
Merged

feat(FR-2627): migrate EduAppLauncher sToken path to STokenLoginBoundary#6864
graphite-app[bot] merged 1 commit into
mainfrom
04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary

Conversation

@nowgnuesLee
Copy link
Copy Markdown
Contributor

@nowgnuesLee nowgnuesLee commented Apr 22, 2026

Resolves FR-2641, FR-2642 (under Story FR-2627, Epic FR-2616)

Summary

Story 3 of Epic FR-2616: route /edu-applauncher and /applauncher now authenticate through STokenLoginBoundary before EduAppLauncher mounts. _token_login and the manual backend-ai-connected dispatch are removed from the component.

Scope

  • Route wrapping (react/src/routes.tsx): both edu-app routes read sToken via useSToken() and URL params via useQueryStates(eduAppExtraParamSpec), then wrap EduAppLauncherPage with STokenLoginBoundary. The URL is intentionally not stripped on success (the launcher still passes sToken prop through for eduApp.get_user_credential and other params drive the launch sequence).
  • EduAppLauncher cleanup (react/src/components/EduAppLauncher.tsx):
    • Removed _token_login() method and the URL parsing it owned.
    • Removed the manual document.dispatchEvent(new CustomEvent('backend-ai-connected')) call (the boundary now dispatches this exactly once).
    • Removed the auth stage from EduAppLaunchStage — authentication is no longer represented in the launcher's state machine or its stepper UI. Since the boundary runs connectViaGQL before EduAppLauncher mounts, the component always starts with a fully authenticated client.
    • Deleted _prepareProjectInformation()connectViaGQL already populates groups / groupIds / current_group / current_group_id with a superset of the fields this helper fetched.
    • Proxy URL attach (_attachProxyURL) remains but is no longer labeled "auth"; failures now surface under the session step.
    • The stepper UI drops from 3 steps to 2 (Preparing SessionLaunching App).
  • extraParams allowlist (react/src/routes.tsx:eduAppExtraParamSpec): added api_version, date, endpoint. These are part of the LMS signing envelope forwarded with sToken in the old URL-scan based _token_login; the nuqs migration replaced the scan with an explicit allowlist and had dropped them, causing manager-side auth hooks that validate the signature against these fields to reject token_login as tampered.

Test plan

  • bash scripts/verify.shALL PASS
  • Manual: launch from LMS URL /edu-applauncher?sToken=<signed>&app=jupyterlab&api_version=...&date=...&endpoint=...&session_id=... and confirm:
    • POST /server/token-login body contains all extra keys (check DevTools Network tab)
    • Stepper shows 2 steps ("Preparing Session", "Launching App") — no "Authentication" step
    • Successful launch opens the app in a new tab
  • Regression scenarios covered by PR test(FR-2639): add E2E regression for sToken login boundary routes #6865 E2E: with / without session_id, invalid sToken surfaces stepper-integrated error

Checklist:

  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Stack

Story 3 of Epic FR-2616. See dev plan for the full story breakdown.

Copy link
Copy Markdown
Contributor Author

nowgnuesLee commented Apr 22, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
8.92% (+0.02% 🔼)
1831/20519
🔴 Branches
8.07% (+0% 🔼)
1165/14429
🔴 Functions
5.28% (+0% 🔼)
294/5567
🔴 Lines
8.65% (+0.02% 🔼)
1723/19908

Test suite run success

865 tests passing in 40 suites.

Report generated by 🧪jest coverage report action from 5716748

@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch 2 times, most recently from 7b879f8 to d798003 Compare April 22, 2026 03:34
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2626_migrate_loginview_stoken_path_to_stokenloginboundary branch from 7c7561a to fc0f377 Compare April 22, 2026 03:43
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch 2 times, most recently from b229d79 to 8e05574 Compare April 22, 2026 03:47
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2626_migrate_loginview_stoken_path_to_stokenloginboundary branch from fc0f377 to f062859 Compare April 22, 2026 03:53
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch from 8e05574 to 541721c Compare April 22, 2026 03:53
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch from 541721c to 41da011 Compare April 22, 2026 09:18
@nowgnuesLee nowgnuesLee marked this pull request as ready for review April 23, 2026 06:09
Copilot AI review requested due to automatic review settings April 23, 2026 06:09
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

Routes for /edu-applauncher and /applauncher are updated so sToken-based authentication happens in STokenLoginBoundary before EduAppLauncher mounts, simplifying the launcher’s internal flow and moving URL-param parsing to a typed nuqs allowlist.

Changes:

  • Wrap /edu-applauncher and /applauncher routes with STokenLoginBoundary, sourcing sToken via useSToken() and allowlisted query params via useQueryStates().
  • Thread sToken and extraParams down through EduAppLauncherPage into EduAppLauncher (no more in-component URL parsing).
  • Simplify EduAppLauncher state machine/stepper by removing the authentication stage and related legacy logic.

Reviewed changes

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

File Description
react/src/routes.tsx Adds eduAppExtraParamSpec allowlist and wraps edu-app routes in STokenLoginBoundary, passing captured sToken/extraParams through.
react/src/pages/EduAppLauncherPage.tsx Updates page components to accept and forward sToken/extraParams props into the lazy launcher.
react/src/components/EduAppLauncher.tsx Removes token-login + URL parsing from the launcher, shifts to prop-based params, and drops the auth stage from the stepper/state machine.

Comment thread react/src/pages/EduAppLauncherPage.tsx Outdated
Comment thread react/src/components/EduAppLauncher.tsx Outdated
Comment thread react/src/routes.tsx Outdated
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2626_migrate_loginview_stoken_path_to_stokenloginboundary branch from f062859 to c9bf0d6 Compare April 23, 2026 07:57
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch 2 times, most recently from 7fba64c to 907c539 Compare April 23, 2026 08:02
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2626_migrate_loginview_stoken_path_to_stokenloginboundary branch 2 times, most recently from 6c16c60 to c683412 Compare April 23, 2026 08:35
@nowgnuesLee nowgnuesLee force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch from 907c539 to 87f643c Compare April 23, 2026 08:35
Copy link
Copy Markdown
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Apr 23, 2026

Merge activity

…ary (#6864)

Resolves FR-2641, FR-2642 (under Story [FR-2627](https://lablup.atlassian.net/browse/FR-2627), Epic [FR-2616](https://lablup.atlassian.net/browse/FR-2616))

## Summary

Story 3 of Epic FR-2616: route `/edu-applauncher` and `/applauncher` now authenticate through `STokenLoginBoundary` before `EduAppLauncher` mounts. `_token_login` and the manual `backend-ai-connected` dispatch are removed from the component.

### Scope

- **Route wrapping** (`react/src/routes.tsx`): both edu-app routes read `sToken` via `useSToken()` and URL params via `useQueryStates(eduAppExtraParamSpec)`, then wrap `EduAppLauncherPage` with `STokenLoginBoundary`. The URL is intentionally not stripped on success (the launcher still passes `sToken` prop through for `eduApp.get_user_credential` and other params drive the launch sequence).
- **`EduAppLauncher` cleanup** (`react/src/components/EduAppLauncher.tsx`):
  - Removed `_token_login()` method and the URL parsing it owned.
  - Removed the manual `document.dispatchEvent(new CustomEvent('backend-ai-connected'))` call (the boundary now dispatches this exactly once).
  - **Removed the `auth` stage from `EduAppLaunchStage`** — authentication is no longer represented in the launcher's state machine or its stepper UI. Since the boundary runs `connectViaGQL` before `EduAppLauncher` mounts, the component always starts with a fully authenticated client.
  - **Deleted `_prepareProjectInformation()`** — `connectViaGQL` already populates `groups` / `groupIds` / `current_group` / `current_group_id` with a superset of the fields this helper fetched.
  - Proxy URL attach (`_attachProxyURL`) remains but is no longer labeled "auth"; failures now surface under the session step.
  - The stepper UI drops from 3 steps to 2 (`Preparing Session` → `Launching App`).
- **`extraParams` allowlist** (`react/src/routes.tsx:eduAppExtraParamSpec`): added `api_version`, `date`, `endpoint`. These are part of the LMS signing envelope forwarded with `sToken` in the old URL-scan based `_token_login`; the nuqs migration replaced the scan with an explicit allowlist and had dropped them, causing manager-side auth hooks that validate the signature against these fields to reject `token_login` as tampered.

## Test plan

- [x] `bash scripts/verify.sh` → `ALL PASS`
- [ ] Manual: launch from LMS URL `/edu-applauncher?sToken=<signed>&app=jupyterlab&api_version=...&date=...&endpoint=...&session_id=...` and confirm:
  - `POST /server/token-login` body contains all extra keys (check DevTools Network tab)
  - Stepper shows 2 steps ("Preparing Session", "Launching App") — no "Authentication" step
  - Successful launch opens the app in a new tab
- [ ] Regression scenarios covered by PR #6865 E2E: with / without `session_id`, invalid sToken surfaces stepper-integrated error

**Checklist:**

- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [x] Minimum requirements to check during review
- [x] Test case(s) to demonstrate the difference of before/after

## Stack

Story 3 of Epic FR-2616. See [dev plan](../blob/main/.specs/draft-stoken-login-boundary/dev-plan.md) for the full story breakdown.

[FR-2627]: https://lablup.atlassian.net/browse/FR-2627
[FR-2616]: https://lablup.atlassian.net/browse/FR-2616
@graphite-app graphite-app Bot force-pushed the 04-22-feat_fr-2626_migrate_loginview_stoken_path_to_stokenloginboundary branch from c683412 to 79e6dc4 Compare April 23, 2026 10:36
@graphite-app graphite-app Bot force-pushed the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch from 87f643c to 5716748 Compare April 23, 2026 10:37
Base automatically changed from 04-22-feat_fr-2626_migrate_loginview_stoken_path_to_stokenloginboundary to main April 23, 2026 10:46
@graphite-app graphite-app Bot merged commit 5716748 into main Apr 23, 2026
8 checks passed
@graphite-app graphite-app Bot deleted the 04-22-feat_fr-2627_migrate_eduapplauncher_stoken_path_to_stokenloginboundary branch April 23, 2026 10:47
graphite-app Bot pushed a commit that referenced this pull request Apr 24, 2026
…6865)

Resolves FR-2639 and FR-2643 (under Stories [FR-2626](https://lablup.atlassian.net/browse/FR-2626) / [FR-2627](https://lablup.atlassian.net/browse/FR-2627), Epic [FR-2616](https://lablup.atlassian.net/browse/FR-2616))

resolves #NNN (FR-MMM)
<!-- replace NNN, MMM with the GitHub issue number and the corresponding Jira issue number. -->

<!--
Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes,
and how it affects the users and other developers.
-->

**Checklist:** (if applicable)

- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after

## Stack

Story 2/3 E2E regression for Epic FR-2616. Sits on top of the Story 2 (#6861) and Story 3 (#6864) implementation PRs.

[FR-2626]: https://lablup.atlassian.net/browse/FR-2626?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[FR-2627]: https://lablup.atlassian.net/browse/FR-2627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[FR-2616]: https://lablup.atlassian.net/browse/FR-2616?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants