Skip to content

Add deny-if-empty feature to ENV authentication#1565

Open
overhacked wants to merge 3 commits intomunkireport:mainfrom
overhacked:overhack/env_auth_conditional
Open

Add deny-if-empty feature to ENV authentication#1565
overhacked wants to merge 3 commits intomunkireport:mainfrom
overhacked:overhack/env_auth_conditional

Conversation

@overhacked
Copy link
Copy Markdown

When using ENV environment variable authentication, add option to deny authentication if the environment variable (default REMOTE_USER) is empty or missing from the environment. When a web server defaults to REMOTE_USER being empty for unauthenticated sessions, this prevents a user with "blank username" from being logged into MunkiReport. This also allows for fallback to another authentication method; for example:

  1. Try ENV auth to check if REMOTE_USER is set for users accessing MunkiReport via Tailscale Serve, which can set an authenticated username request header.
  2. Fall back to LDAP or SAML auth if users are accessing MR not via Tailscale.

Notes:

  • Disabled by default, so the previous ENV behavior is preserved, and this is not a breaking change
  • A present but blank environment variable returns unauthorized, and a missing environment variable returns failed; this would aid in debugging.

Add AUTH_ENV_DENY_EMPTY setting to cause ENV authentication to report
"unauthorized" if the value of the AUTH_ENV_USER_VAR environment
variable is empty. If the environment variable is missing altogether
from the $_SERVER environment, report "failed".

Signed-off-by: Ross Williams <[email protected]>
private $auth_status should be $authStatus, based on
usage in methods.

Signed-off-by: Ross Williams <[email protected]>
Added feature to deny auth if user environment variable is empty

Signed-off-by: Ross Williams <[email protected]>
@overhacked
Copy link
Copy Markdown
Author

I've already posted some Wiki changes related to this pull request. I apologize that the documentation got ahead of the PR; I was writing and forgot that saving to the wiki isn't subject to review. If this doesn't get merged, I'll be happy to go and revert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant