Skip to content

UI: don’t continue with session with an expired userid - #13969

Open
DaanHoogland wants to merge 3 commits into
apache:4.20from
shapeblue:rejectsExpiredSession
Open

UI: don’t continue with session with an expired userid#13969
DaanHoogland wants to merge 3 commits into
apache:4.20from
shapeblue:rejectsExpiredSession

Conversation

@DaanHoogland

Copy link
Copy Markdown
Contributor

Description

This PR fixes a representational issue where a member of the root admin account would think they had now become user “admin” after their cookies would expire. Any action they would take would still be accounted to their own userid as can be shown by events.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@prashanthr2 please have a look.

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan ui

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 16.37%. Comparing base (6136ac9) to head (2c819ca).
⚠️ Report is 19 commits behind head on 4.20.

Additional details and impacted files
@@             Coverage Diff              @@
##               4.20   #13969      +/-   ##
============================================
+ Coverage     16.32%   16.37%   +0.05%     
- Complexity    13556    13602      +46     
============================================
  Files          5669     5669              
  Lines        501399   501446      +47     
  Branches      60902    60913      +11     
============================================
+ Hits          81847    82113     +266     
+ Misses       410390   410129     -261     
- Partials       9162     9204      +42     
Flag Coverage Δ
uitests 4.14% <ø> (-0.01%) ⬇️
unittests 17.23% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland DaanHoogland added this to the 4.20.4 milestone Aug 25, 2026
@vishesh92

Copy link
Copy Markdown
Member

@blueorangutan ui

@vishesh92

Copy link
Copy Markdown
Member

@blueorangutan ui

@vishesh92
vishesh92 force-pushed the rejectsExpiredSession branch from 8c7d95d to 90d11d8 Compare September 4, 2026 07:22
@apache apache deleted a comment from blueorangutan Sep 4, 2026
@apache apache deleted a comment from blueorangutan Sep 4, 2026
@apache apache deleted a comment from blueorangutan Sep 4, 2026
@apache apache deleted a comment from blueorangutan Sep 4, 2026
@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/13969 (QA-JID-988)

@sudo87 sudo87 self-assigned this Sep 9, 2026
@sudo87 sudo87 moved this from Ready to In progress in CloudStack Testing Sep 9, 2026
@sudo87
sudo87 requested a lite review from Copilot September 10, 2026 04:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

This PR aims to prevent the UI from continuing with a cached auth/session state when the userid cookie has expired, which previously could misrepresent the active identity in the UI.

Changes:

  • Refactors duplicated login response handling into shared helper functions.
  • Adds a guard when reusing cached auth state to reject if userid cookie is missing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +49 to +57
function setLoginCookies (result) {
Cookies.set('account', result.account)
Cookies.set('domainid', result.domainid)
Cookies.set('role', result.type)
Cookies.set('timezone', result.timezone)
Cookies.set('timezoneoffset', result.timezoneoffset)
Cookies.set('userfullname', result.firstname + ' ' + result.lastname)
Cookies.set('userid', result.userid)
Cookies.set('username', result.username)
Comment on lines 275 to +279
if (hasAuth) {
if (!Cookies.get('userid')) {
reject(new Error('Identity session expired'))
return
}
commit('SET_LATEST_VERSION', latestVersion)
if (hasAuth) {
if (!Cookies.get('userid')) {
reject(new Error('Identity session expired'))

@sudo87 sudo87 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, tested basic login-logout flow

@sudo87 sudo87 moved this from In progress to In review in CloudStack Testing Sep 11, 2026
@shwstppr shwstppr self-assigned this Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

6 participants