Kareadita / Kavita

Kavita is a fast, feature rich, cross platform reading server. Built with the goal of being a full solution for all your reading needs. Setup your own server and share your reading collection with your friends and family.
http://www.kavitareader.com
GNU General Public License v3.0
5.94k stars 308 forks source link

New Feature: OIDC authentication support #2518

Open jakkarth opened 8 months ago

jakkarth commented 8 months ago

If this is a feature request, request here instead. Feature requests will be deleted from Github.

It's been discussed there as https://feats.kavitareader.com/posts/23/alternative-account-provider but I'd like to take a shot at an actual implementation, which is why I'm posting here.

The pitch:

OIDC is an open security standard that allows applications to offload the task of identity management and validation. There are large companies that implement OIDC, such as Microsoft and Google and Facebook and Discord, there are companies that provide identity management as a service like Okta, and there are self-hosted options like Keycloak and Authentik. Rather than implement an identity management interface ourselves (eg LDAP), OIDC is a light-weight standard in terms of code changes for Kavita. It also allows users/administrators to create more robust authentication mechanisms like MFA that Kavita's built-in user authentication system does not support, or to integrate with their existing communities eg manga discussion discords. This would also give us feature parity for Komga.

Implementation thoughts

This is my first time proposing a new feature for this project, and I'm doing my best to follow the guidelines outlined in CONTRIBUTING.md. I'm very open to constructive feedback. I have my local dev environment set up, but I have not started working on the changes because I want to check the approach with you all first. It sounds like opening an issue is the right way to do that rather than doing work and then opening a PR.

OIDC is an open standard with a single URL the client can use to request all of the necessary authentication endpoint URLs. The general flow would be that the user clicks "Sign in with SSO" option presented at login, the user is redirected to the identity provider (IdP) and completes the appropriate login process if necessary (may be skipped if user is already logged in, for example), the user is redirected back to Kavita with an authentication code. Kavita on the backend makes a request to the IdP to exchange the code for information about the user, such as preferred username and email address. Kavita would then use that information to see if an existing user matches that information and logs them in, or creates a new account if there's no match.

Minimum configuration for Kavita would include the metadata endpoint, the client id, and the client secret. Optional configuration would include a user-friendly name of the IdP they'll be redirected to, and an option to disable the built-in authentication system so that only OIDC authentication will be possible. We may also need options to map values in the OIDC response to Kavita fields, for example an OIDC provider might use preferredName instead of displayName for the user.

My thought about these config options is that they should be set via environment variables since this would be something to configure before the first admin user is created. Putting it in the web interface admin section would mean requiring at least one non-OIDC admin user. However, I don't know of any other options beyond timezone that Kavita currently configures this way. Open to suggestions there.

For the UI, we'd need to disable the user's ability to change password (since the IdP manages that). We'd need to remove the password reset and forgot passwords flows for the same reason. Optionally, we could instead let the admin set a URL to take users to to complete those operations. The initial admin account creation, user creation, and login processes on the UI would need to respect the OIDC configuration options presented by the API. If the user selects to login or register via the IdP, the UI should redirect to the IdP, and should pass the returned authorization code to the API to complete the login or registration process. We'd need to handle a variety of error conditions regarding that flow, plus API responses about conflicts with OIDC-specified users that already exist as non-OIDC users.

On the API side, we'd need an endpoint to return the OIDC settings, endpoint to login via OIDC, endpoint to register via OIDC. We'd need to modify the User model to indicate whether the user is a native Kavita user or an OIDC user (to prevent OIDC users impersonating native Kavita users). We'd need to get the OIDC settings out of the environment. We'd need to store the OIDC partial login state. And we'd need code to talk to the IdP to complete the authorization code->user details conversion.

I think that's about as far into the weeds as I'd want to get at the moment. Am I on the right track here? It looks like it'd be a popular feature based on the voting, and

majora2007 commented 8 months ago

