Charm pam module #15
Merged
Charm pam module #15
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
We move from a purely declarative and linear approach to a more modular react-like (elm) model. Those models are nested, and can display some UIs. Transitioning between models are done via an event based system Co-authored-by: Jean-Baptiste Lallement <[email protected]>
The button now reselect the authentication method to triggers new sms send and other side effect. For elements with wait: true, then, IsAuthorized is cancelled, UI is recreated and IsAuthorized() is called again. Migrate any potential text input fields while the layout is redrawn. Co-authored-by: Jean-Baptiste Lallement <[email protected]>
As we can build it, ensure we don’t commit it by mistake. Co-authored-by: Jean-Baptiste Lallement <[email protected]>
This avoids any potential segfaults when trying to draw the view. Co-authored-by: Jean-Baptiste Lallement <[email protected]>
Keep a TODO to maybe remove it by using the list selection item index.
We were returning the struct content beforehand.
Any other elements would be a programming error, hence panicing as this should be caught by tests.
Make the focused button readable.
Now that the CLI supports it, it’s interesting to set it.
Consequence select call will change temporary code or qr code. This is to simulate and demonstrate sms resends and so on.
Co-authored-by: Jean-Baptiste Lallement <[email protected]>
denisonbarbosa
approved these changes
Aug 10, 2023
pam/authorization.go
Outdated
| if err != nil { | ||
| if st := status.Convert(err); st.Code() == codes.Canceled { | ||
| return isAuthorizedResultReceived{ | ||
| access: "cancelled", |
There was a problem hiding this comment.
Isn't it better to use responses.AuthCancelled instead? Otherwise, it might break if we decide to switch the access message.
Didn't add a suggestion because it would require import fixes as well.
There was a problem hiding this comment.
We can’t here, because as we cancel the context, there is no access to the "access" field (the struct is nil). However, I agree, we can resend responses.AuthCancelled.
Will change that.
pam/authorization.go
Outdated
Comment on lines
113
to
130
| case "allowed": | ||
| return *m, sendEvent(pamSuccess{}) | ||
|
|
||
| case "retry": | ||
| m.errorMsg = dataToMsg(msg.data) | ||
| return *m, sendEvent(startAuthorization{}) | ||
|
|
||
| case "denied": | ||
| errMsg := "Access denied" | ||
| if err := dataToMsg(msg.data); err != "" { | ||
| errMsg = err | ||
| } | ||
| return *m, sendEvent(pamAuthError{msg: errMsg}) | ||
|
|
||
| case "next": | ||
| return *m, sendEvent(GetAuthenticationModesRequested{}) | ||
|
|
||
| case "cancelled": |
There was a problem hiding this comment.
Same comment on the access values here
|
Just pushed 2 extra commits with the new response types and plugging them into PAM. Want to have another look? |
denisonbarbosa
approved these changes
Aug 10, 2023
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.
We move from a purely declarative and linear approach to a more modular
react-like (elm) model.
Those models are nested, and can display some UIs.
Transitioning between models are done via an event based system