-
Notifications
You must be signed in to change notification settings - Fork 14
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add self-service Bootstrap OTP generation. #68
Add self-service Bootstrap OTP generation. #68
Conversation
cmd/keymasterd/2fa_bootstrapOTP.go
Outdated
state.logger.Printf("error generating Bootstrap OTP: %s", err) | ||
return | ||
} | ||
duration := time.Minute * 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this a const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -111,6 +111,44 @@ func (state *RuntimeState) BootstrapOtpAuthHandler(w http.ResponseWriter, | |||
} | |||
} | |||
|
|||
func (state *RuntimeState) trySelfServiceGenerateBootstrapOTP(username string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont like the signature of this function, it should return a bool if successful, otherwise we cannot distingusish sucess from failure... and thus as it is right now there is a race condition where the otp generated but fails to be added to the DB.
Thus on the caller you can do
selfServiceOtpGenerated := state.trySelfServiceGenerateBootstrapOTP.....
I really dislike logic where updates to object are not explicitly communicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the function signature. The caller hasn't changed because there isn't really anything it would do differently regardless of the return status.
No description provided.