search

gotson / komga

Media server for comics/mangas/BDs/magazines/eBooks with API and OPDS support
https://komga.org
MIT License
3.76k stars 219 forks source link

[oauth2] support custom signing algorithms and PKCE for social login #1507

Open ToxicMushroom opened 2 months ago

ToxicMushroom commented 2 months ago

This pr would allow users to configure different oauth2 id_token signing algorithms And prevent against worst consequences of accidental client-secret leakage using PKCE.

Please let me know if I should do the configuration stuff differently or if things (beans) are in the wrong place :) Or if the defaults should stay pkce disabled and RS256 signing.

I currently tested these changes locally using komga [bootRun] dev,localdb,noclaim,oauth2 and my kanidm instance configured as custom oauth2 client/provider. I also tested with the github example, seems to also support ES256 and pkce.

gotson commented 2 months ago

Hi, can you provide more context ? I have no idea what the problem is in the first place.

ToxicMushroom commented 2 months ago

Hi, can you provide more context ? I have no idea what the problem is in the first place.

I cannot use my IDM tool kanidm because of the spring security library only doing RS256 token signing out of the box. Spring also does not come with PKCE by default and disabling PKCE is also considered less secure as per https://kanidm.github.io/kanidm/master/frequently_asked_questions.html#why-is-disabling-pkce-considered-insecure.

These things can sadly not by enabled via the spring oauth provider config (with no interest from spring upstream to add it), thus the code changes here to allow for it.

Sorry for the lack of context, I had provided this in the discord and didn't think further about it after being told to create an issue/pr.

gotson commented 2 months ago

Thanks. This should ideally be baked in Spring Boot, as mentioned here. I have not seen this raised in the Spring Boot repo though.

I am reluctant to merge this PR, as the setting would apply to all registered OAuth2 providers, and not just the one needed.

I think a better approach would be to raise this in the Spring Boot repo and see if they can add support for it in the auto configuration.