Add authentication methods to access private binary repository

[mollyIV]

Description

This pull request is introducing a support for accessing the binary dependencies form private repositories.

Testing

Manual testing

1. Add a private binary dependency to your Cartfile.
2. Run carthage update --platform ios --use-netrc.
3. Check if a private binary dependency has been downloaded.

Unit tests
The bunch of unit tests for Netrc implementation were implemented in NetrcSpec.swift.

Important ⚠️

The current implementation of .netrc parser does not support default, account and macdef parameters. Check the documentation to get more details about given parameters.

closes #1783

[hlineholm]

In order to test the parser I added a ~/.netrc with an Artifactory machine and used it like this: Add auth header if machine exists in .netrc.

[mollyIV]

👋 I am working on adding an option to update command useBinaryCredentialNetrc. The usage could look like:

carthage update platform iOS  --useBinaryCredentialNetrc

EDIT: However I am wondering, if we want to have this type of flag overall 🤔
@blender could you please share your thoughts? 🙏

If we do, is update command the one which shall expose this flag?

[hlineholm]

👋 I am working on adding an option to update command useBinaryCredentialNetrc. The usage could look like:

carthage update platform iOS  --useBinaryCredentialNetrc

In case you decide to use --useBinaryCredentialNetrc; it should be --use-binary-credential-netrc right?

[hasanPrestoq]

This capability would be very useful for me. It looks like it has been approved, and passed tests. Is there any possibility that it could be merged soon?

[mollyIV]

Hey @hasanPrestoq 👋

The PR hasn't been approved yet

however it is ready for another round - all the suggestions have been implemented 🙂

[nuno-vieira]

This would also be very useful to me. Will @mdiep and @blender re-review this anytime soon?

[RonanMcH]

We are also waiting on this. This would be a great help. @mollyIV great work!

[ScottPierce]

This would also be useful to my team.

[tmspzz]

Minor style changes.

Looks good to me.

[mollyIV]

Unfortunately the CI jobs have failed. I did a rebase on top of the master - did not help. Could you please help on that @tmspzz ? 🙏

[tmspzz]

This fixes it but it still needs a few approvals: #2823

[tmspzz]

As @jdhealy commented privately, it makes sense to add a CLI parameter to opt in to this since there are limitations on what is supported.

Something like --use-netrc maybe

Also please update from master to fix the CI 🙇

[mollyIV]

Hey @werner77 👋

Thank you for your questions.

You'll probably also want to apply the .netrc credentials for the urls specified in the binary definition, not only for the definition itself

That's a very good point 👍 Using a .netrc file for both private binary definitions and binaries themselves makes sense 🙇 Most probably the place in the code we need to add it 👇

Carthage/Source/CarthageKit/Project.swift

Lines 1009 to 1018 in c942285

	/// Downloads the binary only framework file. Sends the URL to each downloaded zip, after it has been moved to a
	/// less temporary location.
	private func downloadBinary(dependency: Dependency, version: SemanticVersion, url: URL) -> SignalProducer<URL, CarthageError> {
	let fileName = url.lastPathComponent
	let fileURL = fileURLToCachedBinaryDependency(dependency, version, fileName)
	if FileManager.default.fileExists(atPath: fileURL.path) {
	return SignalProducer(value: fileURL)
	} else {
	return URLSession.shared.reactive.download(with: URLRequest(url: url))

What about the outdated command? It makes sense for that command to also include the --use-netrc option.

I guess it does make sense 👍 @tmspzz could you please confirm that? 🙏

What about OAuth2 authentication (Authorization: Bearer xxxxxxxxxx)?

The curl sets an authentication header as Authorization: Basic xxxx.... The solution provided in this PR is heavily inspired by it.

Can you please describe how we could support a OAuth2 (Authorization: Bearer xxxxxxxxxx) authentication? 🤔 If we decided to support it, it would be great to tackle it in a separate issue.

🙇 🙇

[tmspzz]

What about the outdated command? It makes sense for that command to also include the --use-netrc option.

I guess it does make sense 👍 @tmspzz could you please confirm that? 🙏

Yep

[JUSTINMKAUFMAN]

Would be great to get this merged. @mollyIV has been graciously working on it for a while now and, in addition to the others who have spoken up, this would also be helpful to my team.

[JUSTINMKAUFMAN]

Everything looks good to me!

[liambutler-lawrence]

Awesome that this is already done. My team is trying to add Carthage support to a set of proprietary SDKs we distribute and not having a way to authenticate with Artifactory to pull the artifacts is a blocker for us. What needs to happen to get this merged?

[mollyIV]

Hey @liambutler-lawrence 👋

What needs to happen to get this merged?

I just implemented and pushed to the repo two missing parts:

• Use credentials from .netrc file for binaries download if needed
• Add use-netrc option to outdated command

Now the MR is ready for the final (hopefully 😉) review. CC: @tmspzz

Would be great though if anyone could help us to do some manual testing 🙏

Testing steps:

1. Add a private binary dependency to your Cartfile.
2. Run carthage update --platform ios --use-netrc
3. Check if a private binary dependency has been downloaded.

🙇 🙇

[liambutler-lawrence]

@mollyIV This works for our setup! (JSON spec file is on a public GitHub repo, but the artifact is on Artifactory). Had to merge master into your branch to get make install to work, which I've committed to my own fork: https://github.com/liambutler-lawrence/Carthage. We're going to use this fork until this PR is merged.

Really appreciate all your work on this :)

[mofarajmandi]

fwiw, just tested using @liambutler-lawrence and it worked as expected i.e. downloaded private binary frameworks using .netrc thanks @mollyIV and @liambutler-lawrence !

[liambutler-lawrence]

Bump again. Would love to get back to using the mainline Carthage- while the fork works great internally, we don't want to require that our clients use a fork of Carthage, so we're still using ugly workarounds (like IP whitelisting) to distribute our SDKs publicly.

[nuno-vieira]

@tmspzz It looks to me that we have everything that we need to get this merged? Anything more missing?

[DavidBrunow]

The indentation does not match the existing codebase throughout the changes -- Carthage uses tabs instead of spaces so you might need to change your Xcode settings and reformat.

[darrylbaylissav]

Hey,

We've kept an eye on this PR for a few months. excited to begin using the end result. We were wondering when to expect this to be merged, we're planning features based on it.

[KyleLeneau]

@mdiep @tmspzz What is the chances of getting this merged into the mainline? I used the NSOperations fork of Carthage that has this feature and it works exactly how I would want it to work. I can now securely store pre-built binaries that I manage in our companies Artifactory instance (including the JSON file) and reference the versions in a Cartfile for the team.

I would really like to see this MR / Feature in the mainline soon so I can have the team use avoid the step of getting the NSOperations fork. Can you provide an update on this?

[tmspzz]

LGTM