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.

Sign up for GitHub

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

Jump to bottom

SF6: Use AbstractAuthenticator for TokenAuthenticator #34792

Merged
merged 4 commits into from
Dec 20, 2023
Merged

SF6: Use AbstractAuthenticator for TokenAuthenticator #34792

merged 4 commits into from
Dec 20, 2023

Conversation

M0rgan01
Copy link
Contributor

@M0rgan01 M0rgan01 commented Dec 8, 2023 •
edited
Loading

Questions Answers
Branch? develop
Description? Update anonymous access, see https://symfony.com/doc/current/security.html#allowing-unsecured-access-i-e-anonymous-users. Use AbstractAuthenticator instead of AbstractGuardAuthenticator (deprecated)
Type? improvement
Category? BO
BC breaks? yes
Deprecations? no
How to test? CI and UI tests are green
UI Tests https://github.com/M0rgan01/ga.tests.ui.pr/actions/runs/7197890031
Fixed issue or discussion? Fixes #34762
Related PRs -
Sponsor company -

@M0rgan01 M0rgan01 added this to the 9.0.0 milestone Dec 8, 2023
@prestonBot prestonBot added Bug fix Type: Bug fix BC break Type: Introduces a backwards-incompatible break labels Dec 8, 2023
@M0rgan01 M0rgan01 force-pushed the 34001 branch 8 times, most recently from 0cc5d5b to 37e332e Compare December 11, 2023 12:42
jolelievre
jolelievre reviewed Dec 11, 2023
View reviewed changes
src/Core/Security/OAuth2/TokenAuthenticator.php Outdated
return false;
}
$explode = explode(' ', $authorization);
if (count($explode) >= 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It was already wrong initially, but there should be an else statement that returns false if there are not two parts

@M0rgan01 M0rgan01 force-pushed the 34001 branch 7 times, most recently from 8157f41 to 5359192 Compare December 13, 2023 08:25
@prestonBot prestonBot added develop Branch Improvement Type: Improvement labels Dec 13, 2023
@M0rgan01 M0rgan01 marked this pull request as ready for review December 13, 2023 10:52
@M0rgan01 M0rgan01 requested a review from a team as a code owner December 13, 2023 10:52
@M0rgan01 M0rgan01 requested a review from a team as a code owner December 13, 2023 13:46
matthieu-rolland
matthieu-rolland approved these changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Dec 14, 2023
@M0rgan01 M0rgan01 added BC break Type: Introduces a backwards-incompatible break and removed BC break Type: Introduces a backwards-incompatible break labels Dec 14, 2023
Progi1984
Progi1984 approved these changes Dec 15, 2023
View reviewed changes
Copy link
Member

@Progi1984 Progi1984 left a comment

Choose a reason for hiding this comment

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

🆗 for UI Tests (Ping @PrestaShop/ui-tests-maintainers)

@florine2623 florine2623 added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Dec 19, 2023
@jolelievre jolelievre merged commit 41b1635 into PrestaShop:develop Dec 20, 2023
21 checks passed
@jolelievre
Copy link
Contributor

Thanks @M0rgan01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jolelievre jolelievre jolelievre approved these changes

@matthieu-rolland matthieu-rolland matthieu-rolland approved these changes

@Progi1984 Progi1984 Progi1984 approved these changes

Assignees
No one assigned
Labels
BC break Type: Introduces a backwards-incompatible break Bug fix Type: Bug fix develop Branch Improvement Type: Improvement QA ✔️ Status: check done, code approved
Projects
PR Dashboard
Archived in project
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

Migrate TokenAuthenticator to the new SF system
7 participants
@M0rgan01 @jolelievre @Progi1984 @matthieu-rolland @ps-jarvis @florine2623 @prestonBot