Skip to content
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

Implementing the appwrite plugin #336

Closed
wants to merge 5 commits into from
Closed

Conversation

ra-jeev
Copy link

@ra-jeev ra-jeev commented Jun 30, 2023

Overview

Added Appwrite CLI shell plugin.

Since Appwrite supports an interactive CLI login using Email & Password, we can't use that approach with OP yet. Appwrite also supports a CI mode with a limitation that it only works with one project at a time, and we need to generate a project specific API_KEY with necessary permissions to do so.

Commands requiring access to more than one project (appwrite projects ...), which is supported by the normal CLI won't work in the CI mode.

The best approach is to configure different OP items for each project and using these items from the specific project dir.

Type of change

  • Created a new plugin
  • Improved an existing plugin
  • Fixed a bug in an existing plugin
  • Improved contributor utilities or experience

Related Issue(s)

  • Resolves: #
  • Relates: #

How To Test

After setting the plugin run the following command:
appwrite client --debug

We can also run any of the other commands without a subcommand (e.g. appwrite client, appwrite functions etc. as it only displays a list of available subcommands).

For running project specific commands we need to run the appwrite commands from the project folder containing the appwrite.json file.

Changelog

Adds the appwrite cli shell plugin to 1password CLI

Additional information

Running the appwrite commands from the HOME dir

Screen Shot 2023-06-30 at 6 07 00 PM

Running the appwrite commands from the project dir

Screen Shot 2023-06-30 at 6 08 16 PM

@techcraver
Copy link
Contributor

Hello! Don't forget to also write an accompanying blog post on Hashnode with the tags 1Password and BuildWith1Password (not just putting #1Password in the text - but use the Hashnode tags in the CMS. :)

The full instructions are here.

@ra-jeev
Copy link
Author

ra-jeev commented Jun 30, 2023

Hello! Don't forget to also write an accompanying blog post on Hashnode with the tags 1Password and BuildWith1Password (not just putting #1Password in the text - but use the Hashnode tags in the CMS. :)

The full instructions are here.

I'm starting now. Had some other work to do. This will be a long one as it has passage in it as well 😄.

@arunsathiya arunsathiya added waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member hashnode hackathon Ideas and inspiration for the hackathon running from June 1st - June 30th labels Jul 3, 2023
Copy link
Contributor

@arunsathiya arunsathiya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good start, thank you for your contribution @ra-jeev!

A couple of things to move forward:

  • I'd like some clarity on the three files that Appwrite supports: appwrite.json, prefs.json, import_prefs.json
  • Is there a reason why we are not storing the project ID as a 1Password field? Or is that because project ID is stored in appwrite.json file of that project folder? In other words, do I understand correctly that a combination of appwrite.json file and the API key stored in envvar API_KEY provide authentication? 🤔

plugins/appwrite/api_key.go Outdated Show resolved Hide resolved
plugins/appwrite/appwrite.go Outdated Show resolved Hide resolved
plugins/appwrite/test-fixtures/import_prefs.json Outdated Show resolved Hide resolved
plugins/appwrite/test-fixtures/import_prefs.json Outdated Show resolved Hide resolved
@ra-jeev
Copy link
Author

ra-jeev commented Jul 3, 2023

@arunsathiya I've replied to all of the comments. And you've correctly guessed the reason for not storing the project ID in the prefs.json file. The project ID is stored in the project specific folder. The global config only stores the server endpoint, api key and cookie (in case of normal email / password login scenario).

Will be happy to provide more details. Please do check the article linked in one of the comments for thorough explanation (if needed).

Thanks,
Rajeev

@ra-jeev ra-jeev removed their assignment Jul 4, 2023
@hculea hculea requested a review from arunsathiya July 5, 2023 07:32
@hculea hculea mentioned this pull request Jul 5, 2023
4 tasks
Copy link
Contributor

@arunsathiya arunsathiya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to approve this with the understanding that projects will be switched to NotWhenContainsArgs. Besides that, everything looks good to me and functional testing is done too. Thank you for your contribution, @ra-jeev!

@hculea / @AndyTitu - may I ask one of you to take a look at the code once, before we switch this PR over to waiting-on-sec-review? I have done a functional testing already, so don't feel compelled to do it again:

