Improvements for automated login events#3857
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
estringana
reviewed
May 5, 2026
Benchmarks [ appsec ]Benchmark execution time: 2026-05-06 00:23:00 Comparing candidate commit 734b8fc in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. |
Benchmarks [ tracer ]Benchmark execution time: 2026-05-06 01:03:16 Comparing candidate commit 734b8fc in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 194 metrics, 0 unstable metrics. |
cataphract
force-pushed
the
automated-login-event-improvs
branch
from
da9643c to
0c4b0e8
Compare
cataphract
force-pushed
the
automated-login-event-improvs
branch
from
8324701 to
734b8fc
Compare
Contributor
Author
|
system test failures are expected because the system-tests rely on calling on the implementation-detail _automated php functions. This will be handed on a separate system-tests pr |
estringana
approved these changes
May 6, 2026
…ogin failure - Accept optional `framework` parameter in all `track_user_*` PHP functions - Emit `missing_user_login` / `missing_user_id` telemetry metrics tagged with the framework name when login/id are absent - Pass `'laravel'`, `'symfony'`, `'wordpress'` from each integration - WordPress: stop silently swallowing failures with empty username (pass null through so missing_user_login telemetry fires correctly) - Add WordPress integration test app and WordPressTests test class - Update Laravel8x and Symfony62 integration tests to assert framework tag on missing_user_login / missing_user_id metrics Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Symfony's auto-instrumented login_failure hooks were calling
track_user_login_failure_event_automated(null, null, false, ...);
Laravel's pre-existing post-hook on SessionGuard::attempt did extract
the login from credentials but always reported the user id as null. With
the framework tag now emitted on missing_user_login / missing_user_id
telemetry, every failure on these frameworks was being counted as
missing usr.login or usr.id even when the framework had it in hand. This
commit teaches both integrations to extract the attempted login and,
where available, the resolved user id, populating
appsec.events.users.login.failure.usr.{login,id}.
Symfony
-------
Symfony's Security component went through three generations and we hook the
class on the login-failure path of each. The (Request, AuthenticationException)
signature is shared across all three, so a single extractor handles all of
them.
Guard component (4.x-5.x; deprecated 5.3, removed in 6.0)
AbstractGuardAuthenticator implements Guard\AuthenticatorInterface
↑ extends
AbstractFormLoginAuthenticator <-- hooked
::onAuthenticationFailure(Request, AuthenticationException)
Legacy HTTP firewall listener (5.x; removed in 6.0)
AbstractListener implements FirewallListenerInterface
↑ extends
AbstractAuthenticationListener <-- hooked
::onFailure(Request, AuthenticationException) [private]
New authenticator system (5.1 experimental, 5.3 stable, only option in 6.0+)
AuthenticatorInterface
↑ implemented by
AbstractAuthenticator
↑ extends
AbstractLoginFormAuthenticator implements
AuthenticationEntryPointInterface,
InteractiveAuthenticatorInterface
↑ extends
FormLoginAuthenticator <-- hooked
::onAuthenticationFailure(Request, AuthenticationException)
The exception side is shared:
\RuntimeException
↑
AuthenticationException (getToken(): ?TokenInterface,
setToken(TokenInterface))
↑ extends
UserNotFoundException (5.3+) (getUserIdentifier(): ?string)
UsernameNotFoundException (≤5.2; in 5.3-5.4 a class_alias forwarder
to UserNotFoundException via class_exists();
removed in 6.0; getUsername()/setUsername())
BadCredentialsException (no identifier accessor)
Token attachment differs by manager:
- AuthenticationProviderManager (legacy provider chain) calls
$exception->setToken($token) before throwing, so the failed identifier
survives via getToken()->getUserIdentifier().
- AuthenticatorManager (new system, only option from 6.0) does not. Its
handleAuthenticationFailure is invoked after the Passport's UserBadge
resolution has thrown; it does not setToken on the exception. By default
(hideUserNotFoundExceptions = true) it wraps the original UserNotFoundException
inside a fresh BadCredentialsException as $previous to prevent user
enumeration — so the identifier is still reachable, just one level deeper
in the exception chain.
extractLoginFromAuthFailure() therefore tries three sources in order:
A. $exception->getToken()->getUserIdentifier() — picks up the legacy
ProviderManager case and any custom code that calls setToken().
B. Walk the exception chain via getPrevious() for a node exposing
getUserIdentifier() (UserNotFoundException, 5.3+) or the deprecated
getUsername() (UsernameNotFoundException). Reaches the wrapped
UserNotFoundException under the new manager.
C. $request->getSession()->get('_security.last_username') — the form-login
authenticator stashes the submitted username into the session under
this key as the very first step of authenticate() (in getCredentials(),
before the UserBadge is constructed). The constant is
SecurityRequestAttributes::LAST_USERNAME.
usr.id is left null on Symfony failures: AuthenticationException carries a
TokenInterface at most (and only sometimes), not a UserInterface.
Laravel
-------
Laravel's auth contracts and the concrete classes we hook:
Illuminate\Contracts\Auth\Guard
↑ extends
Illuminate\Contracts\Auth\StatefulGuard
↑ implemented by
Illuminate\Auth\SessionGuard implements
StatefulGuard, SupportsBasicAuth (Laravel >= 5.2)
Illuminate\Auth\Guard (Laravel <= 5.1;
a separate concrete
class, distinct from
the 5.0+ Guard contract)
Illuminate\Contracts\Auth\Authenticatable (getAuthIdentifier())
Illuminate\Contracts\Auth\UserProvider (retrieveByCredentials,
validateCredentials)
Illuminate\Auth\Events\Failed (introduced in 5.2.35)
Illuminate\Auth\Events\Login (plain event objects;
no shared base)
SessionGuard::attempt() (5.2+) is the canonical entry point and runs:
1. fireAttemptEvent($credentials, $remember)
2. $user = $provider->retrieveByCredentials($credentials) // by email/username
3. hasValidCredentials($user, $credentials) // verifies password
4. on failure: fireFailedEvent($user, $credentials) // dispatches Failed
on success: login($user, $remember); return true
The Failed event therefore carries:
- the resolved $user when retrieveByCredentials matched a row
(i.e., wrong-password case → usr.id is known),
- $user === null when no row matched (user-not-found case),
- the credentials array in either case (so usr.login is always derivable).
Hooking Failed::__construct gives us everything in one place. The signature
changed across versions: ($user, $credentials) on 5.2-5.6,
($guard, $user, $credentials) on 5.7+; we branch on count($args).
The pre-existing SessionGuard::attempt post-hook is kept as a fallback for
the 5.2.0-5.2.34 window (SessionGuard already exists but does not dispatch
Failed yet — fireFailedEvent was added in 5.2.35). It now skips when
class_exists('Illuminate\Auth\Events\Failed', false) is true — i.e.,
whenever the event class has been autoloaded into the request, which only
happens once Laravel itself has dispatched it. Without this guard the
post-hook would fire after Failed::__construct and overwrite the captured
usr.id with null. The Laravel <= 5.1 hook on Illuminate\Auth\Guard::attempt
is unchanged (Auth\Guard was renamed to Auth\SessionGuard in 5.2 and is a
separate concrete class).
Tests
-----
- Symfony62Tests: replace the missing-login telemetry assertion (no longer
triggered, since the integration now extracts '_username=aa' via the
session path) with a usr.login == 'aa' assertion.
- Laravel8xTests: existing login-failure test now also asserts usr.login
== the attempted email; new test 'wrong password for existing user'
asserts usr.exists/usr.id/usr.login when the user is found but the
password is wrong; the missing-login-telemetry test is unchanged
(empty email still produces login=='' which still triggers
missing_user_login + missing_user_id).
- laravel8x LoginController: accept ?password= so the fixture can drive
the wrong-password path against an existing seeded user.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ilure ?string return type hint breaks class loading on PHP 7.0, causing all Symfony instrumentation spans to be lost. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These functions are called by framework integrations, not end users. Placing them under an `internal` sub-namespace makes that explicit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cataphract
force-pushed
the
automated-login-event-improvs
branch
from
734b8fc to
6d4437e
Compare
github-actions
Bot
added
profiling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
missing_user_login/missing_user_idtelemetry metrics tagged with the framework name when login/id are absentReviewer checklist