UnifiedPush: Treat non-Basic/Bearer Authorization header like header was not sent

This commit is contained in:
binwiederhier 2023-02-23 10:15:57 -05:00
parent 2ac63c4327
commit 422ad0cc5d
3 changed files with 32 additions and 3 deletions

View file

@ -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) 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). 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:** **Features:**
@ -13,6 +13,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
**Bug fixes + maintenance:** **Bug fixes + maintenance:**
* Web: Do not disable "Reserve topic" checkbox for admins (no ticket, thanks to @xenrox for reporting) * 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:** **Additional languages:**

View file

@ -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 // maybeAuthenticate reads the "Authorization" header and will try to authenticate the user
// if it is set. // 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), // - 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 // 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) header, err := readAuthHeader(r)
if err != nil { if err != nil {
return vip, err return vip, err
} else if header == "" { } else if !supportedAuthHeader(header) {
return vip, nil return vip, nil
} else if s.userManager == nil { } else if s.userManager == nil {
return vip, errHTTPUnauthorized return vip, errHTTPUnauthorized
@ -1563,6 +1564,14 @@ func readAuthHeader(r *http.Request) (string, error) {
return value, nil 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) { func (s *Server) authenticateBasicAuth(r *http.Request, value string) (user *user.User, err error) {
r.Header.Set("Authorization", value) r.Header.Set("Authorization", value)
username, password, ok := r.BasicAuth() username, password, ok := r.BasicAuth()

View file

@ -796,6 +796,25 @@ func TestServer_Auth_ViaQuery(t *testing.T) {
require.Equal(t, 401, response.Code) 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) { func TestServer_StatsResetter(t *testing.T) {
// This tests the stats resetter for // This tests the stats resetter for
// - an anonymous user // - an anonymous user