Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security/Http] Fix compat of persistent remember-me with legacy tokens#49103

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

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?5.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#49100
LicenseMIT
Doc PR-

In#49078, we changed the format of remember-me tokens, effectively invalidating them all.
While the invalidation is intentional for signature-based remember-me handlers, persistent remember-me handlers could accept both legacy and updated tokens.
This PR fixes compat with legacy tokens for persistent remember-me handlers.

allan-simon reacted with heart emoji
@carsonbotcarsonbot added this to the5.4 milestoneJan 25, 2023
@nicolas-grekasnicolas-grekas changed the titleUpdate CHANGELOG for 5.4.19[Security/Http] Fix compat of persistent remember-me with legacy tokensJan 25, 2023
}
$this->tokenProvider =$tokenProvider;
$this->tokenVerifier =$tokenVerifier;
$this->secret =$secret;
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

unused property after#49078

Copy link
Member

Choose a reason for hiding this comment

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

should we deprecate the constructor argument ?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

we could but not in 5.4

Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

How long should they be supported?

@nicolas-grekas
Copy link
MemberAuthor

We can keep that logic forever, we don't really care IMHO.

chalasr reacted with thumbs up emoji

@eexit
Copy link

Hi@nicolas-grekas,

Thanks for the quick fix.
I'm struggling with testing your PR. I use Symfony Flex and after adding this into mycomposer.json, it looks like I still can't install the forked bundled:

"repositories": [    {"_comment":"Testing: https://github.com/symfony/symfony/pull/49103","type":"git","url":"https://github.com/nicolas-grekas/symfony.git"    }]

Doing this command does not work:composer require -W "symfony/security-bundle:dev-sec-http-legacy-persistent-token" --prefer-source.

composer require -W "symfony/security-bundle:dev-sec-http-legacy-persistent-token" --prefer-source./composer.json has been updatedRunning composer update symfony/security-bundle --with-all-dependenciesLoading composer repositories with package informationInfo from https://repo.packagist.org/: #StandWithUkraineUpdating dependenciesYour requirements could not be resolved to an installable set of packages.  Problem 1    - Root composer.json requires symfony/security-bundle dev-sec-http-legacy-persistent-token, found symfony/security-bundle[2.0.7, ..., 2.8.x-dev, v3.0.0-BETA1, ..., 3.4.x-dev, v4.0.0-BETA1, ..., 4.4.x-dev, v5.0.0-BETA1, ..., 5.4.x-dev, v6.0.0-BETA1, ..., 6.3.x-dev] but it does not match the constraint.  Problem 2    - easycorp/easyadmin-bundle is locked to version v*** and an update of this package was not requested.    - easycorp/easyadmin-bundle v*** requires symfony/security-bundle ^5.4|^6.0 -> found symfony/security-bundle[v5.4.0-BETA1, ..., 5.4.x-dev, v6.0.0-BETA1, ..., 6.3.x-dev] but it conflicts with your root composer.json require (dev-sec-http-legacy-persistent-token).Installation failed, reverting ./composer.json and ./composer.lock to their original content.

It looks like composer is ignoring your repo and I have no idea why.
Would you mind giving some insights on how to test this?

Thanks!

@nicolas-grekasnicolas-grekas merged commit96cdc5c intosymfony:5.4Jan 25, 2023
@nicolas-grekasnicolas-grekas deleted the sec-http-legacy-persistent-token branchJanuary 25, 2023 18:24
This was referencedFeb 1, 2023
fabpot added a commit that referenced this pull requestApr 8, 2023
…stentRememberMeHandler constructor (xabbuh)This PR was merged into the 6.3 branch.Discussion----------[Security] deprecate the $secret argument of the PersistentRememberMeHandler constructor| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | no| Deprecations? | yes| Tickets       |#49103 (comment)| License       | MIT| Doc PR        |Commits-------bf81d66 deprecate the $secret argument of the PersistentRememberMeHandler constructor
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof approved these changes

@chalasrchalasrchalasr approved these changes

@wouterjwouterjAwaiting requested review from wouterj

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@nicolas-grekas@eexit@stof@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp