From 422ad0cc5dd7b6bdf69fde1b4fc8d143271e2958 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Thu, 23 Feb 2023 10:15:57 -0500 Subject: [PATCH] UnifiedPush: Treat non-Basic/Bearer `Authorization` header like header was not sent --- docs/releases.md | 3 ++- server/server.go | 13 +++++++++++-- server/server_test.go | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/releases.md b/docs/releases.md index 2354f8ee..498266ea 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -2,7 +2,7 @@ Binaries for all releases can be found on the GitHub releases pages for the [ntfy server](https://github.com/binwiederhier/ntfy/releases) and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/releases). -## ntfy server v2.0.2 (UNRELEASED) +## ntfy server v2.1.0 (UNRELEASED) **Features:** @@ -13,6 +13,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release **Bug fixes + maintenance:** * Web: Do not disable "Reserve topic" checkbox for admins (no ticket, thanks to @xenrox for reporting) +* UnifiedPush: Treat non-Basic/Bearer `Authorization` header like header was not sent ([#629](https://github.com/binwiederhier/ntfy/issues/629), thanks to [@Boebbele](https://github.com/Boebbele) and [@S1m](https://github.com/S1m) for reporting) **Additional languages:** diff --git a/server/server.go b/server/server.go index c4e07238..10b63607 100644 --- a/server/server.go +++ b/server/server.go @@ -1505,7 +1505,8 @@ func (s *Server) autorizeTopic(next handleFunc, perm user.Permission) handleFunc // maybeAuthenticate reads the "Authorization" header and will try to authenticate the user // if it is set. // -// - If the header is not set, an IP-based visitor is returned +// - 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 // @@ -1518,7 +1519,7 @@ func (s *Server) maybeAuthenticate(r *http.Request) (*visitor, error) { header, err := readAuthHeader(r) if err != nil { return vip, err - } else if header == "" { + } else if !supportedAuthHeader(header) { return vip, nil } else if s.userManager == nil { return vip, errHTTPUnauthorized @@ -1563,6 +1564,14 @@ func readAuthHeader(r *http.Request) (string, error) { return value, nil } +// supportedAuthHeader returns true only if the Authorization header value starts +// with "Basic" or "Bearer". In particular, an empty value is not supported, and neither +// are things like "WebPush", or "vapid" (see #629). +func supportedAuthHeader(value string) bool { + value = strings.ToLower(value) + return strings.HasPrefix(value, "basic ") || strings.HasPrefix(value, "bearer ") +} + func (s *Server) authenticateBasicAuth(r *http.Request, value string) (user *user.User, err error) { r.Header.Set("Authorization", value) username, password, ok := r.BasicAuth() diff --git a/server/server_test.go b/server/server_test.go index 4abff399..fddfbba4 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -796,6 +796,25 @@ func TestServer_Auth_ViaQuery(t *testing.T) { require.Equal(t, 401, response.Code) } +func TestServer_Auth_NonBasicHeader(t *testing.T) { + s := newTestServer(t, newTestConfigWithAuthFile(t)) + + response := request(t, s, "PUT", "/mytopic", "test", map[string]string{ + "Authorization": "WebPush not-supported", + }) + require.Equal(t, 200, response.Code) + + response = request(t, s, "PUT", "/mytopic", "test", map[string]string{ + "Authorization": "Bearer supported", + }) + require.Equal(t, 401, response.Code) + + response = request(t, s, "PUT", "/mytopic", "test", map[string]string{ + "Authorization": "basic supported", + }) + require.Equal(t, 401, response.Code) +} + func TestServer_StatsResetter(t *testing.T) { // This tests the stats resetter for // - an anonymous user