Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenIDC SSO through apache stopped working after update to 3.7.6 #15811

Closed
iaingeorgeson opened this issue Apr 22, 2024 · 11 comments · Fixed by #15890
Closed

OpenIDC SSO through apache stopped working after update to 3.7.6 #15811

iaingeorgeson opened this issue Apr 22, 2024 · 11 comments · Fixed by #15890
Assignees
Labels
status: accepted This issue has been accepted for implementation type: documentation A change or addition to the documentation

Comments

@iaingeorgeson
Copy link

Deployment Type

Self-hosted

NetBox Version

v3.7.6

Python Version

3.9

Steps to Reproduce

This is a longstanding NetBox instance. It runs under gunicorn, proxied through apache which is configured to use mod_auth_openid for authentication.

NetBox's configuration includes:
REMOTE_AUTH_ENABLED = True
REMOTE_AUTH_BACKEND = 'netbox.authentication.RemoteUserBackend'
REMOTE_AUTH_HEADER = 'HTTP_OIDC_CLAIM_PREFERRED_USERNAME'
REMOTE_AUTH_AUTO_CREATE_USER = True

This was working fine until the update to 3.7.6 following our usual procedure:

Pull and checkout v3.7.6.

Run upgrade.sh

Restart NetBox gunicorn service, netbox-rq and apache

Since the upgrade, NetBox has presented a login box instead of logging in as the REMOTE_AUTH_HEADER user. Using tcpdump, I can see the "OIDC_CLAIM_preferred_username" header is being sent to gunicorn. Other instances using the same OpenIDC configuration are working.

Expected Behavior

REMOTE_AUTH login using OpenIDC credentials.

Observed Behavior

The web frontend prompts for username and password.

@iaingeorgeson iaingeorgeson added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Apr 22, 2024
@jeremystretch
Copy link
Member

This was working fine until the update to 3.7.6 following our usual procedure

I'm not aware of any changes in v3.7.6 that would impact this functionality. What version of NetBox were you running prior to the upgrade?

The one thing that does jump to mind is that we've bumped gunicorn from 21.2.0 to 22.0.0. There's nothing immediately obvious as impacting in its release notes, but it's probably worth downgrading to the previous version and seeing if that resolves the issue.

@jeremystretch jeremystretch removed their assignment Apr 23, 2024
@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation and removed status: needs triage This issue is awaiting triage by a maintainer labels Apr 23, 2024
@iaingeorgeson
Copy link
Author

Thanks. The previous version was v3.7.5.

I've downgraded gunicorn using

(venv) root@netbox:~# pip3 install --upgrade gunicorn==21.2.0

and restarted the gunicorn service. Now OpenIDC is working as expected.

Looking at the release notes, we might have been impacted by changes related to HTTP headers.

@jeremystretch
Copy link
Member

and restarted the gunicorn service. Now OpenIDC is working as expected.

Nice!

Looking at the release notes, we might have been impacted by changes related to HTTP headers.

That's very strange; I wouldn't expect any of the listed changes to have an impact on normal headers, but maybe we're hitting something else.

@apollo13
Copy link

Hi @iaingeorgeson, would you mind re-configuring apache to send the header with dashes instead of underscores? Ie OIDC-CLAIM-preferred-username, because I think the newest Gunicorn might be dropping headers with underscores in them. Referencing it as HTTP_OIDC_CLAIM_PREFERRED_USERNAME in Netbox/Django is correct though as that is the proper canonicalization.

@apollo13
Copy link

Or you can probably set header-map to https://docs.gunicorn.org/en/stable/settings.html#header-map to dangerous (be aware of the consequences though)

@jeremystretch
Copy link
Member

From what I can tell, this cryptic messages in the gunicorn change log

HTTP header field names Gunicorn cannot safely map to variables are silently dropped, as in other software

is meant to convey the change proposed in this gunicorn FR, which was implemented in v22.0. As @apollo13 helpfully points out above, the original (pre-v22.0) behavior can be restored by setting header_map = 'dangerous' in the gunicorn configuration file.

While I think I understand the motivation for the change in default behavior, it would seem unnecessary for a typical WSGI deployment assuming the HTTP frontend is configured not to accept headers with underscores (nginx disables this by default). More to the point, if the use of such headers is to be discouraged, is there a more preferred means of conveying remote authentication information from the HTTP frontend?

IMO the only immediate course of action as far as this specific issue is to annotate in the NetBox release notes & documentation the need for the additional configuration option where remote authentication and gunicorn are in use.

@apollo13
Copy link

it would seem unnecessary for a typical WSGI deployment assuming the HTTP frontend is configured not to accept headers with underscores

That assumes that there is such a HTTP frontend (I'd hope there generally is, but you never know what people do). Then again in an ideal world a server like Gunicorn could be stable enough to expose to the internet.

More to the point, if the use of such headers is to be discouraged, is there a more preferred means of conveying remote authentication information from the HTTP frontend?

The use of headers containing _ is discouraged and the preferred way is to use - instead.

Whether the change in gunicorn was wise I cannot say.

@jeremystretch
Copy link
Member

Ok, I'm going to treat this as a documentation issue for now as I don't see any clear action to take. We could change the default values of the HTTP headers used to convey remote authentication parameters, but that would break existing deployments. (It's also potentially dangerous.)

@jeremystretch jeremystretch self-assigned this Apr 29, 2024
@jeremystretch jeremystretch added type: documentation A change or addition to the documentation status: accepted This issue has been accepted for implementation and removed type: bug A confirmed report of unexpected behavior in the application status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Apr 29, 2024
jeremystretch added a commit that referenced this issue Apr 29, 2024
@iaingeorgeson
Copy link
Author

I've configured apache to change the header name to contain dashes, and it's now working as expected under gunicorn 22.0.0. Thanks for your help.

@MalfuncEddie
Copy link

I've configured apache to change the header name to contain dashes, and it's now working as expected under gunicorn 22.0.0. Thanks for your help.

Care to share your config? (Same issue)

@candlerb
Copy link
Contributor

Care to share your config? (Same issue)

#16102 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted This issue has been accepted for implementation type: documentation A change or addition to the documentation
Projects
None yet
5 participants