Skip to content

authUserSSO result array: result[3] unset, result[5] double-assigned, wrong log message #820

@ismisepaul

Description

@ismisepaul

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.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

Status

Next Release Backlog

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions