Feature: add support to read from netrc

[adolsalamanca]

Description

This PR includes a tiny new feature to support reading configs from .netrc file as suggested in this issue

Changes

• Create a .envrc reader under /pkg, credits to the folks from this link
• Add support to jira-cli so it verifies .envrc config and also make jira calls rely on it in case it's valid, as a fallback to the token in JIRA_API_TOKEN env variable.
• Update checkForJiraToken function signature in order to check both possibilities now.
• Update README with new alternative use.

Checks

• Envrc reader has unit tests.
• New feature is tested and working.

Let me know or feel free to edit the code if there's something that doesn't fit in terms of structure or approach :)

[ankitpokhrel]

Hi @adolsalamanca, Thank you for working on this 🙌🏻

Since the tool is getting some attention recently and lot of devs relying on the project, I think it would be better to stick with the standard .netrc protocol instead of reinventing the wheel. Lot's of CLI tool supports .netrc and I am not sure why we would need envrc instead of relying on .netrc file. Maybe there is a usecase that I am missing, let me know if that's the case.

As you mentioned in the description, golang already implements netrc protocol. We can borrow the package (with credit) and use it for our token validation. We will have one less thing to maintain if we stick with this.

[adolsalamanca]

Omg @ankitpokhrel, my bad, this was a typo.
It's actually implementing .netrc standard, just mixed up things with package name, branch name and also PR (will fix it now).
In fact it was already relying on netrc.go files, but I was referencing it wrongly.

The thing is that I decided to slightly modify their implementation because they were just relying on global vars, but maybe I can try to make use of the original implementation with a few tweaks.
However as they implement it in their internals and do not expose things to the outside I think we're "forced" to pick the code.

[adolsalamanca]

Thanks for responding so fast mate!! @ankitpokhrel
Just borrowed the original implementation as it was from Go internals, without any customizations like before.

Only thing added was the function ReadNetrcPassword inside, not sure if could be better to extract to another package/file so we remain unchanged netrc files read implementation.

The only problem is that Deepsource is now complaining due to case "default" redundant inside netrc reader.
Let me know what you think about it.

Best regards

[ankitpokhrel]

Hi @adolsalamanca,

Overall this is getting in shape. There are few things we should keep in mind tho.

1. I believe machine name is unique in each .netrc file. If there are duplicates first entry is the valid one. These details are usually used for auto-login process and as per the spec, auto-login can be initiated as soon as the first entry for the machine is found.
2. When the user enters server name during jira init process, we need to make sure not to ask for the username/email if it is present in .netrc file. We can do this in a different PR if this one is getting big.

Let me know if you need any help and thank you again for working on this.

[ankitpokhrel]

netrc has nothing to do with Jira and netrc package should not care anything about it at all.

I would suggest to keep reader and reader_test sync with the original implementation including license information (required as per BSD).

You can have a new file called netrc.go in the package that will expose required methods to work with netrc package.

// Package netrc implements GNU .netrc specification.
// This implementation is borrowed from the original implementation by the go authors.
// See https://github.com/golang/go/blob/master/src/cmd/go/internal/auth/netrc.go
// See https://www.gnu.org/software/inetutils/manual/html_node/The-_002enetrc-file.html
package netrc

import (
	"fmt"
)

// ErrNetRCEntryNotFound is thrown if details for the machine is not found.
var ErrNetRCEntryNotFound = fmt.Errorf("netrc: entry not found in config")

// Entry is a netrc config entry.
type Entry struct {
	Machine  string
	Login    string
	Password string
}

// Read reads config for the given machine.
func Read(machine string) (*Entry, error) {
	netrcOnce.Do(readNetrc)
	if netrcErr != nil {
		return nil, fmt.Errorf("netrc: %w", netrcErr)
	}

	for _, line := range netrc {
		if line.machine == machine {
			return &Entry{
				Machine:  line.machine,
				Login:    line.login,
				Password: line.password,
			}, nil
		}
	}

	return nil, ErrNetRCEntryNotFound
}

Now, you can use the exposed method elsewhere.

netrcEntry, err := netrc.Read(config.Server)

Please feel free to update above implementation as necessary.

[ankitpokhrel]

The only problem is that Deepsource is now complaining due to case "default" redundant inside netrc reader.
Let me know what you think about it.

Regarding this, don't worry about deepsource complaint in third-party or borrowed files. Not all issues reported by deepsource are crucial.

[adolsalamanca]

Thanks for the reviews mate!

netrc has nothing to do with Jira and netrc package should not care anything about it at all.

This is that happens when you refactor in automatic mode, but the original intention was completely separate responsibilities, that's why I created netrc in pkg folder. Good catch!

Will apply suggestions soon, have a good start of the week :)

[adolsalamanca]

Hey @ankitpokhrel
Thanks for your suggestions, I applied most of them.
However, I decided to keep login as a parameter for the reader, I know the chances to have two accounts for the same machine are pretty reduced, but this way it is mandatory to have the .netrc properly configured.

Still, there's something I would like to improve, when the JIRA_API_TOKEN is missing, and the .netrc config is not valid, the 403 could be a bit ambiguous, but we can iterate to improve that.

Thanks!!

[ankitpokhrel]

Hi @adolsalamanca, this seem to be working fine so merging this. Thank you 🙇🏻.

Just a heads up, I might do some changes for better user experience.