Uh oh!
There was an error while loading. Please reload this page.
Satisfy @PasswordConfirmationRequired when authenticating with Bearer Authorization header - #7501
Closed
DamnDam wants to merge 2 commits into
Closed
Satisfy @PasswordConfirmationRequired when authenticating with Bearer Authorization header#7501DamnDam wants to merge 2 commits into
DamnDam wants to merge 2 commits into
Conversation
… Authorisation header
DamnDam
commented
Dec 14, 2017
Author
This is relevant for OAuth2 authentication, especially as an admin @LukasReschke |
DamnDam
commented
Dec 14, 2017
Author
This should fix the same issue as #7487 |
| if(!$this->validateToken($token)) { | ||
| return false; | ||
| } | ||
| $this->session->set('last-password-confirm', $this->timeFactory->getTime()); |
There was a problem hiding this comment.
Looks like this line is also hit for simple browser sessions (line 737 and below) and thus completely disables the password confirmation feature.
Codecov Report
@@ Coverage Diff @@## master #7501 +/- ##
============================================
+ Coverage 51.1% 51.11% +<.01%
Complexity 24903 24903 ============================================
Files 1601 1601 Lines 94779 94780 +1 Branches 1368 1368 ============================================
+ Hits 48439 48443 +4 + Misses 46340 46337 -3
|
ChristophWurst
requested changes
Jan 5, 2018
| return false; | ||
| } | ||
| if($loginWithHeader) { |
There was a problem hiding this comment.
this variable is not set on all previous code paths. it should be initialized with false at the beginning of the method body.
rullzer
commented
Jan 18, 2018
Member
Since #7487 is in. Lets close this. |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Issue:
When authenticated with an access token in a Bearer Authorization header in a stateless context, it is impossible to satisfy
@PasswordConfirmationRequiredroutes.Proposed solution:
When authenticating a Bearer Authorization Header, update the 'last-password-confirm' session variable, in the same way as a Basic Authorization.