First off, I think overall you have a great grasp on the changes needed. I want to bring @DieselTech and @therobbiedavis into the discussion as this is something I know they are also passionate about.

I don't think using environment variables is the correct path forward. Instead, I would like to utilize the appsetting.json, where we already support advanced features, like x-frame options overrides. This will allow us to have strongly typed classes for the settings via IOptions<> pattern in .NET.

I think also investigation in .NET's OIDC should be investigated to see if it will ease any of the implementation from us, as Kavita already uses the Identity system to handle authentication.

For the UI, we'd need to disable the user's ability to change password (since the IdP manages that). We'd need to remove the password reset and forgot passwords flows for the same reason.

I think we can handle this by utilizing roles on the accounts and for OIDC accounts, they will not have these roles. This will allow us to have restrictions on the UI selector to ensure these roles aren't allowed to be assigned to the user.

We may also need options to map values in the OIDC response to Kavita fields, for example an OIDC provider might use preferredName instead of displayName for the user.

My question here would be, are any OIDC providers supported or we provide a limited set of providers out of the box? If the former, I do agree and we'd want to allow for appsettings.json to support, but then allow UI settings to further add new settings, etc. It will have to be it's own system to make it streamlined and not restrictive, outside of the initial setup in appsetting.json.

Once this moves a bit further, I don't mind drawing up some rough draft wireframes. I think many users would love this as it is our top feature request, but something I don't personally use/need.

DieselTech commented 8 months ago

I don't think using environment variables is the correct path forward

I agree as well, especially because Kavita runs natively on windows. You would run the risk of people corrupting their PATH by cramming a bunch of variables in it. Plus I'm just not a fan of exposing application level configs to the entire system so that anyone can read them. Especially something so sensitive as authorization endpoints

utilizing roles on the accounts and for OIDC accounts

I'm thinking a role like 'login' was, except that can't turn it off (without disabling OIDC, more on that shortly)

The thing we'll have to make sure of is allowing people to change OIDC providers if they want. That will involve allowing them to disable / remove the config. We'll have to decide how to handle what happens with the accounts. Most likely is just converting them back into normal accounts and when they re-enable OIDC again they can have their users sign up with the same email to claim an existing account. On that note, using an email is going to be required for it to link with an account on kavita's side. So username only users won't be able to be converted to a OIDC user.

We will also need a way to let the admin change the URL's that they point to for the endpoints. The field should be editable and not locked in case they mistype something and it needs to be changed. Blanking the fields is not the same as disabling the integration because you'll have to run queries against the DB to change the users roles.

jakkarth commented 8 months ago

The more eyes the better! I'll wait to get started until they've had a chance to weigh in.

I hadn't seen the appsettings.json work, and now that I have I agree that makes more sense. I'm glad I checked in first before writing code!

I was thinking about using https://www.nuget.org/packages/Microsoft.AspNetCore.Authentication.OpenIdConnect rather than writing my own implementation as far as possible. Having written my own client implementation from scratch before, I'd much rather let Microsoft carry that maintenance weight. That said, it will be my first time using that library, so I don't know what I don't know.

I think we can handle this by utilizing roles on the accounts and for OIDC accounts, they will not have these roles. This will allow us to have restrictions on the UI selector to ensure these roles aren't allowed to be assigned to the user.

Would this mean not allowing those roles to be added to OIDC accounts? I was anticipating OIDC admin users being a thing, and it seems like admins skip a lot of role checks, but maybe I'm not fully understanding what you mean on that count.

My question here would be, are any OIDC providers supported or we provide a limited set of providers out of the box?

I only have Keycloak to test with, but my goal would be to support anything out of the box. It's also worth noting that even with a single provider like Keycloak, the IdP admin can still configure the response fields to be anything they wish. The field names aren't per vendor.

It sounds like you have an idea of what the web admin interface would look like, and as long as the admin user can be bootstrapped using the config in appsettings.json, I'm on board with that idea I think. I was hoping to minimize the UI work necessary to get it running though. I'd love to see a ui mockup of what you envision the interface looking like in this regard.

I'm just not a fan of exposing application level configs to the entire system so that anyone can read them. Especially something so sensitive as authorization endpoints

The concern about running natively on Windows is fair. My perspective comes from building 12Factor apps intended to run inside docker containers, where providing settings through config files can be cumbersome. I don't share your concern about the authorization endpoints though; they're publicly discoverable and contain no sensitive information. If you're relying on obscurity of a domain name or path element in a URL, you're probably not going to be running Kavita on your system anyway. That said, if it makes more sense from a Windows security standpoint to put it in a config file, I have no opposition to that technically or philosophically.

with the same email to claim an existing account

I have strong concerns about this idea. I believe that OIDC users and native users should never intermingle or be converted between. There is no guarantee that an IdP provider will verify that a user truly owns an email address, for example. So if I knew your admin account's email address and yours was a native account, and I could create an account with that email on the IdP, I could then leverage that to claim your existing admin account. At the very least I think this should be not possible for admin users, require admin approval on a case-by-case basis (perhaps the ability to "merge" an existing native and existing oidc account?), or be a config option with a sternly worded warning message and be disabled by default. If we do allow it, I think we should do a forgot-password-like flow where we generate a "claim this native user" token and validate the email again at least.

I'd consider support for migrating users from one OIDC provider to another, or to a native user type, to be a phase 2 goal. Regardless of the approach chosen from the options above, that's significantly more API and UI work than what I had originally envisioned as minimum viable feature.

Not trying to be argumentative, I think we just have slightly different perspectives on how we envisioned the feature to work. Would you be willing to consider an initial PR that doesn't include claim/merge/change OIDC provider?

majora2007 commented 8 months ago

Would this mean not allowing those roles to be added to OIDC accounts? I was anticipating OIDC admin users being a thing, and it seems like admins skip a lot of role checks, but maybe I'm not fully understanding what you mean on that count.

For this, I was talking about the restriction of change password, etc. We can restrict OIDC users when they are created to NOT have those roles. It is true that admins bypass a lot of those checks, so we might still need an OIDC role so we can explicitly restrict certain functions.

I wasn't envisioning the ability to convert from one to another. I was thinking more of having the option to authenticate with OIDC, but the underlying account is still native. So I could use Google or Facebook and at the end of the day, it will log me into my native account. Is this not a valid solution?

I also wonder if multiple providers are possible, how does the UI know the icons for the different providers? Surely in the form for configuring a provider, we should either resolve it then and put into config/icons as provider_name.png/webp/etc or allow the user to upload their own, if they are using something like Authentik, which may not have an icon.

jakkarth commented 8 months ago

So I could use Google or Facebook and at the end of the day, it will log me into my native account. Is this not a valid solution?

Maybe we're getting stuck on the terminology here. I'm using "native account" to mean "login credentials store in the Kavita database" and "OIDC account" to mean "account that exists in the Kavita database with a flag indicating it's an OIDC account and no login credentials." OIDC accounts can't log in using the username/password fields in kavita's ui, and native accounts cannot be logged into by OIDC methods. It would not be possible to have a single account that can be logged into via both methods. Both accounts still exist in the Kavita database to store user preferences etc.

I also wonder if multiple providers are possible, how does the UI know the icons for the different providers?

I would leave that up to the admin to decide. We could provide a "URL to OIDC IdP Logo" and "OIDC IdP name string" fields or something similar. I wouldn't suggest having 30 built-in configs for various providers. Even if we did, discord as an example lets individual servers have their own icons, and the Kavita admin likely would want to use that logo rather than the generic discord one. I'm also not keen on the legal concern of storing the intellectual property of other company's logos etc. Just let the user set them.

majora2007 commented 8 months ago

native vs oidc

Yes, I understand and am on the same page. Hence why I don't believe conversion is something that can happen.

logo

I'm not too concerned about the logo, but I wont be supplying. Like I did with weblinks, I think we should provide some mechanism to make it as easy as possible for the user to get the logo onto their system and give the option to customize via cover upload component in the UI flow.

jakkarth commented 8 months ago

Hence why I don't believe conversion is something that can happen.

I think it's technically feasible, but I don't think it should happen, for the reasons I outlined with @DieselTech. I'm in favor of easy, and since an existing image upload function exists I'm happy to reuse it :)

majora2007 commented 8 months ago

The problem with not having it is users that have already setup their account wont be able to switch to using OIDC without setting up a new account and thus they loose all their user state.

Do you know how Komga handles this situation?

jakkarth commented 8 months ago

I would expect someone interested in using SSO probably intends to set it up when the Kavita instance is first deployed, so losing user state wouldn't be a concern in that case. And existing users could continue to use the existing login system and simply ignore the OIDC provider login option, no change for their login process and no loss of data.

The case you're describing would be in an admin decided to add OIDC, disable native login credentials, and has existing users. Or I guess if an existing user wanted to switch to using OIDC. One option would be to simply not support that use case, and document it as such, but it sounds like that may not be desirable.

If that is a requirement for this feature, the safest way to do it looks similar to the email validation flow plus the OIDC registration flow:

  1. User selects OIDC login option, or perhaps chooses "convert to OIDC" button in user preferences next to password change UI.
  2. User is redirected to IdP to create an account there or log into an existing account. Either way doesn't change anything for us.
  3. User is redirected with authentication code back to Kavita.
  4. Kavita uses authentication code to fetch user details from IdP.
  5. Kavita identifies an existing native credential account exists with that email address, and user is prompted "Do you wish to convert this account to an OIDC account?" If no, abort the process and return them to the Kavita UI.
  6. If yes, Kavita sends an email to the user asking them to verify they wish to convert the account to OIDC, using a token similar to how email validation already works.
  7. User clicks link in email confirming that they do wish to convert and do legititmately own this email address.
  8. Kavita confirms the validation token matches the email address, and if so, changes the user's credentials and roles as if the user had just been created from OIDC. ( we may want to preserve a subset of roles here rather than clobbering all of them, but specifically we want to set the OIDC role and remove the password change role at minimum)
  9. User is logged into a new session so that new roles etc take effect.

I'm aware that Komga has OIDC support, but I'm a Kavita user and haven't tried Komga. Their documentation doesn't seem to specify how email collisions are handled.

DieselTech commented 8 months ago

The problem with not having it is users that have already setup their account wont be able to switch to using OIDC without setting up a new account and thus they loose all their user state.

That's my real problem. Kavita has been around for 3 years already. Enabling OIDC means anyone who's already been running it has to sacrifice 3 years worth of reading history and bookmarks to use it. We need to have a way to migrate existing users to secure OIDC users somehow. Maybe like what you said where you do the lost password reset flow, or an admin can manually link if needed.

Maybe the admin can set an account flag on a users native account allowing it to be converted. That way the flag can be set right when both users are ready to work on it and can claim it "in real time".

I would expect someone interested in using SSO probably intends to set it up when the Kavita instance is first deployed,

I honestly don't think that is the use case in the selfhosted / homelab world. People usually start with running the apps they want and then months later find out about SSO and want to tack it on as an extra.

jakkarth commented 8 months ago

I honestly don't think that is the use case in the selfhosted / homelab world.

I may be the exception here then, and I recognize that my experience in this area definitely informs my perspective on the popularity of different use cases. I implemented Keycloak for SSO on my homelab services early on, and as that gets easier to do (eg Nextcloud offering OIDC server support with an LDAP backend, Keyclock in a container, Authentik and so on) I suspect that will become an earlier part of the homelab experience for others. But my experience is my own, and you obviously know your userbase better than I do, so I'll trust you on the likelihood of various scenarios.

We need to have a way to migrate existing users to secure OIDC users somehow.

Do you think that admins implementing OIDC would want to wholesale replace authentication for all users at the time that OIDC is implemented? If so, a bulk conversion might be reasonable. The process for that would be to delete all existing credentials, add the OIDC role to all users, add/remove other roles as appropriate. It could be done preemptively at the time OIDC is configured, applies to all users, there are no longer any native logins that need to be claimed. This would obviate the need for the claiming process entirely, with the caveat that you'd have to trust the IdP to validate emails if that's a concern for you. For someone converting an existing server to use OIDC, this may be a viable option.

But if you want to have native login and OIDC login users coexist, I don't see a safe way to do that without the complex flow described above. That's a lot of code to write and maintain to support a coexist use case. Doing anything less than that risks native accounts being sniped.

Our discussions around the admin UI for setting logos etc has already increased the scope of the feature implementation beyond what I had originally anticipated, and I'm honestly not sure I have the time to spend on implementing the claim flow as well. That's what's prompting my comments around minimum viable feature and phase 2. I would rather have a partial working-for-one+-usecase implementation that can be built upon later than not get this feature available at all, if that makes sense. I enjoy Kavita and I would love to be able to give back to the project this way, but I'm also trying to be realistic about what I can commit to.

majora2007 commented 8 months ago

Let's put a hold for right now. Let me talk with Diesel and Robbie and get back to you. I understand you want to get a MVP out and that is fine, but having a vision for the complete product is important. I can jump in and help deliver the product once the MVP is put together, but it is critical we define the initial scope, the total scope, and how the OIDC works.

Having a solution that requires users to restart is not an acceptable solution. Care must be given to ensure it's as seamless and easy for the end users, even if that increases maintenance and time for us developers.

I'll do a bit of research and comment once I have more information in my mind.

jakkarth commented 8 months ago

Sounds good! And I'm not saying I can't do the claim flow too, to be clear. I'm not sure how much the scope is going to change from what I had originally envisioned, and I think the outcome of the conversation you'll have will definitely help inform that.

Also want to mention here that this has been a really fun conversation for me. A lot of times when I offer to implement a feature or fix a bug write some documentation, sometimes even with a PR if it's a small change, I end up ignored for months. You have both been welcoming and thoughtful both in your messages to me and in keeping the best outcomes for the users in mind. Even if we decide not to move forward I still consider this a successful interaction, and I commend you for building a welcoming community around the project.

Please let me know if there's anything you need from me. I'll put this on hold until I hear back. :smiley:

DDriggs00 commented 7 months ago

Any update on the direction we want to take this?

DieselTech commented 7 months ago

Any update on the direction we want to take this?

Documentation needs to be made as to how the ODIC provider will interact with the login process, how the login process itself works, how the .net security works, what screens need checks, what processes can't be put behind it, how to present it to the user, how to convert existing users to ODIC accounts, how to configure the UI around the admin side of setting up the provider.

It's a ton of work and planning before any of the code gets written so we can account for everything from the start.

majora2007 commented 6 months ago

Hey @jakkarth Sorry for the long time between responses. So here's the deal: I don't have the bandwidth right now to scope out the whole feature. But I don't want to stop the motivation.

Here's what I'm proposing. Build a POC for this. Implement something like OIDC with Google Auth, figure out how the customization works and the authorization mechanism. Even if this doesn't get merged in, it can open the door to the proper implementation.

Currently, development is done by me. OIDC is something I don't have much want to work on (despite the community wanting it). If you were to start that investigation and I saw some of the challenges solved, then it's much easier for me to pick up and drive it home.

If you no longer want to work on OIDC, no problem, just close this issue out.

jakkarth commented 6 months ago

