FEAT: Open ID Cconnect authentication

[marekful]

FEAT: Add Open ID Connect authentication method (SSO)

Resolves: #2562 #69 #437 #1624

• add oidc-config setting allowing an admin user to configure parameters
• modify login page to show another button when oidc is configured
• add dependency openid-client v5.4.0
• add backend route to process "OAuth2 Authorization Code" flow
  initialisation
• add backend route to process callback of above flow
• sign in the authenticated user with internal jwt token if internal
  user with email matching the one retrieved from oauth claims exists

Note: Only Open ID Connect Discovery is supported which most modern
Identity Providers offer.

Tested with Authentik 2023.2.2 and Keycloak 18.0.2

[nginxproxymanagerci]

This is an automated message from CI:

Docker Image for build 1 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-2630

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

[nginxproxymanagerci]

This is an automated message from CI:

Docker Image for build 2 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-2630

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

[jc21]

22:45:03  $ /app/node_modules/.bin/eslint .
22:45:03  
22:45:03  /app/setup.js
22:45:03    158:1  error  Expected indentation of 1 tab but found 2   indent
22:45:03    159:1  error  Expected indentation of 2 tabs but found 3  indent
22:45:03    160:1  error  Expected indentation of 2 tabs but found 3  indent
22:45:03    161:1  error  Expected indentation of 2 tabs but found 3  indent
22:45:03    162:1  error  Expected indentation of 2 tabs but found 3  indent
22:45:03    163:1  error  Expected indentation of 2 tabs but found 3  indent
22:45:03    164:1  error  Expected indentation of 3 tabs but found 4  indent
22:45:03    165:1  error  Expected indentation of 4 tabs but found 5  indent
22:45:03    166:1  error  Expected indentation of 5 tabs but found 6  indent
22:45:03    167:1  error  Expected indentation of 5 tabs but found 6  indent
22:45:03    168:1  error  Expected indentation of 6 tabs but found 7  indent
22:45:03    169:1  error  Expected indentation of 6 tabs but found 7  indent
22:45:03    170:1  error  Expected indentation of 6 tabs but found 7  indent
22:45:03    171:1  error  Expected indentation of 6 tabs but found 7  indent
22:45:03    172:1  error  Expected indentation of 6 tabs but found 7  indent
22:45:03    173:1  error  Expected indentation of 5 tabs but found 6  indent
22:45:03    174:1  error  Expected indentation of 5 tabs but found 6  indent
22:45:03    175:1  error  Expected indentation of 6 tabs but found 7  indent
22:45:03    176:1  error  Expected indentation of 5 tabs but found 6  indent
22:45:03    177:1  error  Expected indentation of 3 tabs but found 4  indent
22:45:03    178:1  error  Expected indentation of 3 tabs but found 4  indent
22:45:03    179:1  error  Expected indentation of 4 tabs but found 5  indent
22:45:03    180:1  error  Expected indentation of 3 tabs but found 4  indent
22:45:03    181:1  error  Expected indentation of 2 tabs but found 3  indent
22:45:03  
22:45:03  ✖ 24 problems (24 errors, 0 warnings)

[marekful]

Apologies.

[nginxproxymanagerci]

This is an automated message from CI:

Docker Image for build 7 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-2630

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

[rijnhard]

More of a general question with this PR. is it possible to provide the OIDC parameters using env vars or a config file to the docker container? Reason being, if you misconfigure something and it locks you out, recovery can become very painful. (atleast in my experience with other applications)

[krombel]

A question: This is only meant to allow login to nginx-proxy-manager admin UI, right?
Or would this allow configuring access to proxied sites?

[marekful]

Hey @rijnhard,

The risk of locking yourself out is the same with or without OIDC being involved because the default (built-in) login option is never hidden or becomes unavailable. If OIDC is misconfigured, you can login locally as admin and fix it.

Keep in mind that, as described above, a local user must exist in order to sign in to NPM with OIDC. That local user has a password in NPM that is independent of any remote identity. If you lose access to the configured OIDC identity, you can still sign in to NPM with the users's local password.

[marekful]

Hi @krombel,

Yes, that's correct. Following a successful OIDC authentication, you are practically signed in to NPM with the local user, only we didn't ask for the password (because the IdP already did). So everything stays the same as you were signed in using the same user's password for NPM.

[aaronnad]

