Bugfix for expired trusted browser #451
Open
Conversation
This file contains 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
pthanos
reviewed
Oct 24, 2024
| @@ -122,7 +122,7 @@ public async Task<MfaLoginViewModel> BuildMfaLoginViewModelAsync(string returnUr | |||
| var isExistingBrowser = false; | |||
| if (!string.IsNullOrWhiteSpace(deviceIdentifier.Value)) { | |||
| var browserDevice = await _userManager.GetDeviceByIdAsync(user, deviceIdentifier.Value); | |||
| isExistingBrowser = browserDevice is not null; | |||
| isExistingBrowser = browserDevice is not null && browserDevice.MfaSessionExpirationDate > DateTimeOffset.UtcNow; | |||
There was a problem hiding this comment.
θα έλεγα οτι πρέπει να πάει στην GetDeviceById()
There was a problem hiding this comment.
να σου πω την αληθεια μου φαινεται πιο λογικο να μεινει εκει που ειναι, γιατι το method ειναι GetDeviceById, οχι GetTrustedDeviceById ή κατι τετοιο...
There was a problem hiding this comment.
επισης ας ένα isValid ή isExpired helper πάνω στο device class. Ας πει τι θέλει ο @cleftheris
pthanos
requested changes
Oct 24, 2024
|
Καταρχάς πρόκειται για αλλαγή που δεν έχει parity με το identity server part (new part). Οπότε δε μπορώ να τη δεχτώ. Δευτέρον δε καταλαβαίνω το scenario αν δε μου το εξηγήσει κάποιος σε βάθος. Μέχρι να το δώ να κάνει reproduce και να καταλάβω το scenario δε μπορώ να το κλείσω. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
IsExistingBrowsershould be false if the current browser's identifier is expired, by date-comparing usingMfaSessionExpirationDate.