-
Notifications
You must be signed in to change notification settings - Fork 15
feature: list, add and revoke authorizations #430
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
Conversation
Users can now list, add and revoke permissions with the new `authorizations` subcommand.
|
Failed to retrieve llama text: POST 500: Traceback (most recent call last): The above exception was the direct cause of the following exception: Traceback (most recent call last): |
1yam
left a comment
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.
LGTM
|
|
||
|
|
||
| @app.command() | ||
| async def list( |
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.
Previously we had :
| async def permissions( |
We might want to deprecate it ?
|
|
||
|
|
||
| @app.command() | ||
| async def add( |
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.
same for:
| async def authorize( |
|
|
||
|
|
||
| @app.command() | ||
| async def revoke( |
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.
same for:
| async def revoke( |
| account: AccountTypes = load_account( | ||
| private_key_str=private_key, private_key_file=private_key_file, chain=chain | ||
| ) | ||
| address = account.get_address() |
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.
For UX reasons (ledger), when we do not need to sign any messages, we should use :
aleph-client/src/aleph_client/utils.py
Line 323 in 4cd758f
| def get_account_and_address( |
When using load_account, the Ledger must be unlocked. Using get_account_and_address, if the account is properly configured, avoids requiring the user to unlock the Ledger just to fetch their authorizations.
Users can now list, add and revoke permissions with the new
authorizationssubcommand.Explain what problem this PR is resolving
Related ClickUp, GitHub or Jira tickets : ALEPH-XXX
Self proofreading checklist
Documentation
The documentation regarding the impacted features is available on:
The changes in the documentation are available here:
Changes
Explain the changes that were made. The idea is not to list exhaustively all the changes made (GitHub already provides a full diff), but to help the reviewers better understand:
How to test
Explain how to test your PR.
If a specific config is required explain it here (account, data entry, ...)
Print screen / video
Upload here screenshots or videos showing the changes if relevant.
Notes
Things that the reviewers should know: known bugs that are out of the scope of the PR, other trade-offs that were made.
If the PR depends on a PR in another repo, or merges into another PR (i.o. main), it should also be mentioned here