Extend examplebroker with MFA, password resets and max attempts #23
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
denisonbarbosa
force-pushed
the
extend-example-broker
branch
from
bbfeaef
to
d29dc21
Compare
GabrielNagy
reviewed
Aug 28, 2023
denisonbarbosa
force-pushed
the
extend-example-broker
branch
from
d29dc21
to
4eaf8de
Compare
didrocks
requested changes
Aug 28, 2023
There was a problem hiding this comment.
Disclaimer: I didn’t test it live yet, I have some suggestions for both code maintainance improvements and avoid down the line being puzzled by "why does this user don’t ask me for reset because I didn’t choose password first" for instance.
Once those are resolve, I will move to some manual testing.
The global idea is here, just need a little bit of refinement IMHO :)
| "id": id, | ||
| "label": authMode["selection_label"], | ||
| }) | ||
| if authMode, exists := supportedModes["fidodevice1"]; exists && info.selectedMode != "fidodevice1" { |
There was a problem hiding this comment.
it seems those 3 could be in a for loop (which can be named allModeIDs), no? :)
denisonbarbosa
force-pushed
the
extend-example-broker
branch
from
4eaf8de
to
c7f4073
Compare
didrocks
approved these changes
Aug 30, 2023
We were using this a couple of times in the code, so I think it deserves its own function to check if there's an active session with the specified ID before updating it.
We used to do this inside the the GetAuthenticationModes func, but it will be easier to understand what is happening with MFA and Password Resets if this is in a separate function, specially since it involves a lot of map handling.
denisonbarbosa
force-pushed
the
extend-example-broker
branch
from
c7f4073
to
ce2e90d
Compare
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.
After some API refinements, we added support for password changes and multifactor authentication, but the examplebroker did not have those functionalities to show yet. This PR does some adjustments to the broker and adds those functionalities.
UDENG-1151