I'm glad that there's still interest on your side. There's still interest on mine as well, though I don't have as much time on hand at the moment to work on it as I did when initially proposed. You're proposing that I build a proof of concept implementation of OIDC within Kavita, based on my own understanding of what that implies, is that correct? I can understand your reluctance to deal with the OIDC code, and I have similar reluctance to dive too deeply into the UI code, so maybe that will complement well. If you're okay with me moving forward on the mostly-backend POC based on my own ideas, and then we can regroup and hand it off, that sounds good to me! If I encounter questions along the way, is there a dev chat? Should I just ask questions here in the issue? Or do the best I can to muddle through on my own? Just trying to understand the level of support to expect, any answer is fine.

majora2007 commented 6 months ago

Exactly that, you focus on the backend which will be the most complicated and hand off and I can see how to drive it home, even if I need to recode it.

If you want to reach me and talk, the best place is in dec-discussions on the discord server or we can create a thread and talk there. An issue here (or just on this one here) also works.

Don't hesitate to reach out at all.

C0chett0 commented 5 months ago

TL;DR : I beg you, follow the industry standards and don't create different types of users, merge OIDC incoming users to existing native users when email addresses match, and provision new native users when they don't !!

Hi there,

I was considering adding Kavita to the list of tools I self-host for my family, but to make it to this list SSO is a non negotiable feature which brought me to this page. And reading your conversation I felt the urge to chip in.

Just for context, I'm a Senior Backend Developer with 20+ years of experience, mostly in PHP and JS but I dabbled with .NET a long time ago. So trust me, I know OIDC and most widely SSO in and out.

All of you seem to have a very good grasp on it too except on one point but a very important one. You must NOT have separated OIDC and native users, that goes totally against the idea of SSO. A user is a user no matter how he chooses to identify himself, in your app ! It's up to the admin to choose which entrypoints should be available on his platform (oidc from google, oidc from its own idp, native login/password, ldap, ...). But a single user should be able to use any of this ways at any point and end up connected to his one and only account no matter which flow he chose. You see this feature on many many websites self hosted or not! First create a local account with your google email address then log out and try to login with Google OIDC and you'll be offered the possibility to merge your local account with your OIDC access. That's basic stuff.

If I created my account on a website through my own oidc idp and never used "native" authentication, but the IDP I used disappear for some reason, I can still switch to "native" authentication simply by asking for a lost password flow.

On the other hand when oidc will be available, an admin with existing user base could decide to turn off "native" authentication, every user would have to sign in with oidc and link this with their existing user account, no bulk conversion needed. Obviously this means that the user uses the same email address with the IDP and the Kavita instance, but I think the main use case is self hosted home servers so it should be the case, and even if there are a few holes the admin can always map the missing one by hand.

The last idea against this usual way of dealing with multiple authentication flows I read in your conversation revolves around security.

There is no guarantee that an IdP provider will verify that a user truly owns an email address, for example. So if I knew your admin account's email address and yours was a native account, and I could create an account with that email on the IdP, I could then leverage that to claim your existing admin account.

I very very strongly disagree with this, how on earth are you going to create an account with my admin email on my IDP ? Or even worse on Google IDP ? What kind of admin do you think I am ? Do you really think I would roll out SSO on one of my apps without being sure the native admin account is already mapped to my own IDP account ?? More importantly this sentence implies that the level of confidence is higher in native auth than in an IDP even though IDP have been created specifically to be the highest possible frame of reference for Identity ! To be honest if for some reason there was a discrepancy identifying a user between my local auth and an IDP in my PHP code, I would most likely give priority to the IDP, it's its very own job to be sure !

Arbel-arad commented 5 months ago

many services allow you to pick a user claim type (username, email, user ID) as defined on the server, so users won't be able to match an existing account if configured correctly even with uncontrolled IDPs (which you shouldn't have) another option could be similar to what bookstack uses where "external" accounts get an external account ID that the admin can map manually. https://www.bookstackapp.com/docs/admin/oidc-auth/#switching-to-oidc-with-existing-users

DieselTech commented 5 months ago

https://www.bookstackapp.com/docs/admin/oidc-auth/#switching-to-oidc-with-existing-users

This is good to know and how I sort of envisioned it going for Kavita. Getting some examples of how other projects handle this will help us form a good basis on how to best move forward.

jakkarth commented 5 months ago

@C0chett0 thanks for including your thoughts in the conversation. I am also a 20+ year senior developer who works a lot with authentication systems. I wouldn't use that to discredit someone else's thoughts about security though. 20 years ago we used md5, and we have a lot of things to unlearn that more junior folks may not. Having experience doesn't automatically make one's opinion on a topic better.

You make some categorical generalizations in your post. For example, saying that it's an "industry standard" to automatically merge OIDC and native accounts, or that having different ways of handling them "totally goes against the idea of SSO." If anything in this discussion has been clear, it's that there are no absolutes like this; there is always a tradeoff, whether that's between security, convenience of the user, convenience of the admin, and so on.

For my experience, I have never met a system that would automatically log me into a native account if I used oidc with an account with a matching email address, and I am familiar with no industry standard (as in, a standard the way that OIDC is a published open standard) that would suggest that they should. Pinterest is an example: if I create my account using google sso, I cannot use my gmail address and the forgot password function to gain access to my account there. As for the "idea of SSO," let's be clear on what that actually is: allowing a user with an account on a single IdP to access resources on multiple other systems. It does not mean that a single user can use multiple sign ons for a single account; in fact, that's the opposite use case. SSO is intended to increase convenience for the user by not having to create multiple accounts for different services, not to allow them to log into a single service multiple different ways.

Your statement "how on earth are you going to create an account with my admin email on my IDP ?" misses the point here. OIDC is an open standard. While some Kavita administrators may have their own IdP for the purpose of allowing their users to also use other services they host, the other use case is to allow admins to outsource their authentication to a third party that isn't theirs (with amazon, google, discord, twitch, etc being commonly cited examples). In such a scenario, the admin doesn't control the IdP, can't audit its security policies or its code, nor manage the user accounts that exist there in any way. Thus, the possibility of someone being able to create an account there that doesn't satisfy the email verification requirement is up to the IdP rather than the server admin, and could change after the admin has configured their Kavita instance.

Because any IdP is obviously an entirely separate codebase, and one which the Kavita admin may not have source code to nor admin privileges on, trusting them for authentication by definition increases your attack surface. At the very least there is more code that could contain authentication bugs. At worst, the IdP could be compromised or change its security policy to one the Kavita admin may not agree with. Regardless of whether you would ever run such a setup, I'm sure there are many admins who would be willing to trade some amount of security for the convenience of letting their users use an existing google account to log in, and we need to be able to deal with that situation rationally by at the very least providing tools for the admins to choose how much trust it places in the IdP. Having the option for accounts that are either/or gives the admin the ability to guarantee that a particular email address can only use local authentication and that, regardless of how much trust the admin places in the IdP, the admin account will always be safe. This can and must be the default situation for admin accounts. Any deviation from that, for admins or users, could be achieved in a variety of ways. But supporting separate accounts that only accept a single login path is an absolute security necessity, and should be the default configuration.

You are, of course, free to disagree with me. You are also free to do your own implementation of OIDC for Kavita, and submit the PR. I haven't even started yet, and may not get an opportunity to for quite sometime, and as a Senior Backend Developer with 20+ years of experience who knows OIDC and most widely SSO in and out, I'm sure you could make quick work of it.

zodac commented 3 months ago

I know this discussion was a little while ago and potentially things have moved on, just wanted to comment on this question:

The problem with not having it is users that have already setup their account wont be able to switch to using OIDC without setting up a new account and thus they loose all their user state.

Do you know how Komga handles this situation?

I just set up OIDC with Tandoor. They provide an option to connect an existing user to an external system: image

This worked pretty well for me, I'd like to see similar if Kavita ever adds this integration. :)

gruselglatz commented 1 month ago

To add my 2ct I like how Komga does it. Its simple and good. It would also be nice to start with the simple and open world: Keycloak?