-
Notifications
You must be signed in to change notification settings - Fork 140
Fix: AuthControlled faucet leaves Authority setters unauthenticated #2958
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
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
2a93cf1
merge AuthMethod into AccessControl
onurinanc 8b2527b
changelog
onurinanc 039ced3
fix documentation
onurinanc 2a182da
merge next
onurinanc 9c683be
fix
onurinanc 3c775d2
changelog
onurinanc c9d72ea
Merge branch 'next' into fix-h1-unauthenticated-authority
onurinanc 1402b33
fix comments
onurinanc 75cfa46
Merge remote-tracking branch 'origin/next' into fix-h1-unauthenticate…
onurinanc d6a9220
Merge remote-tracking branch 'origin/fix-h1-unauthenticated-authority…
onurinanc f7ae47d
Merge branch 'next' into fix-h1-unauthenticated-authority
bobbinth 5736d2c
add build_auth_component
onurinanc 9a02599
Merge remote-tracking branch 'origin/next' into fix-h1-unauthenticate…
onurinanc 887143c
Merge remote-tracking branch 'origin/fix-h1-unauthenticated-authority…
onurinanc f5066ef
fix invalid combination
onurinanc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Not for this PR, but I think a safer design for AuthSinglesigAcl might be something similar as what we recently did for AuthMultisig, where if a procedure is called that is not configured, the threshold is set to at least default_threshold.
Similarly for the ACL, we could flip the current logic and say that only configured procedures are exempt from a signature. This is the safer default. E.g. you could say
receive_assetcan run without signature, but anything else requires a signature.It would also make the configuration less cumbersome, e.g. here we need to carefully list all procedures exhaustively and if we forget just one, we might have a security hole.
I'm not yet sure how to marry this concept together with
with_allow_unauthorized_input_notesand thewith_allow_unauthorized_output_notes, but maybe we'd have to remove this and require users to be more explicit (e.g. explicitly configurereceive_and_burnas a no-signature procedure).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 think we can resolve this as described in the second approach here: #2964 (comment)