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] Add a Not_Full_Fledged_handler in ExceptionListener from security login#57661

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

Open
eltharin wants to merge8 commits intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromeltharin:notfullfletchedHandler

Conversation

eltharin
Copy link
Contributor

@eltharineltharin commentedJul 5, 2024
edited
Loading

QA
Branch?7.2
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#54318
LicenseMIT

My proposal for behaviour for acces Denied When User is logged with a remeberme token

Without set anything, the behaviour is the same as actually.
In the Firewall config adding a not_full_fledged_handler waiting a service handler or a predefined value :

  • predefined values :
    • "original" "redirect" for the same behavior than actual replaced bySymfony\Component\Security\Http\Authorization\NotFullFledgedRedirectToStartAuthenticationHandler
    • "same" "equal" for if a user is logged but not not full fledged (and the attribute is not IS_AUTHENTICATED_FULLY) he has the same acces than if he was full logged. this value is replaced bySymfony\Component\Security\Http\Authorization\NotFullFledgedEqualNormalLoginHandler
  • user can create his own service too, class have to implementsSymfony\Component\Security\Http\Authorization\NotFullFledgedHandlerInterface with one method "handle"
    this method can return a reponse to change original Exception Response or null for continue to throw the original access denied exception.

@eltharineltharin requested a review fromchalasr as acode ownerJuly 5, 2024 14:34
@carsonbotcarsonbot added this to the7.2 milestoneJul 5, 2024
@eltharineltharin changed the titleNotfullfletched handlerAdd a Not Full Fletched handler in Security configJul 5, 2024
@eltharineltharinforce-pushed thenotfullfletchedHandler branch 3 times, most recently from64b0bdd to7e36014CompareJuly 6, 2024 16:17
@carsonbotcarsonbot changed the titleAdd a Not Full Fletched handler in Security config[Security] Add a Not Full Fletched handler in Security configJul 8, 2024
@eltharineltharinforce-pushed thenotfullfletchedHandler branch from7e36014 toa7ace88CompareJuly 8, 2024 10:12
@eltharineltharin marked this pull request as draftJuly 9, 2024 11:30
@eltharineltharin marked this pull request as ready for reviewJuly 9, 2024 13:17
@eltharineltharinforce-pushed thenotfullfletchedHandler branch 3 times, most recently frombdb9552 tof64b805CompareJuly 11, 2024 11:05
@eltharineltharin changed the title[Security] Add a Not Full Fletched handler in Security config[Security] Add a Not_Full_Fledged_handler in ExceptionListener from security loginJul 11, 2024
@eltharineltharinforce-pushed thenotfullfletchedHandler branch 4 times, most recently froma5ff172 to05ca8ddCompareJuly 26, 2024 11:36
Copy link
Member

@stofstof left a comment

Choose a reason for hiding this comment

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

I'm not convinced by this PR. To me, it does not actually solve the issue it claims to solve (it solves only a subset of cases, bringing aninvalid behavior for the other cases).
So for now, it gets a -1 vote from me.

}

foreach($accessDeniedException->getAttributes() as $attribute) {
if(in_array($attribute, [AuthenticatedVoter::IS_AUTHENTICATED_FULLY])) {
Copy link
Member

Choose a reason for hiding this comment

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

This is not enough to identify all cases where an access denied exception might depend on the trust level of a token (see my comment on the issue).

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

it totally a case whitch can be set in a custom handler.
SameAsNotFullFledgedHandler is here to anwser to many person, when PR will be merged (yes I beleive on it) if many persons have the same idea it can be possible to propose another PR for add core handlers

@eltharin
Copy link
ContributorAuthor

I don't undestand why you say it does not actually solve the issue. the problem is to obtain a control of AccessDenied Exception when RememberMe login is active, this PR allow a custom handler to choose:

  • original beahviour,
  • behaviour asked by many person : treat remeberme as normal login except if attribute is IS_AUTHENTICATED_FULLY
  • and if user want something specific he can make his own handler to get what he want.

I don't see whitch case is not cover ?

@stof
Copy link
Member

stof commentedAug 6, 2024

@eltharin have you read my comment on the issue ?

@eltharin
Copy link
ContributorAuthor

I do but I don't understand where you find a problem,
You seem to think that I want the new handler that I provide to do the job of all the projects even the most difficult, but I just provide a solution to simple requests, the biggest ones are to be handled by the developers themselves with the customs.

@eltharineltharinforce-pushed thenotfullfletchedHandler branch 2 times, most recently from818ff65 to91dc0c2CompareAugust 13, 2024 21:17
@eltharineltharin requested a review fromstofSeptember 2, 2024 10:11
@eltharin
Copy link
ContributorAuthor

What can we do to advance this PR ?

@fabpotfabpot modified the milestones:7.2,7.3Nov 20, 2024
@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@stofstofAwaiting requested review from stof

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

Assignees
No one assigned
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

Improve behaviour for Access Denied with Remember Me
6 participants
@eltharin@stof@nicolas-grekas@OskarStark@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp