Opinions on new authentication provider system

[stenlan]

Hello, I was directed here from my PR at jellyfin/jellyfin. I would like to hear your opinions about this PR. It:

• Is nearly fully HTTP API backwards compatible, except:
  • AuthenticationProviderId on the User and UserPolicy dto (see note about supporting TOTP MFA in old/incompatible clients). That property is now present on SessionInfoDto.
  • HasPassword and HasConfiguredPassword on User dto have been removed
• Completely refactors authentication in a way that more cleanly separates the responsibilities of the session manager, user manager and the new IUserAuthenticationManager, and decouples it from the username/password type of authentication and instead makes it fully generic. Added some easy to use abstract building blocks for authentication providers;
  • AbstractAuthenticationProvider (used by username password auth)
  • AbstractExternallyTriggeredAuthenticationProvider (used by quick connect auth)
  • AbstractChallengeResponseAuthenticationProvider (WIP; can be used by general challenge-response type authentication providers).
• The default authentication provider has been adapted to the new system, and database migrations have been created to automatically migrate to the new system.
• Adds TOTP MFA support to the back end
  • Includes legacy client authentication fallback by appending TOTP token to password for compatibility with old clients. This is not unheard of, apple has done this too: https://discussions.apple.com/thread/254488819?sortBy=rank
  • Front end/web client support pending PR (TODO: add web client PR reference)).
• Changed IAuthenticationProvider interface to a generic IAuthenticationProvider<T> interface, where T is the type of payload data that an authentication provider takes upon authentication, so that multiple types of authentication can be supported, and different providers can be registered to the same type of payload data, so that different username/password providers can still be easily implemented to replace the existing ones too.
• Now also incorporates Quick Connect as simply an implementation of AbstractExternallyTriggeredAuthenticationProvider.

I have chosen not to use ASP.NET Core Authentication since it's not just authentication but also authorization, which is deeply rooted throughout this entire application. I also don't think it's necessarily the way to go even if it were easier to switch to it, since it's quite opinionated and mostly geared towards web clients, while the Jellyfin backend serves not just web clients but all kinds of clients. My implementation provides a lot more flexibility and is not as opinionated.

[JPVenson]

I am generally against building any more custom implementations when there is already a fitting solution available within aspcore. The statement that aspcore auth code is build towards web is just plain wrong i have been using it for years in pure C# clients without any issue or modifications, it supports bearer tokens out of the box via prebuild providers and that is the way we want to go long term to solve some issues overall.

Also your statement that it tightly interacts with the Authentication is true, which we are already using
https://github.com/jellyfin/jellyfin/blob/master/Jellyfin.Server/Extensions/ApiServiceCollectionExtensions.cs#L101
so i see absolutly no reason not to also use the other half of the apple.

The AspCore auth framework is what most developers should be highly familiar with as its the default every boilerplate aspcore app comes with and there isn't any widely used alternative. When it comes to Authentication and security we should absolutly avoid reinventing the wheel as people far smarter then us already build something for us to use that handles maybe the most critical part of our application.

[stenlan]

Okay

[stenlan]

@JPVenson I considered just giving up, but I just have to say I am quite taken aback by your reception. This isn't just about me "rolling my own" alternative of ASP.NET Core authentication. It also separates the tightly interwoven authentication and authorization logic that was plaguing the back end and which I assume has been causing any progress with regards to authentication to be stagnant for the past 5 years. Even if this isn't a final solution, it's a gigantic leap in the right direction compared to the extremely rigid username password authentication that exists right now. I believe you are making a big mistake.

[JPVenson]

My issue with it is that its yet another custom implementation that we have to refactor into proper ident framework once its in the codebase. I see no value in doing anything in the existing auth framework as we have to rip that all out anyway when we go with the ident framework and with your code touching something as sensible as the auth code, a higher level of scrutiny is required.

This would be the same as going ahead and implementing dapper for database access before my rewrite to EFcore. Its just not adding any long term benefit its just a step sideways though into the wrong direction nethertheless. I would be immensly greatful if you were actually implementing the ident framework proper because from there, TTOP; Oauth and much much more would be a 10 minute implementation task. And you basiclly re-implemented parts of the ident framework anyway but worse, which is what puts the whole idea so far off to me.

[OmicronGuy]

I have no experience in ASP.NET Core or generally coding with frameworks at all.

But is there a specific reason that the authetication methods for 2fa/totp have been standing still for the past 2 years since it was put on planned?

[nielsvanvelzen]

Simple reason, nobody worked on it. When something is put on "planned" it means we want it to happen, not necessarily that somebody is working on it.

[tuckerthomas]

Continuing this discussion a bit:

Reviewing stenlan's code and checking on JPvenson's pre-requisites here:

just as an FYI:

As i already told you in chat and others too:

1. A new Auth implementation should probably build upon ASP.NET Core Identity:
   https://learn.microsoft.com/en-us/aspnet/core/security/authentication/?view=aspnetcore-9.0

2. We should really discuss this first in a MetaIssue before you start coding to avoid you spending time and it then just gets rejected because it does not align with the teams vision.
   https://github.com/jellyfin/jellyfin-meta

Original comment

A few dumb questions as I'm looking to commit some time to contributing code:

1. Does Jellyfin use ASP.NET Core Identity already, or would this require a new implementation?
   Based on, jellyfin's package definition, I don't believe it is.
2. As it sounds like the team has a few specific requirements for something like this to be implemented, is there a chance we could break those down?
3. Are there any docs regarding user authentication and creation I could review to familiarize myself with the way Jellyfin does things?

I reviewed documents in: https://jellyfin.org/docs/general/contributing/development, but things look a bit sparse, which is okay, mostly curious.

Also heads up, new to Jellyfin's codebase in general, so looking through: https://jellyfin.org/docs/general/contributing/ and getting sorted.

I'll likely start on a better "first issue", but might not hurt to start this discussion. Let me know if this needs to be moved as well.

@nielsvanvelzen @JPVenson Since I saw activity from both of you on this discussion already, apologies for the ping.