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

Login fails when BASE_PATH set #10837

Closed
candlerb opened this issue Nov 4, 2022 · 5 comments · Fixed by #10856
Closed

Login fails when BASE_PATH set #10837

candlerb opened this issue Nov 4, 2022 · 5 comments · Fixed by #10856
Assignees
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@candlerb
Copy link
Contributor

candlerb commented Nov 4, 2022

NetBox version

v3.3.7

Python version

3.8

Steps to Reproduce

  1. Install netbox v3.3.7 with BASE_PATH = 'netbox/'. Adjust Apache config accordingly.
  2. Try to login with the local admin login.

Expected Behavior

Should be able to login normally.

Observed Behavior

First time you click on the Log In button: you get the login page, and then you are returned to the front screen with a pop-up saying "Logged in as admin". However at this point you are still logged out, and the top-right button shows "Log In" rather than your username. Subsequent clicks of this button just refresh the screen, but you are still logged out.

Tested with Chrome and Firefox: identical behaviour.

This worked with v3.3.5 but doesn't work with v3.3.7. I haven't determined exactly where it changed, but I have reproduced this on two independent systems. This broke in v3.3.6.

@candlerb candlerb added the type: bug A confirmed report of unexpected behavior in the application label Nov 4, 2022
@candlerb
Copy link
Contributor Author

candlerb commented Nov 4, 2022

Checking developer console in an incognito window, I see the following response headers from the POST to login:

Set-Cookie: messages=W1siX19qc29uX21lc3NhZ2UiLDAsMjAsIkxvZ2dlZCBpbiBhcyBhZG1pbi4iXV0:1oqtqD:9oVpEum9r9ZDaB9A_vkK3hbgV6suhYzoncjEkFpxnBs; HttpOnly; Path=/; SameSite=Lax
Set-Cookie: csrftoken=KAxIwSj0Q6pxZU5hiKoEIGQ8E28FxEnwMJfzwaI5fG6HMyUiZvFjRGIf7y7vSs5X; expires=Fri, 03 Nov 2023 10:22:25 GMT; Max-Age=31449600; Path=netbox/; SameSite=Lax
Set-Cookie: sessionid=2ia9xis4xhgacqmenrztngc2hellcpk8; expires=Fri, 18 Nov 2022 10:22:25 GMT; HttpOnly; Max-Age=1209600; Path=netbox/; SameSite=Lax

I note that the csrftoken and sessionid have path netbox/ when it should be /netbox. (messages should probably also be bound to /netbox, but that doesn't matter too much)

Logging into a VM which is running Netbox v3.2.9 (but also with BASE_PATH='netbox/'), I see:

Set-Cookie: messages=W1siX19qc29uX21lc3NhZ2UiLDAsMjAsIkxvZ2dlZCBpbiBhcyBhZG1pbi4iXV0:1oqu8F:XQFbD1uKnnuF2AwXhc-0KqMrkQEQZqT7EwuBvZKkWnM; HttpOnly; Path=/; SameSite=Lax
Set-Cookie: csrftoken=RkWeYCDonUnHTbMsx7zxTAEOCgM1yR6Xay2ogelFpIpeSipKgnsLlmkBctnYZOmz; expires=Fri, 03 Nov 2023 10:41:03 GMT; Max-Age=31449600; Path=/; SameSite=Lax
Set-Cookie: sessionid=ayx0qo9l4qtn1hsp56zvshvfglw0y2s5; expires=Fri, 18 Nov 2022 10:41:03 GMT; HttpOnly; Max-Age=1209600; Path=/; SameSite=Lax

That seems to confirm my theory.

Maybe this isn't a Netbox issue, but a Django one, since upgrading Netbox would also have rebuilt the virtualenv.

@candlerb
Copy link
Contributor Author

candlerb commented Nov 4, 2022

On my test VM, I wound it back from 3.3.7 to 3.3.5, and I find that login is successful again (and the cookies have Path=/)

I wound it forward to 3.3.6, and it stopped working (Path=netbox/). So the problem is somewhere between 3.3.5 and 3.3.6.

Aha: almost certainly it's this from the 3.3.6 release notes:

  • #10639 - Set cookie paths according to configured BASE_PATH

... which in turn came from PR #10706

@candlerb
Copy link
Contributor Author

candlerb commented Nov 4, 2022

This is a quick-and-dirty patch that fixes it for me. I'll leave you to decide whether you want to DRY this.

--- netbox/netbox/settings.py.orig	2022-11-04 11:12:43.495829728 +0000
+++ netbox/netbox/settings.py	2022-11-04 11:19:23.526013549 +0000
@@ -85,7 +85,7 @@
 CORS_ORIGIN_REGEX_WHITELIST = getattr(configuration, 'CORS_ORIGIN_REGEX_WHITELIST', [])
 CORS_ORIGIN_WHITELIST = getattr(configuration, 'CORS_ORIGIN_WHITELIST', [])
 CSRF_COOKIE_NAME = getattr(configuration, 'CSRF_COOKIE_NAME', 'csrftoken')
-CSRF_COOKIE_PATH = BASE_PATH or '/'
+CSRF_COOKIE_PATH = '/' + BASE_PATH[:-1]
 CSRF_TRUSTED_ORIGINS = getattr(configuration, 'CSRF_TRUSTED_ORIGINS', [])
 DATE_FORMAT = getattr(configuration, 'DATE_FORMAT', 'N j, Y')
 DATETIME_FORMAT = getattr(configuration, 'DATETIME_FORMAT', 'N j, Y g:i a')
@@ -130,8 +130,8 @@
 SENTRY_TAGS = getattr(configuration, 'SENTRY_TAGS', {})
 SESSION_FILE_PATH = getattr(configuration, 'SESSION_FILE_PATH', None)
 SESSION_COOKIE_NAME = getattr(configuration, 'SESSION_COOKIE_NAME', 'sessionid')
-SESSION_COOKIE_PATH = BASE_PATH or '/'
-LANGUAGE_COOKIE_PATH = BASE_PATH or '/'
+SESSION_COOKIE_PATH = '/' + BASE_PATH[:-1]
+LANGUAGE_COOKIE_PATH = '/' + BASE_PATH[:-1]
 SHORT_DATE_FORMAT = getattr(configuration, 'SHORT_DATE_FORMAT', 'Y-m-d')
 SHORT_DATETIME_FORMAT = getattr(configuration, 'SHORT_DATETIME_FORMAT', 'Y-m-d H:i')
 SHORT_TIME_FORMAT = getattr(configuration, 'SHORT_TIME_FORMAT', 'H:i:s')

@jeremystretch jeremystretch added the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label Nov 4, 2022
@jeremystretch
Copy link
Member

Thanks for tracking this down @candlerb!

@prauscher
Copy link
Contributor

Thanks for pointing this out. In https://github.com/netbox-community/netbox/blob/develop/netbox/netbox/settings.py#L81 makes sure BASE_PATH always contains a trailing /. I'd suggest to use

SESSION_COOKIE_PATH = CSRF_COOKIE_PATH = LANGUAGE_COOKIE_PATH = f'/{BASE_PATH.rstrip("/")}'

just to be super explicit (or use a comment on your line that [:-1] only removes the trailing /). Once approved I could provide a PR

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Nov 10, 2022
jeremystretch added a commit that referenced this issue Nov 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants