NotificationsYou must be signed in to change notification settings
Fork9.6k
Star30.4k

[Security] Return null instead of empty username to fix deprecation notice#59586

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

phasdev wants to merge2 commits intosymfony:6.4fromphasdev:fix_59584

Closed

[Security] Return null instead of empty username to fix deprecation notice#59586

phasdev wants to merge2 commits intosymfony:6.4fromphasdev:fix_59584

Conversation

Copy link

Contributor

phasdev commentedJan 22, 2025•
edited by chalasr
Loading

Q	A
Branch?	6.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues	Fix#59584
License	MIT

RemoteUserAuthenticator may return an empty string when extracting a username from the configured$_SERVER parameter (e.g.REMOTE_USER).

An empty username triggers theUser Deprecated: Since symfony/security-http 7.2: Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.

Returnnull instead of empty username to skip authenticator when username is empty and fix Symfony 8 deprecation notice.

[Security] Return null instead of empty username to fix deprecation n…

aaeb846

…otice

phasdev requested a review fromchalasr as acode owner

January 22, 2025 21:58

carsonbot added Status: Needs Review Bug Security labels

Jan 22, 2025

This comment was marked as outdated.

Copy link

ContributorAuthor

phasdev commentedJan 22, 2025

Therelease page shows5.4 accepting security fixes. Since this is part of the Security component, I figured that's where the change should be made. Please let me know if you'd prefer I make this change in6.4 and I'll submit a new PR.

Copy link

Member

alexandre-daubois commentedJan 24, 2025•
edited
Loading

This should be a bug fix, not a security fix. Security fixes corresponds to modifications that address CVEs or vulnerability that endangers apps security, likethese reports.

phasdev changed the base branch from5.4 to6.4

January 24, 2025 16:47

Copy link

ContributorAuthor

phasdev commentedJan 24, 2025•
edited
Loading

@alexandre-daubois Thanks for the clarification; I've changed the base branch to 6.4.

Copy link

Member

nicolas-grekas commentedJan 28, 2025

Can you please add a test case to cover this?

xabbuh reviewed

Jan 28, 2025

View reviewed changes

src/Symfony/Component/Security/Http/Authenticator/RemoteUserAuthenticator.php

		@@ -45,6 +45,6 @@ protected function extractUsername(Request $request): ?string
		throw new BadCredentialsException(sprintf('User key was not found: "%s".', $this->userKey));
		}

		return $request->server->get($this->userKey);
		return $request->server->get($this->userKey) ?: null;