npe guard on missing session attribute - #13871
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13871 +/- ##
=========================================
Coverage 16.26% 16.27%
- Complexity 13434 13439 +5
=========================================
Files 5667 5667
Lines 500731 500731
Branches 60803 60803
=========================================
+ Hits 81455 81476 +21
+ Misses 410172 410151 -21
Partials 9104 9104
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland 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. |
There was a problem hiding this comment.
Pull request overview
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.
Adds a null-safe guard when reading the 2FA verification flag from the HTTP session to avoid NPEs when the attribute is missing, and updates unit tests accordingly.
Changes:
- Make
skip2FAcheckForUserresilient to absentIS_2FA_VERIFIEDsession attribute. - Update Mockito usage in tests (static
whenimport) and add a regression test for the missing-attribute scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/src/main/java/com/cloud/api/ApiServlet.java | Makes 2FA verification flag retrieval null-safe via Boolean.TRUE.equals(...). |
| server/src/test/java/com/cloud/api/ApiServletTest.java | Refactors some Mockito stubbing and adds a test covering absent 2FA session attribute. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ConfigKey<Boolean> enableUserTwoFactorAuthentication = Mockito.mock(ConfigKey.class); | ||
| AccountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthentication; | ||
| when(enableUserTwoFactorAuthentication.valueIn(1L)).thenReturn(false); | ||
|
|
||
| boolean result = servlet.skip2FAcheckForUser(session); | ||
| Assert.assertEquals(true, result); |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18859 |
|
Description
This PR...
Fixes: #13815
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?