refactor(lightspeed): add middleware for resolving user identity by Jdubrick · Pull Request #3227 · redhat-developer/rhdh-plugins

Jdubrick · 2026-05-21T20:09:57Z

Hey, I just made a Pull Request!

Moves the resolution of the user identity to a middleware step, this reduces a lot of the duplicated resolving code and makes it easier to extend for tasks in 2.1.0. This adds the user identity to the individual request by extending the Express Request type, so it is easy to reference.

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or Updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)

rhdh-qodo-merge · 2026-05-21T20:10:10Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

1. ~~Identity errors misclassified~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

createIdentityMiddleware catches all exceptions from httpAuth/userInfo and always returns 401 with a
generic log line, even when the failure is an internal error (e.g., userInfo
outage/misconfiguration). This masks non-auth failures as “Unauthorized” and loses stack traces,
making production diagnosis and alerting significantly harder.

Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.ts[R43-52]

Relevance

⭐⭐⭐ High
Team recently fixed incorrect status mapping to preserve upstream codes; masking all errors as 401
likely flagged similarly.
PR-#3070
PR-#1786

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The middleware’s catch block always returns 401 and does not log the thrown error, whereas other
parts of the backend preserve error details and distinguish forbidden vs internal failures; this
inconsistency will hide real failures behind an auth error and remove stack traces.
workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.ts[38-52]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[239-245]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/utils.ts[49-73]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`createIdentityMiddleware` currently catches *all* errors during identity resolution and responds with `401 Unauthorized`, while also logging without including the thrown error object. This makes internal failures indistinguishable from auth failures and drops stack traces.

## Issue Context
The middleware runs early for most routes (and for notebooks under `/v1`) so its error handling becomes the system-of-record for identity failures.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.ts[38-52]

## What to change
1. Log the actual error object (so stack traces are preserved), and include basic request context (method + path).
2. Return status codes based on error type:
  - For expected auth failures (e.g., `NotAllowedError` / auth-specific errors thrown by Backstage auth), respond `401` (or `403` if that matches your auth model).
  - For unexpected/internal failures, respond `500` (or `next(error)` to let the global Backstage error middleware format and log it consistently).
3. If returning JSON directly, consider aligning the error shape with the rest of this backend’s handlers (optional), but do not lose the underlying error in logs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

rhdh-gh-app · 2026-05-21T20:10:23Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend	workspaces/lightspeed/plugins/lightspeed-backend	patch	`v2.9.0`

rhdh-qodo-merge · 2026-05-21T20:16:59Z

Review Summary by Qodo

Refactor user identity resolution into reusable middleware

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Centralizes user identity resolution via middleware to eliminate code duplication
• Extends Express Request type with credentials and userEntityRef fields
• Implements identity middleware in both lightspeed and notebooks routers
• Adds comprehensive tests for middleware, permission denial, and identity deduplication

Diagram

flowchart LR
  A["Express Request"] -->|"createIdentityMiddleware"| B["Resolve Credentials & UserEntityRef"]
  B -->|"Attach to req"| C["Request with Identity"]
  C -->|"getIdentity"| D["Extract & Validate Identity"]
  D -->|"Return typed object"| E["Route Handlers"]
  E -->|"Use credentials & userEntityRef"| F["Authorization & Operations"]

File Changes

1. workspaces/lightspeed/plugins/lightspeed-backend/src/service/express.d.ts ✨ Enhancement +25/-0

Extend Express Request type with identity fields
• Extends Express Request interface with credentials and userEntityRef optional fields
• Enables type-safe access to identity information across route handlers
• Populated by the identity middleware during request processing
workspaces/lightspeed/plugins/lightspeed-backend/src/service/express.d.ts

2. workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.ts ✨ Enhancement +70/-0

Add middleware for resolving user identity

• Creates createIdentityMiddleware that resolves user identity once per request
• Implements guard check to skip resolution if identity already populated
• Provides getIdentity utility function to extract and validate identity from request
• Returns 401 Unauthorized on identity resolution failure

workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.ts

3. workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.test.ts 🧪 Tests +55/-0

Add tests for identity middleware
• Tests successful identity extraction when both credentials and userEntityRef are present
• Validates error handling when credentials or userEntityRef are missing
• Ensures proper error messages for middleware validation failures
workspaces/lightspeed/plugins/lightspeed-backend/src/service/middleware/getIdentity.test.ts

View more (4)

4. workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts ✨ Enhancement +13/-11

Integrate identity middleware in notebooks router

• Integrates createIdentityMiddleware into notebooks router at /v1 path
• Replaces inline getUserId function calls with getIdentity utility
• Removes duplicate credential resolution logic from permission and ownership checks
• Simplifies route handlers by using pre-resolved identity from middleware

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts

5. workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouter.test.ts 🧪 Tests +108/-1

Add permission denial and deduplication tests
• Adds test suite for 403 Permission Denied responses across all notebook endpoints
• Implements identity deduplication test to verify middleware runs only once
• Refactors test setup to store httpAuth reference for spy verification
• Tests permission denial for POST/GET sessions and document operations
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouter.test.ts

6. workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts ✨ Enhancement +43/-41

Integrate identity middleware in main router

• Integrates createIdentityMiddleware into main router with path-based guard for notebooks
• Replaces all inline httpAuth.credentials() and userInfo.getUserInfo() calls with
 getIdentity()
• Eliminates duplicate identity resolution across MCP server, feedback, query, and conversation
 endpoints
• Simplifies code by removing intermediate variable assignments for user identity

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts

7. workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts 🧪 Tests +80/-0

Add notebook conversation IDs endpoint tests

• Adds tests for /notebook-conversation-ids endpoint filtering by authenticated user
• Tests successful retrieval of conversation IDs for user's notebook sessions
• Tests empty array response when user has no notebook sessions
• Validates filtering excludes other users' sessions and sessions without conversation IDs

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts

codecov · 2026-05-21T20:17:55Z

Codecov Report

❌ Patch coverage is 85.96491% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.62%. Comparing base (44b418c) to head (7fa3d14).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3227      +/-   ##
==========================================
+ Coverage   53.61%   53.62%   +0.01%     
==========================================
  Files        2408     2409       +1     
  Lines       86622    86633      +11     
  Branches    24010    24012       +2     
==========================================
+ Hits        46439    46456      +17     
+ Misses      38683    38677       -6     
  Partials     1500     1500

Flag	Coverage Δ		*Carryforward flag
adoption-insights	`83.58% <ø> (ø)`		Carriedforward from 44b418c
ai-integrations	`70.03% <ø> (ø)`		Carriedforward from 44b418c
app-defaults	`69.60% <ø> (ø)`		Carriedforward from 44b418c
augment	`46.39% <ø> (ø)`		Carriedforward from 44b418c
bulk-import	`72.86% <ø> (ø)`		Carriedforward from 44b418c
cost-management	`16.49% <ø> (ø)`		Carriedforward from 44b418c
dcm	`45.40% <ø> (ø)`		Carriedforward from 44b418c
extensions	`61.79% <ø> (ø)`		Carriedforward from 44b418c
global-floating-action-button	`74.30% <ø> (ø)`		Carriedforward from 44b418c
global-header	`61.63% <ø> (ø)`		Carriedforward from 44b418c
homepage	`51.52% <ø> (ø)`		Carriedforward from 44b418c
konflux	`91.01% <ø> (ø)`		Carriedforward from 44b418c
lightspeed	`68.50% <85.96%> (+0.16%)`	⬆️
mcp-integrations	`85.46% <ø> (ø)`		Carriedforward from 44b418c
orchestrator	`37.34% <ø> (ø)`		Carriedforward from 44b418c
quickstart	`62.88% <ø> (ø)`		Carriedforward from 44b418c
sandbox	`79.56% <ø> (ø)`		Carriedforward from 44b418c
scorecard	`83.84% <ø> (ø)`		Carriedforward from 44b418c
theme	`64.54% <ø> (ø)`		Carriedforward from 44b418c
translations	`8.49% <ø> (ø)`		Carriedforward from 44b418c
x2a	`78.79% <ø> (ø)`		Carriedforward from 44b418c

*This pull request uses carry forward flags. Click here to find out more.

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44b418c...7fa3d14. Read the comment docs.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

HusneShabbir

cc: @its-mitesh-kumar / @karthikjeeyar

maysunfaisal

lgtm in general, you may wait on the others

yangcao77 · 2026-06-01T22:22:02Z

+      req.userEntityRef = user.userEntityRef;
+      return next();
+    } catch (error) {
+      logger.error('Identity resolution failed for request');


the actual error object is not logged here. do we want to log the error?

logger.error('Identity resolution failed for request', { error });

I've added the logging of the error in 7fa3d14

yangcao77 · 2026-06-01T22:28:44Z

+      if (error instanceof NotAllowedError) {
+        return res.status(403).json({ error: 'Forbidden' });
+      }
+      return res.status(401).json({ error: 'Unauthorized' });


getting credentials/identity only have 401/403 errors?
what if internal errors?

Good catch, I updated the handling to check explicitly for 401/403, then log as a server error for the rest that are unexpected in 7fa3d14

yangcao77

generally looks good to me. just some minor comments on error handling

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

…ebooks Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

sonarqubecloud · 2026-06-02T14:57:17Z

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

yangcao77

changes lgtm

Jdubrick requested review from a team, karthikjeeyar, rohitkrai03 and yangcao77 as code owners May 21, 2026 20:09

github-actions Bot added the workspace/lightspeed label May 21, 2026

rhdh-qodo-merge Bot added enhancement New feature or request Tests labels May 21, 2026

Jdubrick force-pushed the refactor-middleware-lightspeed branch from 231d37a to e5744e4 Compare May 21, 2026 20:22

HusneShabbir reviewed May 24, 2026

View reviewed changes

Jdubrick force-pushed the refactor-middleware-lightspeed branch 2 times, most recently from ede49bf to c2c8bed Compare June 1, 2026 14:17

maysunfaisal approved these changes Jun 1, 2026

View reviewed changes

yangcao77 reviewed Jun 1, 2026

View reviewed changes

Jdubrick added 7 commits June 2, 2026 10:55

add extra tests for notebooks auth

90acc3d

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

extend express Request type for creds and userEntityRef

a227780

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

add middleware for fetching user identity

e9fc953

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

implement middleware for fetching user identity in lightspeed and not…

ddab139

…ebooks Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

catch 403 error for identity resolution

b0ccc76

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

add changeset

55bd82b

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

improve error handling for identity resolution

7fa3d14

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

Jdubrick force-pushed the refactor-middleware-lightspeed branch from c2c8bed to 7fa3d14 Compare June 2, 2026 14:55

Jdubrick requested a review from yangcao77 June 2, 2026 14:56

yangcao77 approved these changes Jun 2, 2026

View reviewed changes

Jdubrick merged commit d49a228 into redhat-developer:main Jun 2, 2026
85 of 86 checks passed

Conversation

Jdubrick commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hey, I just made a Pull Request!

✔️ Checklist

Uh oh!

rhdh-qodo-merge Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

rhdh-gh-app Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changed Packages

Uh oh!

rhdh-qodo-merge Bot commented May 21, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

codecov Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

HusneShabbir left a comment

Choose a reason for hiding this comment

Uh oh!

maysunfaisal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangcao77 Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Jdubrick Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

yangcao77 Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Jdubrick Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

yangcao77 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jun 2, 2026

Quality Gate passed

Uh oh!

yangcao77 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jdubrick commented May 21, 2026 •

edited

Loading

rhdh-qodo-merge Bot commented May 21, 2026 •

edited

Loading

rhdh-gh-app Bot commented May 21, 2026 •

edited

Loading

codecov Bot commented May 21, 2026 •

edited

Loading

maysunfaisal left a comment •

edited

Loading