Merging oidc branch with develop

[jc21]

See discussion on #753

See this doc for instructions.

NOTE: For anyone wanting to test this patch, back up your entire NginxProxyManager config and database first. You won't be able to jump back to the latest tag afterwards as this PR will run a database migration.

[jc21]

This is an automated message from CI:

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

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

[noelhibbard]

I just updated my docker-compose to use jc21/nginx-proxy-manager:github-pr-1388 and I don't see a new "OpenID Connect" tab. I'm looking at your commits and I see where you added a tab, I'm just not seeing it. I must be missing something.

[notchrissss]

Looks like it breaks the "New Proxy" button on Edge (my fault for using Edge, but strange as it does still work in Chrome).
edit: adding proxies breaks in this image.

[notchrissss]

I just updated my docker-compose to use jc21/nginx-proxy-manager:github-pr-1388 and I don't see a new "OpenID Connect" tab. I'm looking at your commits and I see where you added a tab, I'm just not seeing it. I must be missing something.

You could try doing a CTRL+F5, your browser could be caching the dashboard but it's unlikely.

[noelhibbard]

I just updated my docker-compose to use jc21/nginx-proxy-manager:github-pr-1388 and I don't see a new "OpenID Connect" tab. I'm looking at your commits and I see where you added a tab, I'm just not seeing it. I must be missing something.

You could try doing a CTRL+F5, your browser could be caching the dashboard but it's unlikely.

Ctrl+F5 was the first thing I tried. I believe I was editing an existing host though. Maybe this tab only shows up when creating a "new" host. I already rolled back to my previous docker image so I can't easily check.

[jakefrancois5]

Looks like it breaks the "New Proxy" button on Edge (my fault for using Edge, but strange as it does still work in Chrome). edit: adding proxies breaks in this image.

Same error in chromium & firefox. Appears to be an issue with openid connect mandatory fields still being required trying to create a new proxy host without filling in the open id connect tab.. However if I create a new host using open id connect, it works.

Error:
[10/2/2021] [3:24:18 PM] [Express ] › ⚠ warning insert into proxy_host (access_list_id, advanced_config, allow_websocket_upgrade, block_exploits, caching_enabled, certificate_id, created_on, domain_names, forward_host, forward_port, forward_scheme, hsts_enabled, hsts_subdomains, http2_support, locations, meta, modified_on, openidc_allowed_users, openidc_auth_method, openidc_enabled, openidc_restrict_users_enabled, owner_user_id, ssl_forced) values (0, '', false, true, false, 0, NOW(), '["something.com"]', '192.168.10.40', 7878, 'http', false, false, false, '[]', '{"letsencrypt_agree":false,"dns_challenge":false}', NOW(), '[]', 'client_secret_post', false, false, 1, false) - ER_NO_DEFAULT_FOR_FIELD: Field 'openidc_redirect_uri' doesn't have a default value

[notchrissss]

At least you actually know what to look for 🤣🤣

…

