Merge 5f0c30a151 into abe7275f0c
				
					
				
			This commit is contained in:
		
						commit
						7722ae635a
					
				
					 5 changed files with 122 additions and 11 deletions
				
			
		|  | @ -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-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-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-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-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-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)"}), | 	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") | 	authFile := c.String("auth-file") | ||||||
| 	authStartupQueries := c.String("auth-startup-queries") | 	authStartupQueries := c.String("auth-startup-queries") | ||||||
| 	authDefaultAccess := c.String("auth-default-access") | 	authDefaultAccess := c.String("auth-default-access") | ||||||
|  | 	authUserHeader := c.String("auth-user-header") | ||||||
| 	attachmentCacheDir := c.String("attachment-cache-dir") | 	attachmentCacheDir := c.String("attachment-cache-dir") | ||||||
| 	attachmentTotalSizeLimitStr := c.String("attachment-total-size-limit") | 	attachmentTotalSizeLimitStr := c.String("attachment-total-size-limit") | ||||||
| 	attachmentFileSizeLimitStr := c.String("attachment-file-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") | 		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 != "") { | 	} 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") | 		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 { | 	} else if enableSignup && !enableLogin { | ||||||
| 		return errors.New("cannot set enable-signup without also setting enable-login") | 		return errors.New("cannot set enable-signup without also setting enable-login") | ||||||
| 	} else if stripeSecretKey != "" && (stripeWebhookKey == "" || baseURL == "") { | 	} else if stripeSecretKey != "" && (stripeWebhookKey == "" || baseURL == "") { | ||||||
|  | @ -316,6 +320,7 @@ func execServe(c *cli.Context) error { | ||||||
| 	conf.AuthFile = authFile | 	conf.AuthFile = authFile | ||||||
| 	conf.AuthStartupQueries = authStartupQueries | 	conf.AuthStartupQueries = authStartupQueries | ||||||
| 	conf.AuthDefault = authDefault | 	conf.AuthDefault = authDefault | ||||||
|  | 	conf.AuthUserHeader = authUserHeader | ||||||
| 	conf.AttachmentCacheDir = attachmentCacheDir | 	conf.AttachmentCacheDir = attachmentCacheDir | ||||||
| 	conf.AttachmentTotalSizeLimit = attachmentTotalSizeLimit | 	conf.AttachmentTotalSizeLimit = attachmentTotalSizeLimit | ||||||
| 	conf.AttachmentFileSizeLimit = attachmentFileSizeLimit | 	conf.AttachmentFileSizeLimit = attachmentFileSizeLimit | ||||||
|  |  | ||||||
|  | @ -92,6 +92,7 @@ type Config struct { | ||||||
| 	AuthDefault                          user.Permission | 	AuthDefault                          user.Permission | ||||||
| 	AuthBcryptCost                       int | 	AuthBcryptCost                       int | ||||||
| 	AuthStatsQueueWriterInterval         time.Duration | 	AuthStatsQueueWriterInterval         time.Duration | ||||||
|  | 	AuthUserHeader                       string | ||||||
| 	AttachmentCacheDir                   string | 	AttachmentCacheDir                   string | ||||||
| 	AttachmentTotalSizeLimit             int64 | 	AttachmentTotalSizeLimit             int64 | ||||||
| 	AttachmentFileSizeLimit              int64 | 	AttachmentFileSizeLimit              int64 | ||||||
|  | @ -247,5 +248,6 @@ func NewConfig() *Config { | ||||||
| 		WebPushEmailAddress:                  "", | 		WebPushEmailAddress:                  "", | ||||||
| 		WebPushExpiryDuration:                DefaultWebPushExpiryDuration, | 		WebPushExpiryDuration:                DefaultWebPushExpiryDuration, | ||||||
| 		WebPushExpiryWarningDuration:         DefaultWebPushExpiryWarningDuration, | 		WebPushExpiryWarningDuration:         DefaultWebPushExpiryWarningDuration, | ||||||
|  | 		AuthUserHeader:                       "", | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -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 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), | //   - If the header is not set or not supported (anything non-Basic and non-Bearer), | ||||||
| //     an IP-based visitor is returned | //     an IP-based visitor is returned | ||||||
| //   - If the header is set, authenticate will be called to check the username/password (Basic auth), | //   - 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 | // 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. | // 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 | 	// 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) | 	header, err := readAuthHeader(r) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return vip, err | 		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! | 		return vip, errHTTPUnauthorized // Always return visitor, even when error occurs! | ||||||
| 	} | 	} | ||||||
| 	// Authentication with user was successful | 	// 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 ...). | // authenticate a user based on basic auth username/password (Authorization: Basic ...), or token auth (Authorization: Bearer ...). | ||||||
|  |  | ||||||
|  | @ -95,6 +95,20 @@ | ||||||
| # auth-default-access: "read-write" | # auth-default-access: "read-write" | ||||||
| # auth-startup-queries: | # 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 | # If set, the X-Forwarded-For header is used to determine the visitor IP address | ||||||
| # instead of the remote address of the connection. | # instead of the remote address of the connection. | ||||||
| # | # | ||||||
|  |  | ||||||
|  | @ -6,8 +6,6 @@ import ( | ||||||
| 	"encoding/base64" | 	"encoding/base64" | ||||||
| 	"encoding/json" | 	"encoding/json" | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"golang.org/x/crypto/bcrypt" |  | ||||||
| 	"heckel.io/ntfy/user" |  | ||||||
| 	"io" | 	"io" | ||||||
| 	"math/rand" | 	"math/rand" | ||||||
| 	"net/http" | 	"net/http" | ||||||
|  | @ -22,6 +20,9 @@ import ( | ||||||
| 	"testing" | 	"testing" | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
|  | 	"golang.org/x/crypto/bcrypt" | ||||||
|  | 	"heckel.io/ntfy/user" | ||||||
|  | 
 | ||||||
| 	"github.com/SherClockHolmes/webpush-go" | 	"github.com/SherClockHolmes/webpush-go" | ||||||
| 	"github.com/stretchr/testify/require" | 	"github.com/stretchr/testify/require" | ||||||
| 	"heckel.io/ntfy/log" | 	"heckel.io/ntfy/log" | ||||||
|  | @ -777,6 +778,58 @@ func TestServer_SubscribeWithQueryFilters(t *testing.T) { | ||||||
| 	require.Equal(t, keepaliveEvent, messages[2].Event) | 	require.Equal(t, keepaliveEvent, messages[2].Event) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | func TestServer_User_Auth_Success_Admin(t *testing.T) { | ||||||
|  | 	c := newTestConfigWithAuthFile(t) | ||||||
|  | 	header := "X-User-Header" | ||||||
|  | 	c.AuthUserHeader = header | ||||||
|  | 	c.BehindProxy = true | ||||||
|  | 	s := newTestServer(t, c) | ||||||
|  | 
 | ||||||
|  | 	require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) | ||||||
|  | 
 | ||||||
|  | 	response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{ | ||||||
|  | 		header: "phil", | ||||||
|  | 	}) | ||||||
|  | 	require.Equal(t, 200, response.Code) | ||||||
|  | 	require.Equal(t, `{"success":true}`+"\n", response.Body.String()) | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | func TestServer_User_Auth_Unknown_Admin(t *testing.T) { | ||||||
|  | 	c := newTestConfigWithAuthFile(t) | ||||||
|  | 	header := "X-User-Header" | ||||||
|  | 	c.AuthUserHeader = header | ||||||
|  | 	c.BehindProxy = true | ||||||
|  | 	s := newTestServer(t, c) | ||||||
|  | 
 | ||||||
|  | 	response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{ | ||||||
|  | 		header: "unknown", | ||||||
|  | 	}) | ||||||
|  | 	require.Equal(t, 401, response.Code) | ||||||
|  | 	require.Equal(t, 40101, toHTTPError(t, response.Body.String()).Code) | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | func TestServer_User_Auth_Fail_Rate_Limit(t *testing.T) { | ||||||
|  | 	c := newTestConfigWithAuthFile(t) | ||||||
|  | 	header := "X-User-Header" | ||||||
|  | 	c.AuthUserHeader = header | ||||||
|  | 	c.BehindProxy = true | ||||||
|  | 	c.VisitorAuthFailureLimitBurst = 10 | ||||||
|  | 	s := newTestServer(t, c) | ||||||
|  | 
 | ||||||
|  | 	for i := 0; i < 10; i++ { | ||||||
|  | 		response := request(t, s, "PUT", "/announcements", "test", map[string]string{ | ||||||
|  | 			header: "phil", | ||||||
|  | 		}) | ||||||
|  | 		require.Equal(t, 401, response.Code) | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	response := request(t, s, "PUT", "/announcements", "test", map[string]string{ | ||||||
|  | 		header: "phil", | ||||||
|  | 	}) | ||||||
|  | 	require.Equal(t, 429, response.Code) | ||||||
|  | 	require.Equal(t, 42909, toHTTPError(t, response.Body.String()).Code) | ||||||
|  | } | ||||||
|  | 
 | ||||||
| func TestServer_Auth_Success_Admin(t *testing.T) { | func TestServer_Auth_Success_Admin(t *testing.T) { | ||||||
| 	c := newTestConfigWithAuthFile(t) | 	c := newTestConfigWithAuthFile(t) | ||||||
| 	s := newTestServer(t, c) | 	s := newTestServer(t, c) | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue