Skip to content

Conversation

@dk1844
Copy link
Collaborator

@dk1844 dk1844 commented Aug 1, 2025

This PR introduces different non-ok response to users besides the original 401, on LDAP connection errors.

In this particular situation, failing to connect to LDAP should give users 504.

Example run with mangled config to induce the error:

POST http://localhost:9090/token/generate?groups-prefixes=
with e.g. Authorization: Basic bXl1c2VybmFtZTp3aGF0LWFyZS15b3UtbG9va2luZy1mb3ItaGVyZT8=

->
504

{"error": "LDAP connection failed: LDAP connection issue: corp.bogus.com:636; nested exception is javax.naming.CommunicationException: corp.bogus.com:636 [Root exception is java.net.UnknownHostException: corp.bogus.com]"}

@TheLydonKing

Release notes:

  • For LDAP connection errors, 504 error code is communciated to signify the state difference compared to previsouly present 504

Closes #133

@dk1844 dk1844 requested a review from jakipatryk as a code owner August 1, 2025 12:42
@dk1844 dk1844 requested a review from TheLydonKing August 1, 2025 13:27
@github-actions
Copy link

github-actions bot commented Aug 1, 2025

JaCoCo code coverage report - scala:2.12.17

File Coverage [58.1%]
AuthManagerConfig.scala 96.88% 🍏
SecurityConfig.scala 88.37% 🍏
JWTService.scala 82.77% 🍏
RestResponseEntityExceptionHandler.scala 62.71%
ActiveDirectoryLDAPAuthenticationProvider.scala 51.32%
LdapUserRepository.scala 21.53%
KerberosFailureHandler.scala 0%
KerberosSPNEGOAuthenticationProvider.scala 0%
LdapConnectionException.scala 0%
Total Project Coverage 68.84% 🍏

Copy link
Collaborator Author

@dk1844 dk1844 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@TheLydonKing TheLydonKing left a comment

Choose a reason for hiding this comment

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

LGTM and tested on my side

Copy link
Collaborator Author

@dk1844 dk1844 left a comment

Choose a reason for hiding this comment

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

LGTM

@dk1844 dk1844 merged commit 7c94e10 into master Aug 5, 2025
3 of 4 checks passed
@dk1844 dk1844 deleted the bugfix/133-ldap-error50x branch August 15, 2025 07:01
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 error 401 -> 50x

3 participants