Fix saml bug unable to login #10868

vits-hugs · 2025-05-14T18:12:51Z

Description

After setting a user to login using SAML, and then disabling the login by SAML for the user, he cannot access the account anymore, neither by saml nor by username and password. So this PRs adds and verification to let a user with SAML authentication disabled login with username and password.

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)

Bug Severity

How Has This Been Tested?

To test, i followed this process:

With the config enable.login.saml.unathourized set to true:
- Created a new user, and configured it to authenticate using SAML.
- Login with SAML to test the new User.
- Logout, and login with an Admin account.
- Remove login with SAML of the user.
- Login with the user using username and password.
*With the config enable.login.saml.unathourized set to false:
- Created a new user, and configured it to authenticate using SAML.
- Login with SAML to test the new User.
- Logout, and login with an Admin account.
- Remove login with SAML of the user.
- Verify that i couldn't login with the user, with SAML and with username and password.

vits-hugs · 2025-05-14T19:16:20Z

@blueorangutan package

blueorangutan · 2025-05-14T19:18:03Z

@vits-hugs a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-05-14T20:07:31Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13387

DaanHoogland

clgtm, maybe this should be a setting allowing the operator to enable the ‘bypass’ login or not. What do you think @vits-hugs ?

codecov · 2025-05-15T06:44:39Z

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.23%. Comparing base (e8200a0) to head (49266dd).
⚠️ Report is 58 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...g/apache/cloudstack/saml/SAML2AuthManagerImpl.java	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.20   #10868   +/-   ##
=========================================
  Coverage     16.23%   16.23%           
- Complexity    13371    13374    +3     
=========================================
  Files          5657     5657           
  Lines        498860   498864    +4     
  Branches      60543    60544    +1     
=========================================
+ Hits          81003    81004    +1     
- Misses       408824   408825    +1     
- Partials       9033     9035    +2