➜  ~ source ~/.op/plugins.sh
➜  ~ which appwrite
appwrite: aliased to op plugin run -- appwrite
➜  ~ appwrite storage listBuckets
#########################################################################
# WARNING: 'appwrite' is not from the official registry.                #
# Only proceed if you are the developer of 'appwrite'.                  #
# Otherwise, delete the file at /Users/arun/.op/plugins/local/appwrite. #
#########################################################################
total : 0
buckets
[]
✓ Success
➜  ~ appwrite functions list
#########################################################################
# WARNING: 'appwrite' is not from the official registry.                #
# Only proceed if you are the developer of 'appwrite'.                  #
# Otherwise, delete the file at /Users/arun/.op/plugins/local/appwrite. #
#########################################################################
total : 0
functions
[]
✓ Success
➜  ~ appwrite databases list
#########################################################################
# WARNING: 'appwrite' is not from the official registry.                #
# Only proceed if you are the developer of 'appwrite'.                  #
# Otherwise, delete the file at /Users/arun/.op/plugins/local/appwrite. #
#########################################################################
total : 0
databases
[]
✓ Success

plugins/appwrite/appwrite.go Outdated Show resolved Hide resolved
plugins/appwrite/appwrite.go Outdated Show resolved Hide resolved
@ra-jeev
Copy link
Author

ra-jeev commented Jul 7, 2023

I've pushed the requested change. Please let me know if any other information is needed.

Thanks

Copy link
Contributor

@arunsathiya arunsathiya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ra-jeev, this looks good to me now! 🚀

Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tag @arunsathiya .

Only have a couple remarks.

plugins/appwrite/api_key.go Outdated Show resolved Hide resolved
plugins/appwrite/api_key.go Outdated Show resolved Hide resolved
plugins/appwrite/api_key.go Outdated Show resolved Hide resolved
Comment on lines +24 to +27
needsauth.NotForExactArgs("teams"),
needsauth.NotForExactArgs("users"),
needsauth.NotForExactArgs("account"),
needsauth.NotForExactArgs("avatars"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These looks like commands that would be scoped to a specific account.

Is there any documentation you could point me to about these commands? I wonder why they would not require auth.

Copy link
Author

@ra-jeev ra-jeev Jul 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hculea without any argument these commands behave as if they were called with --help argument. So they just give you the list of available subcommands and options. Also, these are project specific commands. So if you run appwrite users list from the project folder (this requires auth, and can only be run from project folder which contains the appwrite.json config file), you'll get the list of all users of that project.

Here is the CLI documentation (they don't say the above in the docs, but all the examples given there are with arguments). I think @arunsathiya has verified this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my functional testing: The project token that we provision seems to work for users list and teams list. I'd imagine that it works for the rest of the subcommands for users and teams.

However, account get (one of the many subcommands under account) does not work with the project token, rather requires account scope. I think that's expecting a combination of username and password?

➜  ~ appwrite account get
#########################################################################
# WARNING: 'appwrite' is not from the official registry.                #
# Only proceed if you are the developer of 'appwrite'.                  #
# Otherwise, delete the file at /Users/arun/.op/plugins/local/appwrite. #
#########################################################################
✗ Error app.6495df658da7c618cab3@service.cloud.appwrite.io (role: applications) missing scope (account)

Not too sure how avatars' subcommands work though.

without any argument these commands behave as if they were called with --help argument

This matches my testing too. 👍🏼

Copy link
Author

@ra-jeev ra-jeev Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

account commands work with currently logged in user (requires a session). users is a more appropriate command from server's perspective. account is for impersonating a user and acting in their behalf using their JWT token.

avatars offers below subcommands:

getBrowser
getCreditCard
getFavicon
getFlag
getImage
getInitials
getQR

These are some utility functions for getting images/icons for stuffs. e.g. getBrowser is for getting browser icon, getFlag for getting a country's flag icon etc.

Marking this conversation unresolved for now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hculea Please let me know if I need to change anything, or provide more info.

Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if account doesn't work with the API KEY we provision anyway (it only works with a JWT), we should make sure we also exclude this command from out auth scheme as well: needsauth.NotWhenContainsArgs("account"),.

As a bit of a background, where does that JWT come from? We should make sure this doesn't get placed somewhere on disk in plain-text.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndyTitu The JWT comes when you use a Login/Password to login to the CLI (which we can't use with 1Passsword as this is an interactive process). Also, in this flow the JWT actually gets written to the same config file (~/.appwrite/prefs.json) on disk, again there won't be any JWT if we're using the CI mode (which we're doing with this integration).

You're right that the account commands don't work with an API KEY. Please let me know if I need to make this change needsauth.NotWhenContainsArgs("account").

Thanks,
Rajeev

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that change makes sense.

Sorry for the delayed reply!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem @accraw. Don't think this will be merged so it is okay. :-)

@AndyTitu AndyTitu added question Further information is requested and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels Aug 30, 2023
@accraw accraw added in-progress this PR is being worked on/comments are in the process of being addressed by the contributor and removed question Further information is requested labels Oct 18, 2023
@ra-jeev ra-jeev closed this Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashnode hackathon Ideas and inspiration for the hackathon running from June 1st - June 30th in-progress this PR is being worked on/comments are in the process of being addressed by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants