Private feed implementation

[tomasfil]

Summary of the changes (in less than 80 chars)

Implements private feeds resolves #142

Used by nuget:
NuGet Sources Add -Name "feed_name" -Source "http://localhost:50557/v3/index.json" -UserName "username" -Password "password"

Addresses #142 #147

[tomasfil]

Added code to resolve #147 partially. It does not restrict to given library, but it provides support for multiple apiKeys, so each team can have unique one and thus in need of change it is easier. Also it should not introduce any breaking change

[Regenhardt]

Also I'd like additional opinions on this. Once we have user accounts, we should probably add the user as package owner, even if it's just to see who pushed what package. After that we can talk about authorization concepts like restricting uploads to an owner or something.

Once established, an auth concept is very complicated to amend so I'd like to do it right from the beginning.

[Regenhardt]

Could you also add an integration test for a private feed? Maybe a new class AuthIntegrationTests. Or add an authenticated request to the nuget client integration test, although I'm not sure if it gets ugly to add authentication for just one test method and not the others.

[tomasfil]

LMK if you need anything else

[Regenhardt]

Very nice, I think it's just minor things now but all in all it looks good. Would still prefer an additional review though, @bagetter/maintainers. I won't block the PR for long if nobody else finds time for a review either though, don't worry.

[Regenhardt]

I see #147 asks for package-specific API keys, do you think this PR may be later extended to enable that? The authentication options might be extended to include a package prefix, e.g.

"Authentication": { 
  "Credentials": [
    {
       "Prefix": "Company.Team1.*",
       "Username": "Team1User",
       "Password": "Team1Pass"
     }
  ]
}

Maybe the additional ApiKeys should also be in another object for extension without breaking changes later? Maybe in the same Authentication object like

"Authentication": {
  "ApiKeys": [
    {
      "Key": "..."
    }
  ]
}

Not sure if this is overkill or if there is a different, easier/better way. This would however let us extend it with package prefixes later without introducing breaking changes.

[Regenhardt]

Oh when the handler is in .Web, you could add an extension method so the startup can call it like services.AddAuthentication().AddNugetBasicHttpAuthentication() or something.

Possible calls:
services.AddAuthentication().AddNugetBasicHttpAuthentication() as auth scheme next to calls like ´AddBearerToken() services.AddBasicHttpAuthentication()as complete alternative toAddAuthentication() BaGetterApplication.AddBasicHttpAuthentication()` which would make sure it can only used with BaGetter and not with other applications accidentally that just include BaGetter

I'm not currently fit enough to completely make the choice so, anything with an explanation why would be great.

[tomasfil]

The apiKey object makes sense. I would split the per/package authentication into separate PR later on. I can implement that. Here I would like to focus on the base with enabling the functionality down the road.

Because there is to consider if we would like to go with prefix like that, or maybe define auth scopes (array of prefixes), that could be then referenced in api keys and credentials. Might sound like a lot, but I dont think that would be that big of overhead for configuration.

Or any other auth structure. Depends....

[tomasfil]

Yup, makes sense to limit that for BaGetterApplication, so it does not interfere with anything else

[tomasfil]

I have a dilema.
Move ApiKeys into NugetAuthenticationOptions, or leave it in base next to the original ApiKey, both have pros and cons.

But maybe moving it into NugetAuthenticationOptions would be sensible in case another features like the per package/scope auth feature implementation?

[seriouz]

@tomasfil Okay, but don't stress yourself!

[seriouz]

@tomasfil Just bumbing as not to be forgotten :)

[tomasfil]

@seriouz @Regenhardt Resolved the comments.

[Regenhardt]

Thanks, gonna take a look when I'm back. I've been in the hospital for a week unfortunately.

[tomasfil]

How are we looking here?

I hope you are doing well @Regenhardt !

[Regenhardt]

Oh yes, way better, walking almost looks normal again 😊. Still getting settled and cleaning up after a month of not being at home, I'm gonna look at this soon, haven't forgotten it don't worry.

[chenzuo]

I hope the PR merge can be successful and add a private package option. for any teams using this open source project hope for authenticated access rather than open access. this is a very important feature. without this feature, we can only solve it through other additional ways, but this solution does not seem to be based on software engineering principles. so I hope this PR merge successful soon.

[tomasfil]

I will just add, that I am using locally deployed image for production sice Jul with 5 developers and everything works as expected, nothing broke yet and we didn't find any security issue of leaking the packages without authorization

[Regenhardt]

LGTM, thanks a lot for this great addition @tomasfil! Will try to make a new release today :)

[Regenhardt]

Turns out the test project doesn't build, probably needs a using updated.

[chenzuo]

I’m waiting for your feature to be released because I plan to share it at a local tech meetup, focusing on project and code security. I’m really excited about it and check multiple times a day to see if it’s out yet.

[chenzuo]

thanks a ton for all ! this given us an amazing and super useful feature. we really appreciate it!

[theazgra]

Hi,

great to see private feed getting implemented. I have already deployed it. Only question I have is. While feed is private (access to index.json is protected by basic auth) the Web UI search is not limited, and packages can be downloaded from there. Is there solution to this, which I have overlooked?

Thanks

[tomasfil]

@theazgra They can be browsed, but not downloaded when credentials are setup.

[tomasfil]

Once you insert credentials for download, then your browser remembers it tho

[theazgra]

@tomasfil Ah i See, ok, that makes sense. :) this + path deny in HAProxy is sufficient for our needs. Thanks