Skip to content

Add plugin for Nimble package manager - #255

Open
electrocally wants to merge 4 commits into
1Password:mainfrom
electrocally:add/nimble
Open

electrocally wants to merge 4 commits into
1Password:mainfrom
electrocally:add/nimble

Conversation

@electrocally

Copy link
Copy Markdown

Overview

This plugin adds support for https://nimble.directory, the package manager for https://nim-lang.org

Type of change

  • Created a new plugin

How To Test

  1. Install Nim, this will also install nimble
  2. Install this plugin using these instructions with make nimble/build & op plugin init nimble. This may prompt for your PAT, explained in step 4
  3. Run nimble init to create a new Nim package (additional docs here)
  4. When executing nimble publish, 1Password will prompt for your GitHub Personal Access Token. This will need public_repo permissions.

Changelog

Authenticate the publishing Nim (Nimble) packages using Touch ID and other unlock options with 1Password Shell Plugins.

@AndyTitu AndyTitu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the contribution! Left a few comments. Let me know if you need additional guidance!

Comment thread plugins/nimble/personal_access_token.go Outdated
}

func TryNimbleConfigFile() sdk.Importer {
// TODO: Try importing from ~/.nimble/github_api_token file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this TODO a change to be carried on in this PR? If not, I would suggest adding an importer for a potentially already existing NIMBLE_GITHUB_API_TOKEN as a quick win, just so not to ship this with absolutely no importers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

importer.TryEnvVarPair(defaultEnvVarMapping),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks so much for the feedback & suggestions, @AndyTitu !
I'll add in the env var importer this evening.

For the file importer, this is similar to the HashiCorp Vault plugin. I wasn't able to create a working version as the ~/.nimble/github_api_token file doesn't have any formatting/extension type, just the token in plaintext.
I was hoping to ask for suggestions - I read through the other plugins to compare, but as far as I can see, the package seems to expect a key/value.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've pushed TryEnvVarPair to the top of the TryAll in importers. Cheers!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something like this should do the trick for the file importer, provided the file only contains the token:

func TryNimbleConfigFile(path string) sdk.Importer {
	return importer.TryFile(path, func(ctx context.Context, contents importer.FileContents, in sdk.ImportInput, out *sdk.ImportAttempt) {
		credential := contents.ToString()
		fields := make(map[sdk.FieldName]string)
		if len(credential) != 0 {
			fields[fieldname.Token] = credential
		}
		out.AddCandidate(sdk.ImportCandidate{
			Fields: fields,
		})
	})
}

I didn't test this code, let us know in case this doesn't work and we can look deeper into it 😄

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hey @hculea, thank you so much for your help on this! That seems to be working well & I've just tested that it can now import from that config file

Comment thread plugins/nimble/personal_access_token.go
Comment thread plugins/nimble/nimble.go
),
Uses: []schema.CredentialUsage{
{
Name: credname.PersonalAccessToken,

@AndyTitu AndyTitu May 1, 2023 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we go on the route mentioned in the other comment, here we could just specify Plugin: github, Name: credname.PersonalAccessToken and Provisioner: the provisioner you created.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm all for using the standardised plugin! The schema suggests this needs to be a string for the plugin, e.g. Plugin: "github"?
I can't seem to see other plugins using it for an example, so any help would be massively appreciated.

As I've not added a custom provisioner, would something along the lines of this be close?

...

Uses: []schema.CredentialUsage{
    {
      Plugin: "github",
      Name:   credname.PersonalAccessToken,
    },
},

Thanks in advance

@AndyTitu AndyTitu May 8, 2023 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@electrocally Yes, this is right!

The only extra thing is that you need to define a custom provisioner for this credential usage, as you noted in this other thread's reply.

So this should work:

Uses: []schema.CredentialUsage{
    {
      Plugin: "github",
      Name:   credname.PersonalAccessToken,
      Provisioner: provision.EnvVars(nimbleDefaultEnvVarMapping),
    },
},

...

var nimbleDefaultEnvVarMapping = map[string]sdk.FieldName{
	"NIMBLE_GITHUB_API_TOKEN": fieldname.Token,
}

while everything else in nimble/personal_access_token.go can be safely removed.

You could then remove the provisioner test case entirely or modify it to test this custom provisioner provision.EnvVars(nimbleDefaultEnvVarMapping)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's add the provisioner here in the credential usage, as we discussed before

@AndyTitu AndyTitu added the in-progress this PR is being worked on/comments are in the process of being addressed by the contributor label May 1, 2023
@AndyTitu AndyTitu added waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member and removed in-progress this PR is being worked on/comments are in the process of being addressed by the contributor labels May 4, 2023

@hculea hculea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution! ❤️ I've only left a few comments, looks great for the most part.

Comment thread plugins/nimble/nimble.go
Runs: []string{"nimble"},
DocsURL: sdk.URL("https://github.com/nim-lang/nimble"),
NeedsAuth: needsauth.IfAll(
needsauth.IfAny(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for IfAny in this situation, since it only contains one rule.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This has yet to be resolved

Comment thread plugins/nimble/personal_access_token.go Outdated
}

func TryNimbleConfigFile() sdk.Importer {
// TODO: Try importing from ~/.nimble/github_api_token file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something like this should do the trick for the file importer, provided the file only contains the token:

func TryNimbleConfigFile(path string) sdk.Importer {
	return importer.TryFile(path, func(ctx context.Context, contents importer.FileContents, in sdk.ImportInput, out *sdk.ImportAttempt) {
		credential := contents.ToString()
		fields := make(map[sdk.FieldName]string)
		if len(credential) != 0 {
			fields[fieldname.Token] = credential
		}
		out.AddCandidate(sdk.ImportCandidate{
			Fields: fields,
		})
	})
}

I didn't test this code, let us know in case this doesn't work and we can look deeper into it 😄

Comment thread plugins/nimble/personal_access_token.go Outdated
Comment on lines +50 to +52
type Config struct {
Token string `ini:"token"`
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can get rid of this, I think.

Comment on lines +26 to +36
func TestPersonalAccessTokenImporter(t *testing.T) {
plugintest.TestImporter(t, PersonalAccessToken().Importer, map[string]plugintest.ImportCase{
"environment": {
Environment: map[string]string{
"NIMBLE_GITHUB_API_TOKEN": "ghp_ahhfw82h48fh72nfn29fn291nwidhf8EXAMPLE",
},
ExpectedCandidates: []sdk.ImportCandidate{
{
Fields: map[sdk.FieldName]string{
fieldname.Token: "ghp_ahhfw82h48fh72nfn29fn291nwidhf8EXAMPLE",
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's make sure we also add a test for the file importer, once that's sorted out.

@AndyTitu AndyTitu added in-progress this PR is being worked on/comments are in the process of being addressed by the contributor and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels May 8, 2023
@AndyTitu

Copy link
Copy Markdown
Contributor

@electrocally Are there any further blockers you would need some help with, following our developer Slack conversation? Let me know 😄 !

@electrocally

Copy link
Copy Markdown
Author

@electrocally Are there any further blockers you would need some help with, following our developer Slack conversation? Let me know 😄 !

Thanks for checking in @AndyTitu! I've not had a lot of time for my projects lately. Hoping to do a bit more on this over the coming evenings 😁

@hculea

hculea commented May 30, 2023

Copy link
Copy Markdown
Member

@electrocally Do let us know if there is anything that we can help with, to keep this moving 😄

@electrocally

Copy link
Copy Markdown
Author

Hi guys! Thanks so much for the continued encouragement the last few weeks. It's been really great to contribute to this project 😀

I've just pushed up some changes - there's good and bad news.
The good: it works! I've tidied up some of the code but as always would love to hear any further suggestions/optimisations.

The not so good: I love the concept of using the Plugin: github setup & I've really appreciated all your help with it. It works brilliantly to provision credentials and can be reused across all sorts of apps that rely on GitHub.
That said, as we discussed on Slack, until we can add in an Importer argument to the CredentialUsage type, I can't seem to find a way to get that configuration working with importer(s).
For now I'd like to prioritise the onboarding process of the plugin to anyone using the default Nimble setup & would be more than happy to contribute to adding that functionality to the type definition

@@ -0,0 +1 @@
ghp_ahhfw82h48fh72nfn29fn291nwidhf8EXAMPLE No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: add an extra line

@AndyTitu

Copy link
Copy Markdown
Contributor

@electrocally I'm fine with not attempting to change the schema to adhere to the importing scenario in this MR. Let's make sure we create a separate issue for that though, if one doesn't exist already.

But there are still a few open discussions to be resolved before we could merge this PR

@rr3khan rr3khan closed this Oct 12, 2026
@rr3khan rr3khan reopened this Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants