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

Refine user header code #816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ var flagsServe = append(
altsrc.NewStringFlag(&cli.StringFlag{Name: "auth-file", Aliases: []string{"auth_file", "H"}, EnvVars: []string{"NTFY_AUTH_FILE"}, Usage: "auth database file used for access control"}),
altsrc.NewStringFlag(&cli.StringFlag{Name: "auth-startup-queries", Aliases: []string{"auth_startup_queries"}, EnvVars: []string{"NTFY_AUTH_STARTUP_QUERIES"}, Usage: "queries run when the auth database is initialized"}),
altsrc.NewStringFlag(&cli.StringFlag{Name: "auth-default-access", Aliases: []string{"auth_default_access", "p"}, EnvVars: []string{"NTFY_AUTH_DEFAULT_ACCESS"}, Value: "read-write", Usage: "default permissions if no matching entries in the auth database are found"}),
altsrc.NewStringFlag(&cli.StringFlag{Name: "auth-user-header", Aliases: []string{"auth_user_header"}, EnvVars: []string{"NTFY_AUTH_USER_HEADER"}, Usage: "HTTP header that may be used to pass an authenticated user from a proxy, e.g. X-Forwarded-User"}),
altsrc.NewStringFlag(&cli.StringFlag{Name: "attachment-cache-dir", Aliases: []string{"attachment_cache_dir"}, EnvVars: []string{"NTFY_ATTACHMENT_CACHE_DIR"}, Usage: "cache directory for attached files"}),
altsrc.NewStringFlag(&cli.StringFlag{Name: "attachment-total-size-limit", Aliases: []string{"attachment_total_size_limit", "A"}, EnvVars: []string{"NTFY_ATTACHMENT_TOTAL_SIZE_LIMIT"}, DefaultText: "5G", Usage: "limit of the on-disk attachment cache"}),
altsrc.NewStringFlag(&cli.StringFlag{Name: "attachment-file-size-limit", Aliases: []string{"attachment_file_size_limit", "Y"}, EnvVars: []string{"NTFY_ATTACHMENT_FILE_SIZE_LIMIT"}, DefaultText: "15M", Usage: "per-file attachment size limit (e.g. 300k, 2M, 100M)"}),
Expand Down Expand Up @@ -147,6 +148,7 @@ func execServe(c *cli.Context) error {
authFile := c.String("auth-file")
authStartupQueries := c.String("auth-startup-queries")
authDefaultAccess := c.String("auth-default-access")
authUserHeader := c.String("auth-user-header")
attachmentCacheDir := c.String("attachment-cache-dir")
attachmentTotalSizeLimitStr := c.String("attachment-total-size-limit")
attachmentFileSizeLimitStr := c.String("attachment-file-size-limit")
Expand Down Expand Up @@ -227,6 +229,8 @@ func execServe(c *cli.Context) error {
return errors.New("base-url and upstream-base-url cannot be identical, you'll likely want to set upstream-base-url to https://ntfy.sh, see https://ntfy.sh/docs/config/#ios-instant-notifications")
} else if authFile == "" && (enableSignup || enableLogin || enableReservations || stripeSecretKey != "") {
return errors.New("cannot set enable-signup, enable-login, enable-reserve-topics, or stripe-secret-key if auth-file is not set")
} else if authUserHeader != "" && !behindProxy {
return errors.New("if auth-user-header is set, behind-proxy must also be set; this is a security measure")
} else if enableSignup && !enableLogin {
return errors.New("cannot set enable-signup without also setting enable-login")
} else if stripeSecretKey != "" && (stripeWebhookKey == "" || baseURL == "") {
Expand Down Expand Up @@ -316,6 +320,7 @@ func execServe(c *cli.Context) error {
conf.AuthFile = authFile
conf.AuthStartupQueries = authStartupQueries
conf.AuthDefault = authDefault
conf.AuthUserHeader = authUserHeader
conf.AttachmentCacheDir = attachmentCacheDir
conf.AttachmentTotalSizeLimit = attachmentTotalSizeLimit
conf.AttachmentFileSizeLimit = attachmentFileSizeLimit
Expand Down
2 changes: 2 additions & 0 deletions server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ type Config struct {
AuthDefault user.Permission
AuthBcryptCost int
AuthStatsQueueWriterInterval time.Duration
AuthUserHeader string
AttachmentCacheDir string
AttachmentTotalSizeLimit int64
AttachmentFileSizeLimit int64
Expand Down Expand Up @@ -247,5 +248,6 @@ func NewConfig() *Config {
WebPushEmailAddress: "",
WebPushExpiryDuration: DefaultWebPushExpiryDuration,
WebPushExpiryWarningDuration: DefaultWebPushExpiryWarningDuration,
AuthUserHeader: "",
}
}
55 changes: 46 additions & 9 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1858,24 +1858,61 @@ func (s *Server) autorizeTopic(next handleFunc, perm user.Permission) handleFunc
}
}

// maybeAuthenticate reads the "Authorization" header and will try to authenticate the user
// maybeAuthenticate delegates between auth based on the Authorization header (Bearer/Basic), and auth
// based on the user-defined header (as defined in the "auth-user-header" setting). The function prefers
// the user-defined header, if both are present.
//
// This function will ALWAYS return a visitor, even if an error occurs (e.g. unauthorized), so
// that subsequent logging calls still have a visitor context.
func (s *Server) maybeAuthenticate(r *http.Request) (*visitor, error) {
ip := extractIPAddress(r, s.config.BehindProxy)
vip := s.visitor(ip, nil) // IP-based visitor
if s.userManager == nil {
return vip, nil
}
if s.config.AuthUserHeader != "" && s.config.BehindProxy {
username := r.Header.Get(s.config.AuthUserHeader) // Do not allow a query param, only a header!
if username != "" {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, but shouldn't this be located inside authenticateViaUserDefinedHeader(), to keep the abstraction layer more consistent (i.e. for the auth header authn, the header is alos read in authenticateViaUserDefinedHeader(), not in the caller function)?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did realize, that if the username is not provided, you will fall back to the default authorization, so ignore my comment.

return s.authenticateViaUserDefinedHeader(r, vip, username)
}
}
return s.authenticateViaAuthHeader(r, vip)
}

// authenticateViaUserDefinedHeader tries to authenticate the user via the header defined in the "auth-user-header"
// configuration value if it is set. The value of the passed username is used to lookup the user in the database.
// If it exists, authentication is successful.
//
// This function will ALWAYS return a visitor, even if an error occurs (e.g. unauthorized), so
// that subsequent logging calls still have a visitor context.
func (s *Server) authenticateViaUserDefinedHeader(r *http.Request, vip *visitor, username string) (*visitor, error) {
// Check the rate limiter first
if !vip.AuthAllowed() {
return vip, errHTTPTooManyRequestsLimitAuthFailure // Always return visitor, even when error occurs!
}
// Retrieve user from database; if found, we have a successful authentication
u, err := s.userManager.User(username)
if err != nil || u.Deleted {
vip.AuthFailed()
logr(r).Err(err).Debug("Authentication failed")
return vip, errHTTPUnauthorized
}
// User was found, meaning that auth was successful
return s.visitor(vip.ip, u), nil
}

// authenticateViaAuthHeader reads the "Authorization" header and will try to authenticate the user
// if it is set.
//
// - If auth-file is not configured, immediately return an IP-based visitor
// - If the header is not set or not supported (anything non-Basic and non-Bearer),
// an IP-based visitor is returned
// - If the header is set, authenticate will be called to check the username/password (Basic auth),
// or the token (Bearer auth), and read the user from the database
//
// This function will ALWAYS return a visitor, even if an error occurs (e.g. unauthorized), so
// that subsequent logging calls still have a visitor context.
func (s *Server) maybeAuthenticate(r *http.Request) (*visitor, error) {
func (s *Server) authenticateViaAuthHeader(r *http.Request, vip *visitor) (*visitor, error) {
// Read "Authorization" header value, and exit out early if it's not set
ip := extractIPAddress(r, s.config.BehindProxy)
vip := s.visitor(ip, nil)
if s.userManager == nil {
return vip, nil
}
header, err := readAuthHeader(r)
if err != nil {
return vip, err
Expand All @@ -1893,7 +1930,7 @@ func (s *Server) maybeAuthenticate(r *http.Request) (*visitor, error) {
return vip, errHTTPUnauthorized // Always return visitor, even when error occurs!
}
// Authentication with user was successful
return s.visitor(ip, u), nil
return s.visitor(vip.ip, u), nil
}

// authenticate a user based on basic auth username/password (Authorization: Basic ...), or token auth (Authorization: Bearer ...).
Expand Down
14 changes: 14 additions & 0 deletions server/server.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,20 @@
# auth-default-access: "read-write"
# auth-startup-queries:

# If set, the value of the defined header will be used as an authenticated user (DANGER DANGER!).
#
# For instance, if "auth-user-header: X-Forwarded-User", a request from a client (or reverse proxy)
# with the header "X-Forwarded-User: myuser" would be authenticated as the user "myuser" without any
# further password checking.
#
# This is useful to integrate ntfy with other authentication systems such as Authelia,
# or Keycloak. This setting can only be set if "behind-proxy" is also set.
#
# WARNING: Be sure that your proxy or auth system manages the defined header, and that attackers
# cannot just pass it manually. Otherwise, they can impersonate any user!
#
# auth-user-header:

# If set, the X-Forwarded-For header is used to determine the visitor IP address
# instead of the remote address of the connection.
#
Expand Down