Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix AuditCheckMockMvcTests with ldap profile #3206

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

Kehrlann
Copy link
Contributor

The AuditCheckMockMvcTests started failing when running with the ldap + default profiles, after gh-3186. We can see the issue in Concourse

This is because:

  1. We removed the unexpected behavior in @DefaultTestContext which set the profiles to default regardless of what profile was passed in from the command line. This means the failing tests had not run with the ldap profile for years.
  2. We don't run tests with the ldap profile on Github actions, so we did not catch this early (We probably should).
  3. There is an old commit from 2019 that had reworked that test class, and lost some of the nuance - it used to be "at least 3 events", not "exactly 3 events". See for example the diff below. At the time the commit was made, the default profile was already active and so there was never an ldap provider here.
-        ArgumentCaptor<AbstractUaaEvent> captor = ArgumentCaptor.forClass(AbstractUaaEvent.class);
-        verify(listener, atLeast(3)).onApplicationEvent(captor.capture());
+        assertNumberOfAuditEventsReceived(3);

The failing tests verify that audit events are emitted when the user tries to log in with an incorrect password. Without LDAP AuthzAuthenticationManager (internal UAA userstore) emits an IdentityProviderAuthenticationFailureEvent, and then two technical events are emitted, for a total of three. When the ldap profile is turned on, the DynamicLdapAuthenticationManager also tries to authenticate the user, and also throws an IdentityProviderAuthenticationFailureEvent, bringing the total number of events to 4.

Same goes for userNotFoundLoginUnsuccessfulTest but the use-case is "user not found" and 2/3 events.

This fixes those failing tests, by ignoring the extra events that having an LDAP provider creates.

@Kehrlann Kehrlann force-pushed the dgarnier/fix-ldap-tests branch from d470c68 to eddf83e Compare December 19, 2024 21:42
@Kehrlann Kehrlann force-pushed the dgarnier/fix-ldap-tests branch from eddf83e to e05948b Compare December 19, 2024 21:46
@duanemay duanemay merged commit e814390 into cloudfoundry:develop Dec 19, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants