From 7126af6d7ca0565a2d3cd83974978d3ca99d0547 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 9 Jul 2023 21:17:34 -0400 Subject: [PATCH] Refine user header code --- cmd/serve.go | 5 +++++ server/config.go | 2 ++ server/server.go | 55 +++++++++++++++++++++++++++++++++++++++-------- server/server.yml | 14 ++++++++++++ 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 87b83dda..c5498d7a 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -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)"}), @@ -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") @@ -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 == "") { @@ -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 diff --git a/server/config.go b/server/config.go index 9815aa88..9b0e0229 100644 --- a/server/config.go +++ b/server/config.go @@ -92,6 +92,7 @@ type Config struct { AuthDefault user.Permission AuthBcryptCost int AuthStatsQueueWriterInterval time.Duration + AuthUserHeader string AttachmentCacheDir string AttachmentTotalSizeLimit int64 AttachmentFileSizeLimit int64 @@ -247,5 +248,6 @@ func NewConfig() *Config { WebPushEmailAddress: "", WebPushExpiryDuration: DefaultWebPushExpiryDuration, WebPushExpiryWarningDuration: DefaultWebPushExpiryWarningDuration, + AuthUserHeader: "", } } diff --git a/server/server.go b/server/server.go index 0ab36524..02b33e77 100644 --- a/server/server.go +++ b/server/server.go @@ -1858,10 +1858,52 @@ 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 != "" { + 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), @@ -1869,13 +1911,8 @@ func (s *Server) autorizeTopic(next handleFunc, perm user.Permission) handleFunc // // 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 @@ -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 ...). diff --git a/server/server.yml b/server/server.yml index 6b2fc989..47530f1d 100644 --- a/server/server.yml +++ b/server/server.yml @@ -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. #