Flag	Coverage Δ
uitests	`4.00% <ø> (ø)`
unittests	`17.09% <20.00%> (+<0.01%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

yadvr · 2025-05-15T11:59:02Z

Could be a security risk - perhaps wrap around a global setting. Some orgs may not want that?

vits-hugs · 2025-05-15T18:38:03Z

I think that displaying warning would make more sense, after you disable the SAML login, since if an admin would want to unable a user to access the application, it should disable the account. What do you think about that?

DaanHoogland · 2025-05-16T07:17:47Z

I think that displaying warning would make more sense, after you disable the SAML login, since if an admin would want to unable a user to access the application, it should disable the account. What do you think about that?

I think certain operators would want to be able to guarantee a user can only login via SSO. For some other operators your change makes sense. That is why we are asking to make the behaviour configurable.

vits-hugs · 2025-06-03T20:05:49Z

@DaanHoogland @rohityadavcloud, As requested i added a configuration to make the behaviour configurable, please review the changes.

DaanHoogland

clgtm

vits-hugs · 2025-06-05T17:51:13Z

@blueorangutan package

blueorangutan · 2025-06-05T17:52:03Z

@vits-hugs a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-06-05T18:48:34Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13639

DaanHoogland · 2025-06-09T14:30:00Z

@blueorangutan test

blueorangutan · 2025-06-09T14:32:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-06-10T07:07:05Z

[SF] Trillian test result (tid-13483)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 56764 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10868-t13483-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_secure_vm_migration	`Error`	136.84	test_vm_life_cycle.py
test_01_secure_vm_migration	`Error`	136.85	test_vm_life_cycle.py

weizhouapache · 2025-06-10T07:22:00Z

I am not a SAML user, but I have two questions

if the SAML user is disabled, should we allow it to log into cloudstack ?
what the default value of new global settings should be ?

DaanHoogland · 2025-12-12T10:04:31Z

@vits-hugs , do you still want to move this forwards? It need addressing @weizhouapache ’s comments and a test/review.

erikbocks · 2025-12-12T14:13:39Z

Hello, @DaanHoogland and @weizhouapache

@vits-hugs , do you still want to move this forwards? It need addressing @weizhouapache ’s comments and a test/review.

Yes, I think we can move this PR forward. I will take care of it on behalf of Vitor.

if the SAML user is disabled, should we allow it to log into cloudstack ?

It's not the SAML user that is disabled, but the possibility of logging in with SAML SSO.

As an example, let's say the user used to log in with their own ACS credentials (username and password), and then SAML authentication was added to the environment by the operators. If, after a while, the operators decide to remove the SAML authentication from the environment, the user cannot log in anymore, neither with SAML (as it was removed), nor with their credentials.

Thus, this PR allows users in these scenarios to log in with their credentials again, even if their SAML login was disabled. If the operator wishes to actually disable the user, they should use lockUser or disableUser instead.

what the default value of new global settings should be ?

Personally, I think that the default value for the configuration should be true, as it doesn't make sense to disable all access when a dedicated API for it already exists, and the intuitive behavior would be to return to the state before it was enabled in the first place.

However, I also understand that this PR implements a sensitive change and it could take operators by surprise. Therefore, I propose the change of the default value to false and letting operators decide whether they want it to be possible or not. What do you think about it? @DaanHoogland @weizhouapache

DaanHoogland · 2025-12-15T08:12:28Z

I think your approach is sane @erikbocks . The only snag is that saml users may not have a local account at all.

hsato03

clgtm, I agree changing the new global setting to have the default value as false to keep the current behavior.

weizhouapache · 2025-12-19T08:11:15Z

clgtm, I agree changing the new global setting to have the default value as false to keep the current behavior.

yes, the default value should be false

@erikbocks
can you rebase with 4.20 ? we do not maintain 4.19 any more.
for the global setting name, enable.login.saml.unathourized implies the saml user is unathourized, not disabled. should it be changed ?

…n.with.disabled.saml

RosiKyu

LGTM

Tested PR #10868 which adds the enable.login.saml.unathourized setting to control login behavior when SAML SSO is disabled for a user.

Environment: CloudStack 4.19.3.0-SNAPSHOT, Keycloak 22.0 as SAML IdP

Test Execution Summary

TC	Description	Result
TC1	Password login succeeds when SAML disabled with setting=true (source changes to UNKNOWN)	PASSED
TC2	Password login blocked when SAML disabled with setting=false (source changes to SAML2DISABLED)	PASSED
TC3	SAML SSO login fails when user not authorized for SAML	PASSED
TC4	SAML SSO login succeeds when user authorized for SAML	PASSED

All test cases passed. The new setting correctly controls whether users can fall back to password authentication when their SAML authorization is disabled.

Detailed Test Report

TC1: Login with password after SAML disabled (setting=true)

Objective
Verify that when enable.login.saml.unathourized=true, a user whose SAML SSO has been disabled can successfully login using username and password. This is the core bug fix of PR #10868.

Test Steps

Set global configuration enable.login.saml.unathourized to true
Disable SAML SSO for the test user using authorizeSamlSso enable=false
Verify user source changed to UNKNOWN in database
Attempt login via CloudStack UI Portal login with username/password

Expected Result:

User source should change from SAML2 to UNKNOWN (not SAML2DISABLED)
User should be able to login successfully with username and password

Actual Result:

User source changed to UNKNOWN
User successfully logged in via Portal login

Test Evidence:

(admin) 🐱 > update configuration name=enable.login.saml.unathourized value=true
{
  "configuration": {
    "category": "Advanced",
    "component": "SAML2-PLUGIN",
    "defaultvalue": "true",
    "description": "When enabled, if SAML SSO is disabled, enables user to login with user and password, otherwise a user with SAML SSO disabled cannot login",
    "displaytext": "Enable login saml unathourized",
    "group": "Access",
    "isdynamic": true,
    "name": "enable.login.saml.unathourized",
    "subgroup": "SAML",
    "type": "Boolean",
    "value": "true"
  }
}
(admin) 🐱 > authorizeSamlSso userid=13c22633-feff-4f9b-9b47-43319c8eb719 enable=false
{
  "success": true
}

mysql> SELECT id, username, source FROM cloud.user WHERE username='samluser';
+----+----------+---------+
| id | username | source  |
+----+----------+---------+
|  5 | samluser | UNKNOWN |
+----+----------+---------+
1 row in set (0.00 sec)

UI Login: Successfully logged in as "SAML TestUser" via Portal login
Event log shows: "26 Jan 2026 15:02:16 USER.LOGIN - samluser user has logged in from IP Address /10.0.3.251"

TC2: Login with password after SAML disabled (setting=false)

Objective
Verify that when enable.login.saml.unathourized=false, a user whose SAML SSO has been disabled CANNOT login using username and password. This confirms the configurable behavior works correctly - operators can choose to block password login for SAML-disabled users.

Test Steps

Re-enable SAML for the test user (pre-requisite from TC1)
Set global configuration enable.login.saml.unathourized to false
Disable SAML SSO for the test user using authorizeSamlSso enable=false
Verify user source changed to SAML2DISABLED in database
Attempt login via CloudStack UI Portal login with username/password

Expected Result:

User source should change from SAML2 to SAML2DISABLED
User should NOT be able to login with username and password
Login should fail with authentication error

Actual Result:

User source changed to SAML2DISABLED
Login failed with error message

Test Evidence:

(admin) 🐱 > update configuration name=enable.login.saml.unathourized value=false
{
  "configuration": {
    "category": "Advanced",
    "component": "SAML2-PLUGIN",
    "defaultvalue": "true",
    "description": "When enabled, if SAML SSO is disabled, enables user to login with user and password, otherwise a user with SAML SSO disabled cannot login",
    "displaytext": "Enable login saml unathourized",
    "group": "Access",
    "isdynamic": true,
    "name": "enable.login.saml.unathourized",
    "subgroup": "SAML",
    "type": "Boolean",
    "value": "false"
  }
}
(admin) 🐱 > authorizeSamlSso userid=13c22633-feff-4f9b-9b47-43319c8eb719 enable=false
{
  "success": true
}

mysql> SELECT id, username, source FROM cloud.user WHERE username='samluser';
+----+----------+---------------+
| id | username | source        |
+----+----------+---------------+
|  5 | samluser | SAML2DISABLED |
+----+----------+---------------+
1 row in set (0.00 sec)

UI Login Error: "Error 531: Failed to authenticate user samluser in domain 1; please provide valid credentials"

**Status: PASSED **

TC3: SAML SSO Login Fails When User Unauthorized

Objective
Verify that when SAML SSO authorization is disabled for a user, attempting to login via SAML SSO fails with an appropriate error message. This confirms that the SAML authorization check is working correctly.

Test Steps

Ensure SAML IdP (Keycloak) is configured and working
Disable SAML SSO for the test user using authorizeSamlSso enable=false
Attempt login via CloudStack UI using "Single sign-on" option
Authenticate with Keycloak (samluser/password123)

Expected Result:

SAML authentication with IdP should succeed
CloudStack should reject the login because user is not authorized for SAML SSO
Error message should indicate user is not authorized

Actual Result:

SAML authentication with Keycloak succeeded
CloudStack rejected login with authorization error

Test Evidence:

(admin) 🐱 > authorizeSamlSso userid=13c22633-feff-4f9b-9b47-43319c8eb719 enable=false
{
  "success": true
}

SSO Login Error:

<loginresponse cloud-stack-version="4.19.3.0-SNAPSHOT">
  <errorcode>531</errorcode>
  <errortext>Your authenticated user is not authorized for SAML Single Sign-On, please contact your administrator</errortext>
</loginresponse>

**Status: PASSED **

TC4: SAML SSO Login Works For Authorized Users

Objective
Verify that when SAML SSO authorization is enabled for a user with the correct IdP entity, the user can successfully login via SAML SSO. This confirms the baseline SAML functionality works correctly.

Test Steps

Ensure SAML IdP (Keycloak) is configured and working
Enable SAML SSO for the test user with specific IdP entity using authorizeSamlSso enable=true entityid=<idp_url>
Attempt login via CloudStack UI using "Single sign-on" option
Authenticate with Keycloak (samluser/password123)

Expected Result:

SAML authentication with IdP should succeed
CloudStack should accept the login and redirect to dashboard
User should be logged in successfully

Actual Result:

SAML authentication with Keycloak succeeded
User redirected to CloudStack dashboard
Dashboard shows "SAML TestUser" logged in

Test Evidence:

(admin) 🐱 > authorizeSamlSso userid=13c22633-feff-4f9b-9b47-43319c8eb719 enable=true entityid=http://10.0.35.115:8081/realms/cloudstack
{
  "success": true
}

UI Login: Successfully logged in as "SAML TestUser" via SAML SSO
Event log shows: "26 Jan 2026 16:15:58 USER.LOGIN - samluser user has logged in from IP Address /10.0.3.251"

Screencast.from.2026-01-26.18-15-50.webm

**Status: PASSED **

vits-hugs changed the title ~~Fix check~~ Fix saml bug unable to login May 14, 2025

DaanHoogland reviewed May 15, 2025

View reviewed changes

DaanHoogland force-pushed the 4.19 branch from 8ef8b46 to 857ccb0 Compare May 27, 2025 14:19

boring-cyborg bot added the component:saml label Jun 3, 2025

DaanHoogland approved these changes Jun 4, 2025

View reviewed changes

sureshanaparti added this to the 4.19.4 milestone Jun 5, 2025

weizhouapache modified the milestones: 4.19.4, 4.20.2 Sep 2, 2025

weizhouapache modified the milestones: 4.20.2, 4.20.3 Sep 10, 2025

DaanHoogland added the status:needs-testing label Dec 15, 2025

hsato03 approved these changes Dec 18, 2025

View reviewed changes

Vitor Hugo Homem Marzarotto and others added 3 commits December 19, 2025 10:38

Fix check

14a7871

Adds configuration for behaviour, when SAML SSO is disabled for a user

652094d

set default configuration value to false and rename it to enable.logi…

49266dd

…n.with.disabled.saml

erik-bock-silva force-pushed the fix-saml2-bug-unable-to-login branch from a578d56 to 49266dd Compare December 19, 2025 13:42

boring-cyborg bot added component:agent component:build component:integration labels Dec 19, 2025

JoaoJandre changed the base branch from 4.19 to 4.20 December 19, 2025 13:46

erikbocks removed component:agent component:build component:integration labels Dec 19, 2025

RosiKyu self-assigned this Jan 26, 2026

RosiKyu approved these changes Jan 26, 2026

View reviewed changes

RosiKyu added status:ready-for-merge and removed status:needs-testing labels Jan 26, 2026

Fix saml bug unable to login #10868

Are you sure you want to change the base?

Fix saml bug unable to login #10868

Conversation

vits-hugs commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Bug Severity

How Has This Been Tested?

Uh oh!

vits-hugs commented May 14, 2025

Uh oh!

blueorangutan commented May 14, 2025

Uh oh!

blueorangutan commented May 14, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yadvr commented May 15, 2025

Uh oh!

vits-hugs commented May 15, 2025

Uh oh!

DaanHoogland commented May 16, 2025

Uh oh!

vits-hugs commented Jun 3, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

vits-hugs commented Jun 5, 2025

Uh oh!

blueorangutan commented Jun 5, 2025

Uh oh!

blueorangutan commented Jun 5, 2025

Uh oh!

DaanHoogland commented Jun 9, 2025

Uh oh!

blueorangutan commented Jun 9, 2025

Uh oh!

blueorangutan commented Jun 10, 2025

Uh oh!

weizhouapache commented Jun 10, 2025

Uh oh!

DaanHoogland commented Dec 12, 2025

Uh oh!

erikbocks commented Dec 12, 2025

Uh oh!

DaanHoogland commented Dec 15, 2025

Uh oh!

hsato03 left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Dec 19, 2025

Uh oh!

RosiKyu left a comment

Choose a reason for hiding this comment

LGTM

Test Execution Summary

Detailed Test Report

TC1: Login with password after SAML disabled (setting=true)

TC2: Login with password after SAML disabled (setting=false)

TC3: SAML SSO Login Fails When User Unauthorized

TC4: SAML SSO Login Works For Authorized Users

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

vits-hugs commented May 14, 2025 •

edited

Loading

codecov bot commented May 15, 2025 •

edited

Loading