This is just what im looking for at ensuring all my home services are SSO compatible! Would definitely love to see this in a release soon. (also using authentik as well).

[mwstamant]

Great PR! 👍

• SSO users dashboard metrics do not seem to populate correctly. i.e. 0 Proxy Hosts, 0 Redirection Hosts etc. (there are in fact entries that correctly display when authenticated as a local-only user.

[acester822]

For those following this, I actually got it to work right out of the box as configured lol. I basically evoked the lua scripts from a default.conf in NPM and from there I basically tweaked, added, and manipulated a configuration that automatically adds my OIDC aka Jumpcloud without me having to touch it on the NGINX side. I do have to add it to my OIDC integration on the other side, which is super easy. Now all my apps are secure again without cloudflare and with facial/fingerprint recognition!

[isaiah-v]

+1
I am also interested in this. I've pulled it down and tested it and everything seems to work, but it's been 8 months and the branch is starting to fall behind. Is it possible to resolve the conflicts add the NPM owner as a reviewer?

[acester822]

+1 I am also interested in this. I've pulled it down and tested it and everything seems to work, but it's been 8 months and the branch is starting to fall behind. Is it possible to resolve the conflicts add the NPM owner as a reviewer?

Ye, it is a little buggy the way that I implemented it, but I am sure it is because of crappy lua configuring, I bet it would be easy to implement properly,
basically all I did to get it to work was figure out the right places for the code and since this is based off of openresty it worked,

I made a default.conf and added this;

lua_package_path` '~/lua/?.lua;;';


  lua_ssl_trusted_certificate /etc/ssl/certs/ca-certificates.crt;
  lua_ssl_verify_depth 5;

  # cache for discovery metadata documents
  lua_shared_dict discovery 1m;
  # cache for JWKs
  lua_shared_dict jwks 1m;

Then in NPM, added this into the custom locations section,

	access_by_lua_block {

          local opts = {
             redirect_uri_path = "/redirect_uri",
             accept_none_alg = true,
             redirect_uri_scheme = "https",
             discovery = "https://oauth.id.jumpcloud.com/.well-known/openid-configuration",
             client_id = "get your own",
             client_secret = "nope",

          }

          -- call authenticate for OpenID Connect user authentication
          local res, err = require("resty.openidc").authenticate(opts)

          if err then
            ngx.status = 500
            ngx.say(err)
            ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR)
          end

          ngx.req.set_header("X-USER", res.id_token.sub)

Doing it this way, I can get it to work on a per port basis so things like Nextcloud with native SAML support work great, but all the other random ports I want to protect I can. The issue is on first page load it often times gives me an error, but on reload it works fine. Obviously this would not be the most elegant solution, but would be pretty simple to add in it seems. Hell you could even do a simple mode, which is basically what it is now, or switch to advanced mode where it would show the additional options.

[acester822]

@jc21 what are your thoughts?

[SecureCPU]

is this activated? or can I activate in my instance of NPM?

[acester822]

is this activated? or can I activate in my instance of NPM?

kind of but it is buggy and did not work well, I ended up going with traefik instead

[jcebrianalonso]

I'm super interested on this feature. Is there any plan to merge it soon?

[Shocktrooper]

@marekful Can you solve the merge conflicts?

[alexsalex]

@marekful Can you solve the merge conflicts?

[alexsalex]

Please please please resolve the conflict and merge it,please!

[alexsalex]

After testing, I have some issues.
I can't open the settings window in Safari browser.
I can't log out in any browser.

Clear cache solve the issue)

Otherwise, everything works fine! Love it!!!

[jcebrianalonso]

please @marekful solve the conflicts and merge. This would be a great addition to NGINX Proxy Manager!!

[ELISHELL]

不要将所有的后台程序都让NPM来代理认证。仅仅让NPM的管理页面可以享受OPENID就好。请速度合并。

Don't let NPM handle authentication for all backend processes. Only allow the management page of NPM to use OPENID for authentication. Please merge quickly.

全てのバックエンドプロセスに認証の代理をNPMに任せないでください。NPMの管理ページのみがOPENIDを利用できるようにしてください。速やかにマージしてください。

Lassen Sie nicht zu, dass NPM die Authentifizierung für alle Backend-Prozesse übernimmt. Nur die Verwaltungsseite von NPM sollte OPENID für die Authentifizierung nutzen können. Bitte fusionieren Sie schnell.

Не давайте NPM обрабатывать аутентификацию для всех фоновых процессов. Разрешите использование OPENID только для страницы управления NPM. Пожалуйста, проведите скорое объединение.

[moutasem1989]

I am looking forward for this to be solved and merged for the next release. Is there a time plan for it?

[ClearSeve]

Looking forward to the support of this feature

[LtCMDstone]

Another problem, wanted to test it but because of self signed certs I get this error
Discovery failed for the specified URL with message: unable to verify the first certificate

Is there any way to tell the OICD part another cert or disable the ssl check?

[ELISHELL]

If your keycloak service and Nginx proxy manager service are in a docker container, check the docker network for both. Chances are, they're not on an external network.

…

---Original--- From: ***@***.***> Date: Thu, Jun 6, 2024 17:34 PM To: ***@***.***>; Cc: ***@***.******@***.***>; Subject: Re: [NginxProxyManager/nginx-proxy-manager] FEAT: Open ID Cconnectauthentication (PR #2630) Another problem, wanted to test it but because of self signed certs I get this error Discovery failed for the specified URL with message: unable to verify the first certificate Is there any way to tell the OICD part another cert or disable the ssl check? image.png (view on web) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[LtCMDstone]

@ELISHELL thanks.
But I "fixed" it, noticed its a node module so I put in NODE_EXTRA_CA_CERTS= and gave it our cert, so no problems for now 😄

[alexsalex]

Please merge it

@jc21

[CrazyWolf13]

@marekful @jc21

Please there has been done soo much work to support OIDC leaving it just as a open PR is way too less.

Please consider adding fixing this PR and support for OIDC, this would really enhance the UX.

[AstralJaeger]

I'm also waiting for this feature to be completed.

[paradizelost]

This would be awesome to finally have!

[nbently]

It looks like there’s a lot of interest in getting this PR merged, but there are a few things that might need attention first.

There are some merge conflicts that would need to be resolved. If anyone has the time, it might be worth picking up this work, creating a new PR, and handling those conflicts.

Additionally, it seems like some issues came up during testing, and it might be important to address those as well. Adding some documentation would also be really helpful for others who want to use this feature after it is merged.

Just a friendly reminder that comments like “please merge” or “waiting on this” are not likely to speed things up, since open source contributors often have limited time. It’s possible the original author doesn’t have the bandwidth to finish this right now.

[oechsler]

It looks like there’s a lot of interest in getting this PR merged, but there are a few things that might need attention first.

There are some merge conflicts that would need to be resolved. If anyone has the time, it might be worth picking up this work, creating a new PR, and handling those conflicts.

Additionally, it seems like some issues came up during testing, and it might be important to address those as well. Adding some documentation would also be really helpful for others who want to use this feature after it is merged.

Just a friendly reminder that comments like “please merge” or “waiting on this” are not likely to speed things up, since open source contributors often have limited time. It’s possible the original author doesn’t have the bandwidth to finish this right now.

I’ve created a follow-up PR, resolved the merge conflicts, and corrected the branch to merge against develop. Unfortunately, I probably won’t be able to continue with further development or testing since I’m not too deep into the codebase at the moment. If anyone can help pick it up from here, that would be amazing!

Thanks for understanding!

[Shocktrooper]

@oechsler Do you just need someone to pickup review comments that might happen now? What else needs to be done

[oechsler]

@Shocktrooper,

I would be glad if someone could address arising comments, but of course, I’m happy to help wherever I can as well.

In my opinion, a good next step would be to get the maintainers or contributors involved so that we can proceed with the review and move towards a merge.

From my perspective, the feature is complete by the original creator. I’ve tested it, and it works as expected.

[ArthoPacini]

Any updates on this? This is huge for node proxy manager, integrating it with a self hosted LogTo instance for example.

It enables MFA, with hardware keys like Yubikey for example.

[oechsler]

Any updates on this? This is huge for node proxy manager, integrating it with a self hosted LogTo instance for example.

It enables MFA, with hardware keys like Yubikey for example.

@ArthoPacini - There is #4010 as a continuation, since you mentioned LogTo, would you be willing to test OIDC with that? - Currently as to @jc21's reply testing with different providers is needed. - #4010 (comment) - There is a Docker image available in the PR.