________________________________ From: jakefrancois5 ***@***.***> Sent: Sunday, 3 October 2021 1:00 AM To: jc21/nginx-proxy-manager ***@***.***> Cc: Christian Clark ***@***.***>; Comment ***@***.***> Subject: Re: [jc21/nginx-proxy-manager] Merging oidc branch with develop (#1388) Looks like it breaks the "New Proxy" button on Edge (my fault for using Edge, but strange as it does still work in Chrome). edit: adding proxies breaks in this image. [image] <https://user-images.githubusercontent.com/89428437/135395202-a5e95aad-618f-469e-868b-5fa31a5ebfc9.png> Same error in chromium & firefox. Appears to be an issue with openid connect mandatory fields still being required trying to create a new proxy host without filling in the open id connect tab.. However if I create a new host using open id connect, it works. Error: [10/2/2021] [3:24:18 PM] [Express ] › ⚠ warning insert into proxy_host (access_list_id, advanced_config, allow_websocket_upgrade, block_exploits, caching_enabled, certificate_id, created_on, domain_names, forward_host, forward_port, forward_scheme, hsts_enabled, hsts_subdomains, http2_support, locations, meta, modified_on, openidc_allowed_users, openidc_auth_method, openidc_enabled, openidc_restrict_users_enabled, owner_user_id, ssl_forced) values (0, '', false, true, false, 0, NOW(), '["radarr.metalrip.com"]', '192.168.10.40', 7878, 'http', false, false, false, '[]', '{"letsencrypt_agree":false,"dns_challenge":false}', NOW(), '[]', 'client_secret_post', false, false, 1, false) - ER_NO_DEFAULT_FOR_FIELD: Field 'openidc_redirect_uri' doesn't have a default value — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#1388 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AVKJDVOP5QFBTRHBJCOZ7F3UE4QRTANCNFSM5DVZKHZQ>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[skoging]

Currently additional locations are not protected behind oidc. Would adding this line to /backend/templates/_location.conf be sufficient?

Maybe this should be behind a toggle so that opt-in or opt-out is possible per location?

[skoging]

A workaround for anyone else that need this is to copy the generated oidc config into the advanced section of the locations.

In my case I copied it to a file in the custom folder and imported that in the locations that need it. This also make it easier to share the same oidc setup for multiple proxy hosts, but that isn't as big of a deal for me.

[ForceConstant]

Any updates on this? Would love to get support for Keycloak in master.

[jakefrancois5]

I've identified the cause of the issue. The default values of '' for the new 'text' type mariadb columns such as 'openidc_redirect_uri' are not actually being set despite the knex code migrations telling it to.

Manually connecting to the database and executing the following SQL command for each text column related to openidc fixes the issue:

ALTER TABLE npm.proxy_host MODIFY COLUMN openidc_client_secret text CHARACTER SET utf8 COLLATE utf8_general_ci DEFAULT '' NOT NULL;

Reasonable solutions/workarounds for this would be:

1. Allow null values on these columns
2. Try using the string type (varchar) instead of text and see if that works (would be limited to 255 characters)
3. (Below)

Not tested, but I'm thinking that adding the following code for each column in backend/models/proxy_host.js may fix the issue as well:

// Set defaults for blank text columns
		if (typeof this.openidc_redirect_uri=== 'undefined') {
			this.openidc_redirect_uri= '';
		}

Found this issue for Knex that would appear to be the root cause:
knex/knex#2649

[natereprogle]

I'd love to see this pushed to main as well. However, when I tried to pull the github-pr-1388 build I had the same issue @noelhibbard did, in which the OpenID tab would not appear. Official support for OpenID would be very nice compared to the Access Lists that already exist (Which have issues of their own)

[clemans]

Any updates? Really want this feature merged into main! Recently revamping a project that absolutely need OIDC support and as a recent supporter of NPM would hate to have to switch to Traefik.

[mattchewey]

Adding my vote that this would be very beneficial. Hope this can get merged soon.

[JeWe37]

Is there some means of helping out with this so it can get merged? Any way to be useful for verification?

[JeWe37]

To give perhaps a few more datapoints: I gave this a shot myself, rebasing to develop was easy enough, and to me at least it seems to work just fine. I also haven't been having any of the issues mentioned above on firefox. In so far as i understand it what is described regarding the database was already fixed in 9f2d3a1 . Are there any blockers to merging this officially? For the time being now that i figured out how to build this I can keep on using my version.

For reference: I use Keycloak as my OIDC provider and have had no issues just setting it up the standard way.

[validide]

First of all I would like to say I really like the project.

I am wondering what is the state of this merge request or it there is any way of assisting with it.

[Kurnihil]

I've "rebase" the openidc branch to develop, you can find it here:
https://github.com/Kurnihil/nginx-proxy-manager/tree/oidc

i've setup without problems keycloack to protect phpmyadmin login page...

if something more is needed to merge it into develop let me know!

[Hadatko]

+1

[tezgno]

@Kurnihil, how can I pull your version to test on my end?

EDIT:

I forked the repo and added the changes. Going to do some testing over the next few days.

[skutter-de]

What is the current status here?

[zzzz0317]

I noticed that this branch in the project hasn't been merged into the master for three years. I'm interested in this feature and willing to help. Could anybody tell me its status and what I can do to assist?

[DFS-90]

I've "rebase" the openidc branch to develop, you can find it here: https://github.com/Kurnihil/nginx-proxy-manager/tree/oidc

i've setup without problems keycloack to protect phpmyadmin login page...

if something more is needed to merge it into develop let me know!

@Kurnihil :
Thanks for your efforts!

I wanted to test oidc, but I never built docker images on my own before.
The steps I took (trial and error) to get this ready:

• spin up a Linux machine (I took a Debian VM) and install docker and git
• log in as root
• cd ~
• git clone --branch oidc https://github.com/Kurnihil/nginx-proxy-manager.git
• cd ~/nginx-proxy-manager/scripts/ci
• ./frontend-build
• cd ~/nginx-proxy-manager/scripts
• ./buildx
• docker image ls -> find image without name and write down its ID
• docker tag IMAGE ID USERNAME/nginx-proxy-manager:TAG
  -> e.g. docker tag e82k4nd2ij dfs90/nginx-proxy-manager:oidc

If you want to push your image to Docker hub:

• register at Docker hub and generate a personal token (see guides on the internet)
• docker login -u USERNAME
• use token as password
• docker image push USERNAME/nginx-proxy-manager:TAG

The image I generated can be found as "dfs90/nginx-proxy-manager:oidc" on Docker hub and is built on Kurnihil's "oidc" branch.

@Kurnihil :
Sorry for my dumb question:
As I am not capable of "rebasing" the "openidc" branch to "develop" branch myself (I just don't know how to do it):
Is it possible to do an update on your "oidc" branch so that it uses the most recent "develop" branch of the npm project?

Thanks and best regards,
David

[KeinNiemand]

Is this the same features wise as #2630? Both PR try to add OIDC support yet both seem to have existed for over a year without getting merged.

[oechsler]

Is this the same features wise as #2630? Both PR try to add OIDC support yet both seem to have existed for over a year without getting merged.

I thing these are different topics. To my understanding this feature is using Open ID Connect as authentication for a proxy host, whilst #2630 uses it to authenticate users on the management interface.

[github-actions]

PR is now considered stale. If you want to keep it open, please comment 👍

[DFS-90]

Please merge this pull request as duplicate "custom-forward-host-help" in "frontend/js/i18n/messages.json" breaks building. Thanks!