nginx-proxy-manager icon indicating copy to clipboard operation
nginx-proxy-manager copied to clipboard

FEAT: Open ID Connect authentication

Open oechsler opened this issue 1 year ago • 29 comments

Continuation of: #2630

  • Merge develop into the feature branch, resolving conflicts
  • Fix issues with the settings list template, where the oidc-config would have the text of the default-site

Tested with Authentik 2024.8.2

P.S.: I hope we can get this finally merged! :rofl:

oechsler avatar Sep 19 '24 17:09 oechsler

It's probably worth adding those PRs:

  • https://github.com/NginxProxyManager/nginx-proxy-manager/pull/3952
  • https://github.com/NginxProxyManager/nginx-proxy-manager/pull/2630

Seems like there are multiple PRs implementing the same feature, it's probably the best to wait for @jc21 to decide which one to merge.

CrazyWolf13 avatar Sep 23 '24 17:09 CrazyWolf13

@CrazyWolf13 This is a continuation of the abandoned #2630 PR and #3952 was done as part of this PR it looks like. We just need @jc21 to take a look and merge if everything is good

Shocktrooper avatar Sep 24 '24 19:09 Shocktrooper

Any update on this? I would love to see it merged

zanda8893 avatar Oct 21 '24 15:10 zanda8893

There are conflicts, and CI won't build you a docker image until they're resolved

jc21 avatar Oct 30 '24 04:10 jc21

There are conflicts, and CI won't build you a docker image until they're resolved

Thanks for clarifying, I will resolve them later today.

oechsler avatar Oct 30 '24 06:10 oechsler

I'm glad this PR will be finally merged! Thanks a lot for the effort of all the people involved @jc21 @oechsler @Shocktrooper

jcebrianalonso avatar Oct 30 '24 23:10 jcebrianalonso

Yes many thanks for the contribution but don't count your chickens just yet. This is a big PR and it will need thorough testing both with OIDC and without. I'd feel happier if there was some cypress tests for both scenarios as well.

Besides Authentik, can we get this tested with other providers too?

Also some documentation will need to be added around this

jc21 avatar Oct 30 '24 23:10 jc21

Yes many thanks for the contribution but don't count your chickens just yet. This is a big PR and it will need thorough testing both with OIDC and without. I'd feel happier if there was some cypress tests for both scenarios as well.

Besides Authentik, can we get this tested with other providers too?

Also some documentation will need to be added around this

@jc21 - Thank you very much for the review and your feedback on this topic!

Could you possibly elaborate a bit more on what the tests should specifically cover? I don’t have much experience with the Cypress framework, but I’m more than willing to invest the time needed to bring this PR to completion.

As for testing with other OIDC providers, I’d leave that to others in this thread for now, as I currently only have Authentik running. However, with the new Docker test image, it should be straightforward to set up an instance.

I’m also happy to take on the documentation. Given that OIDC isn’t always the most straightforward topic, I agree it would be beneficial to provide clear guidance. Should we add a dedicated section to the documentation for this, or integrate it into an existing section?

- Nonetheless I would also be happy if somebody else could lend a hand :wink:

oechsler avatar Oct 31 '24 20:10 oechsler

Besides Authentik, can we get this tested with other providers too?

I just tested this with Auth0 by Okta and can confirm that it works. You need to set up an Application as a "Single Page Application". Auth0 also requires that your Callback URL (aka "Redirect URL") be secured with HTTPS.

Then, the config maps as follows:

Auth0 Setting Name NGINX Proxy Manager Setting Name Example Value
Domain Issuer URL https://dev-foobarbaz.us.auth0.com
Client ID Client ID Aan...J7
Client Secret Client Secret jh...._...Hit
Allowed Callback URLs Redirect URL https://nginx-proxy-manager-url.foo.com/api/oidc/callback

I also tested it with my own Authentik installation and can confirm that it works there as well.

chutch1122 avatar Dec 09 '24 06:12 chutch1122

Just tested this with Authelia and can confirm that it works. This is my snippet from authelia configuration.yml:

identity_providers:
  <OTHER NECESSARY CONFIGURATION FOR OIDC>
  oidc:
    clients:
      -
        client_id: 'iPCR.......c6h'
        client_name: 'Nginx Proxy Manager'
        client_secret: '$pbkdf2-sha512$310000......UIpZA'  # SPECIAL: has to be pbkdf2-digested. In NPM UI, there is the undigested secret
        public: false
        consent_mode: 'auto'
        pre_configured_consent_duration: '1 week'
        redirect_uris:
          - 'https://nginx-proxy-manager-url.mydomain.tld/api/oidc/callback'
        scopes:
          - 'openid'
          - 'email'
          - 'profile'
        authorization_policy: 'two_factor'
        authorization_signed_response_alg: 'none'

      -
        <OTHER OIDC CLIENTS>

As long as the email in Authelia matches the E-Mail in NPM, it works. However, this is not the cleanest solution, as the email is not guaranteed to be unique in the OIDC standard. Howerver, this is something that I came across ofthen when implementing OIDC services and doesn't bother me much in an home lab environement (which is think is the main target of NPM, anyway). Some Clients use preferred_name some use the email, and some the ID. It's not very consistent across the board...

svenwanzenried avatar Dec 09 '24 23:12 svenwanzenried

One thing to note @svenwanzenried is that unless NPM intends to support multiple concurrent OIDC providers email should be fine to match. I believe that NPM would need to rework users to a large degree if it started to allow multiple users to all use the same email seeing as how email is currently used in the tool

Shocktrooper avatar Dec 10 '24 16:12 Shocktrooper

