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 https://github.com/bagetter/BaGet/issues/142 https://github.com/bagetter/BaGet/issues/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

[tomasfil]

LMK if you need anything else

[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?

[Regenhardt]

I'd say move it into the options object, so the root API key stays as the simple default case where you just want to restrict pushing (I assume many companies just have their feed available in their local network anyway), and the complex options are an optional extension for more sophisticated concepts.

[tomasfil]

Will do tomorrow

[tomasfil]

As for me its GTG

[arkangel-dev]

I propose that we actually implement full access control with actual user entities... Im currently working on implementing that... I've finished linking packages to accounts. It just needs the following

• Implement checks during write operations
• Implement DbMigrations for the storage providers
• Maybe a UI for updating the users. Right now all of it is managed by the REST endpoint

Id love it you went through it @Regenhardt : https://github.com/bagetter/BaGetter/compare/main...arkangel-dev:BaGetter:main

Ive had to put it off for a while because of work, and the codebase being extremely abstracted

[tomasfil]

I understand. But I must say. I come from open-source and corporate. And this implementation is big no go from many point of views, because this means another management and ecosystem locking.

Also its not only about User entities. How do you define permissions, you would have to think about setting up roles, control groups, share groups, there is a LOT to consider when going into those waters...

It propably makes sense to create full access control with user entities, but it is definetly not straighfowrard as this. Because many existing users use Azure, Windows AD auth etc.

Simple auth scheme such as this PR can be later on easily extended, but I dont think its wise to lock users in this way. Because I could see implemening certificate auth, or something completely different like AWS. It should consider authentication flows like OAuth 2.0 etc, to allow automatization of workflows etc...

I think that the approach should propably be

1. Implement simple and basic configurable auth - Needed by majority
2. Thoroughly design Auth API
3. Allow extension nuget packages to implement and inject them

[arkangel-dev]

I just pushed the final commit to my fork, its finished, I understand where you're coming from, what I've done is a very simplified implementation of access control. Just enough to suit my needs and I dont mind if my changes aren't merged into the main fork, but the way I see it, it is bounds better than just multiple api keys having full write access to all packages.

Also i dont understand what you mean by

It should consider authentication flows like OAuth 2.0 etc, to allow automatization of workflows etc...

Are you talking about the web page?

[tomasfil]

Also i dont understand what you mean by

Are you talking about the web page?

What I was trying to say is, that people should have the ability to use the authentication that they have now, if at all possible

[arkangel-dev]

I think I got what you mean. The authentication they have now, as in the API key that is currently implemented right?

[tomasfil]

No, I think the authentication should be in core simple as possible to allow basic control like this PR does. - Also package control can be added in similiar fasion

And after that everything else should be abstracted out in some way (To allow any authentication solution to be used) and possibly provide built in solution like your implementation. But there are many possible solutions from simple to complex and it really depends what are the goals with the Bagetter

[arkangel-dev]

I dont mind the authentication. I think its really well done... Its the authorization performed when writing to the package list. I think I'll take a stab at implementing the authorization part after your merge 😄

[Regenhardt]

I do like both of your implementations, I think with the extensibility @tomasfil got going on here we could very well extend it using @arkangel-dev 's implementation afterwards, thanks a lot for that! I must admit that, in the beginning, I definitely wanted something like the implementation of @arkangel-dev, but now am excited by the simplicity and extensibility of what @tomasfil built here.

@arkangel-dev How do you actually use your admin controller, do you have your own client? I didn't see a new page for that. Or does the nuget client include an interface for that? There should definitely be a service handling all that from an MVC page, so then there can be both the page and the controller injecting the service.
Also when checking the token, the checks that do not access the database should probably happen before the check that accesses the database to avoid unnecessary DB connections. Also constants for the auth schemes and roles, unless we secretly plan to make roles configurable too in the future so it doesn't matter anyway.
I'd be absolutely ok with a draft PR of your changes so we can track it and have a better space to talk about how to proceed with it, even if it takes months. I removed the stale-issue-bot exactly because development is slow here and I don't think a to-do is done just because I ignored it long enough.

@tomasfil I've gotta look at the latest changes but I think we're good to go too, just don't have time right now, I'll try on the weekend.

I do like the idea of another project having the actual identity implementation, we could have BaGetter reference and use that directly and publish it as NuGet package for other people consuming the library. It would however be nice if it's compatible with ASP.NET Core identity somehow, although I just don't have the overview about that one to even guess how compatible it currently it. That would pave the way to easier addition of other identity systems like oauth2 though.

Similar to how other providers work, the whole identity implementation should only be used (i.e. DB created/migrated and context called) if it is configured at all, if possible the addition should maintain the default configuration of a simple to set up nuget feed with next to no configuration necessary to run. This is of course all speculative about how optional all that can be done. I think BaGetter does have a very nice architecture for these kind of things though.