Skip to content

Fix NPE in LDAP user lookup when memberOf attribute is absent (service accounts)#178

Merged
dk1844 merged 3 commits into
masterfrom
copilot/fix-npe-ldap-user-lookup
Jun 24, 2026
Merged

Fix NPE in LDAP user lookup when memberOf attribute is absent (service accounts)#178
dk1844 merged 3 commits into
masterfrom
copilot/fix-npe-ldap-user-lookup

Conversation

Copilot AI commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

LdapUserRepository.resultToUserEntry crashes with a NullPointerException for LDAP principals that have no memberOf attribute (e.g. service accounts), because attrs.get("memberOf") returns null and .getAll() is called unconditionally.

Overview

Fix — make memberOf access null-safe in resultToUserEntry:

  • Option(attrs.get("memberOf")) defaults to Seq.empty[String] when the attribute is absent
  • Emits a WARN log when memberOf is missing so the gap is visible without noise
// Before
val groups = attrs.get("memberOf").getAll.asScala.map(...)  // NPE if absent

// After
val groups = Option(attrs.get("memberOf")) match {
  case None =>
    logger.warn(s"No memberOf attribute found for user: $username - defaulting to empty groups")
    Seq.empty[String]
  case Some(memberOfAttr) =>
    memberOfAttr.getAll.asScala.map(group => group.toString.substring(3, group.toString.indexOf(","))).toSeq
}

TestabilityresultToUserEntry changed from private to private[search] to allow direct unit testing.

Tests added in LdapUserRepositoryTest using ScalaMock covering:

  • Normal user with one or more memberOf group DNs parsed correctly
  • Service account with no memberOf attribute → empty groups, no exception
  • User with memberOf present but empty → empty groups
  • Optional attributes mapped correctly when memberOf is absent

Release Notes

  • Fix: LDAP user lookup no longer throws NullPointerException for principals without a memberOf attribute (e.g. service accounts); missing memberOf now defaults to empty groups

Related

Closes #177

Copilot AI added 2 commits June 24, 2026 12:58
- Make memberOf attribute access null-safe in resultToUserEntry
- Log a warning when memberOf is absent (e.g. service accounts)
- Expose resultToUserEntry as private[search] for testability
- Add unit tests covering: groups present, no memberOf (service account),
  empty memberOf, and optional attributes with absent memberOf
Copilot AI changed the title [WIP] Fix NPE in LDAP user lookup for missing memberOf attribute Fix NPE in LDAP user lookup when memberOf attribute is absent (service accounts) Jun 24, 2026
Copilot AI requested a review from lsulak June 24, 2026 13:00

@lsulak lsulak left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I checked the OneDS and the user indeed does have memberOf atteibute empty. The fix basically just wraps the processing of this attribute into Option and matches on it. Also added few unit tests, this should be ok, approving but feel free to check @TheLydonKing or @dk1844

@lsulak lsulak marked this pull request as ready for review June 24, 2026 13:07
@lsulak lsulak requested review from dk1844 and jakipatryk as code owners June 24, 2026 13:07

@dk1844 dk1844 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM (read the code, ran tests)

@github-actions

Copy link
Copy Markdown

JaCoCo Coverage Report

Metric (instruction) Coverage Threshold Status
Overall 71.91% 71.0%
Changed Files 40.38% 80.0%
Group Coverage (O/Ch) Threshold (O/Ch) Status (O/Ch)
login-service-api 73.74% / 40.38% 73.0% / 80.0% ✅/❌
Report Coverage (O/Ch) Threshold (O/Ch) Status (O/Ch)
Report: api - scala:2.12.17 73.74% / 40.38% 73.0% / 80.0% ✅/❌
File Path Coverage Threshold Status
LdapUserRepository.scala 40.38% 60.0%

Run 28100271342 · Event: pull_request

@dk1844 dk1844 merged commit 17937a3 into master Jun 24, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LDAP user lookup throws NPE when memberOf attribute is missing (service accounts)

3 participants