OWASP / OWASP/SecurityShepherd
authUserSSO result array: result[3] unset, result[5] double-assigned, wrong log message
@ismisepaul is already working on this.
Since Apr 1, 2026.
- Dominant language
- Java
- Stars
- 1.5k
- Forks
- 515
- Avg merge
- 3h 46m
- Merged PRs (30d)
- 1
Description
Problem
Getter.authUserSSO() has two copy-paste bugs from authUser() that affect the result array contract:
1. result[5] assigned twice — first value silently lost
// Getter.java lines 454-456
result[5] = "false"; // sso logins can't change password
result[4] = classId; // classId
result[5] = Boolean.toString(isTempUsername); // overwrites the line above
The first assignment ("false") is immediately overwritten by isTempUsername. The comment suggests the intent was to set a "can't change password" flag at index 3 (the tempPassword slot), not index 5.
2. result[3] (tempPassword flag) never set — left as null
The authUser() method sets all 6 indices:
// authUser contract (Getter.java lines 158-166)
result[0] = userId
result[1] = userName
result[2] = role
result[3] = Boolean.toString(tempPassword) // ← set in authUser
result[4] = classId
result[5] = Boolean.toString(tempUsername)
authUserSSO() skips result[3] entirely. Any caller that reads result[3] (e.g. Boolean.parseBoolean(result[3])) will get false by accident (parseBoolean treats null as false), but any caller that does result[3].equals(...) will NPE.
3. Log message says "End authUser" instead of "End authUserSSO"
// Getter.java line 458
log.debug("$$$ End authUser $$$"); // should be authUserSSO
Misleading when debugging auth flows in logs.
Proposed fix
result[0] = userID;
result[1] = userName;
result[2] = userRole;
result[3] = "false"; // tempPassword — SSO logins can't change password
result[4] = classId;
result[5] = Boolean.toString(isTempUsername);
log.debug("$$$ End authUserSSO $$$");
This aligns authUserSSO with the same result array contract as authUser.
Risk
Low — the fix is mechanical (reorder assignments, fix log message). No logic change, no schema change. But it touches a security-sensitive auth path, so it should be reviewed carefully and tested with SSO login flow if available.
Context
Identified by Copilot code review on PR #817 (connection leak fixes). Deferred from that PR to avoid scope creep in a pooling/leak-fix change.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.