that may be so. and as i said, it's probably fine. But if you would define multiple users in Authelia with same email but different username (e.g. Sven and SvenAdmin) which is absolutely possible, then you'd have a conflict on login. You'd probably just end up in the same Account in NPM.. Which.. considering you have control over the same email address may or may not be an issue.

svenwanzenried avatar Dec 10 '24 17:12 svenwanzenried

I made some changes to @oechsler 's branch to address some of the requested items (PR here).

  • Cypress automated tests for updating the OIDC config
  • Documentation for setting up SSO with OIDC

I also made the following UI/UX changes:

  • A more user-friendly error message for when a user does not exist in NPM and they are attempting to sign in via an IdP
  • Fixing a typo in the "hint" description so that the name of "Redirect URL" matches the field label in the modal.

chutch1122 avatar Dec 10 '24 23:12 chutch1122

Hey @chutch1122, and thank you also, @svenwanzenried, for testing with other providers, dedicating your time to implementing the Cypress tests, and writing the documentation. I’ll take a look at the changes over the next few days and merge them into the PR branch as soon as possible. I’m really happy to see the momentum behind this feature —great work!

oechsler avatar Dec 10 '24 23:12 oechsler

I also added some additional UI-based end to end tests. These tests update the OIDC config to be enabled/disabled and ensure that users can still log in.

I'd like to add some additional tests around logging in with a "dummy" OIDC provider, but this should provide some good coverage and peace of mind that users won't get locked out.

Edit: Not sure why the build is failing. The tests ran on my machine. Edit 2: Figured it out. Some things changed with error messages since this branch was created. Merging in develop and updating some of the test expectations seems to do the trick.

chutch1122 avatar Dec 11 '24 19:12 chutch1122

Docker Image for build 4 is available on DockerHub as nginxproxymanager/nginx-proxy-manager-dev:pr-4010

Note: ensure you backup your NPM instance before testing this image! Especially if there are database changes Note: this is a different docker image namespace than the official image

@jc21 Do you want E2E tests with an actual mock OIDC provider to be included before merging this in? Other than that, is there anything else you'd like added?

Otherwise, I think it's ready for review again and includes your requests from above.

chutch1122 avatar Dec 13 '24 15:12 chutch1122

I've recently added OIDC support to the v3 branch, and that also adds a locally running Authentik server pre-configured.

I'd suggest waiting for me to merge one of the outstanding postgres PR's which I can then add the authentic db to that, then the cypress e2e tests can be improved from there. Here's the v3 test I've written.

jc21 avatar Dec 16 '24 22:12 jc21

The develop branch now has postgres support and Authentik setup in both CI and dev.

To bring up the dev environment:

./scripts/start-dev
# and remove it after you're done
./scripts/destroy-dev

Then access the web hosts with:

  • NPM: http://127.0.0.1:3081
  • Authentik: http://127.0.0.1:9000

Authentik notes:

  • log in with akadmin:admin
  • Support for LDAP and Oauth/OpenID is ready to go
  • Login with Oauth/OpenID using cypress:fqXBfUYqHvYqiwBHWW7f

See the following cypress tests, they are skipped currently however

to run cypress locally:

cd tests
yarn install
yarn cypress:headless --spec cypress/e2e/api/Ldap.cy.js

jc21 avatar Dec 24 '24 08:12 jc21

@jc21 Any updates?

Shocktrooper avatar Jan 13 '25 16:01 Shocktrooper

Would be nice to see this merged. Hope you manage to complete testing this asap

LuKePicci avatar Feb 05 '25 10:02 LuKePicci

Looking forward to seeing this in the primary docker soon.

adamoutler avatar Feb 14 '25 11:02 adamoutler

Just wondering if any progress has been made with this? I have Authentik setup myself and would be happy to test if additional testing is necessary.

gdeeble avatar Mar 04 '25 13:03 gdeeble

Would love to see this merged, looking forward to working with my keycloak instance without needing some kind of auth proxy middleware

WirtsLegs avatar Mar 13 '25 17:03 WirtsLegs

Sorry to add another useless message, but please, when you see that the last 4 messages are nothing but people being impatient about the feature, maybe you could just think twice before posting the 5th message about being impatient about this feature.

Billos avatar Mar 13 '25 21:03 Billos

Could somebody explained me why this PR can’t be merged from September 2024? Is any unresolved issues? Or are we waiting NPMv3? Please, let us know!

alexsalex avatar Mar 28 '25 01:03 alexsalex

Could somebody explained me why this PR can’t be merged from September 2024? Is any unresolved issues? Or we’re waiting NPMv3? Please, let us know!

As far as I can see in the comments here, this is running in Dev Branch. The developer wants to ensure high quality before unleashing upon the general population. There are some major changes coming.

adamoutler avatar Mar 28 '25 02:03 adamoutler

Could somebody explained me why this PR can’t be merged from September 2024? Is any unresolved issues? Or we’re waiting NPMv3? Please, let us know!

As far as I can see in the comments here, this is running in Dev Branch. The developer wants to ensure high quality before unleashing upon the general population. There are some major changes coming.

I would gladly try the dev branch if there were an image for it. I haven't been able to figure out how to build a container from my own copy of this repo. :-\

lhoekenga avatar Nov 03 '25 23:11 lhoekenga

@lhoekenga Here for this PR: https://hub.docker.com/layers/nginxproxymanager/nginx-proxy-manager-dev/pr-4010/images/sha256-63d49412b3363fa2fcda5a0d1321fd9c0fe25813ef3334220fc9fc7fad1b3943

and here for the dev branch: https://hub.docker.com/r/nginxproxymanager/nginx-proxy-manager-dev/tags

as far as I can tell this from their dockerhub.

CrazyWolf13 avatar Nov 04 '25 12:11 CrazyWolf13