2024-01-04 10:07:12 +00:00
|
|
|
// Copyright (C) 2024 The Syncthing Authors.
|
|
|
|
//
|
|
|
|
// This Source Code Form is subject to the terms of the Mozilla Public
|
|
|
|
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
|
|
|
|
// You can obtain one at https://mozilla.org/MPL/2.0/.
|
|
|
|
|
|
|
|
package api
|
|
|
|
|
|
|
|
import (
|
lib/api: Extract session store (#9425)
This is an extract from PR #9175, which can be reviewed in isolation to
reduce the volume of changes to review all at once in #9175. There are
about to be several services and API handlers that read and set cookies
and session state, so this abstraction will prove helpful.
In particular a motivating cause for this is that with the current
architecture in PR #9175, in `api.go` the [`webauthnService` needs to
access the
session](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dR309-R310)
for authentication purposes but needs to be instantiated before the
`configMuxBuilder` for config purposes, because the WebAuthn additions
to config management need to perform WebAuthn registration ceremonies,
but currently the session management is embedded in the
`basicAuthAndSessionMiddleware` which is [instantiated much
later](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dL371-R380)
and only if authentication is enabled in `guiCfg`. This refactorization
extracts the session management out from `basicAuthAndSessionMiddleware`
so that `basicAuthAndSessionMiddleware` and `webauthnService` can both
use the same shared session management service to perform session
management logic.
### Testing
This is a refactorization intended to not change any externally
observable behaviour, so existing tests (e.g., `api_auth_test.go`)
should cover this where appropriate. I have manually verified that:
- Appending `+ "foo"` to the cookie name in `createSession` causes
`TestHtmlFormLogin/invalid_URL_returns_403_before_auth_and_404_after_auth`
and `TestHtmlFormLogin/UTF-8_auth_works` to fail
- Inverting the return value of `hasValidSession` cases a whole bunch of
tests in `TestHTTPLogin` and `TestHtmlFormLogin` to fail
- (Fixed) Changing the cookie to `MaxAge: 1000` in `destroySession` does
NOT cause any tests to fail!
- Added tests `TestHtmlFormLogin/Logout_removes_the_session_cookie`,
`TestHTTPLogin/*/Logout_removes_the_session_cookie`,
`TestHtmlFormLogin/Session_cookie_is_invalid_after_logout` and
`TestHTTPLogin/200_path#01/Session_cookie_is_invalid_after_logout` to
cover this.
- Manually verified that these tests pass both before and after the
changes in this PR, and that changing the cookie to `MaxAge: 1000` or
not calling `m.tokens.Delete(cookie.Value)` in `destroySession` makes
the respective pair of tests fail.
2024-03-21 12:09:47 +00:00
|
|
|
"net/http"
|
2024-02-10 20:02:42 +00:00
|
|
|
"slices"
|
lib/api: Extract session store (#9425)
This is an extract from PR #9175, which can be reviewed in isolation to
reduce the volume of changes to review all at once in #9175. There are
about to be several services and API handlers that read and set cookies
and session state, so this abstraction will prove helpful.
In particular a motivating cause for this is that with the current
architecture in PR #9175, in `api.go` the [`webauthnService` needs to
access the
session](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dR309-R310)
for authentication purposes but needs to be instantiated before the
`configMuxBuilder` for config purposes, because the WebAuthn additions
to config management need to perform WebAuthn registration ceremonies,
but currently the session management is embedded in the
`basicAuthAndSessionMiddleware` which is [instantiated much
later](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dL371-R380)
and only if authentication is enabled in `guiCfg`. This refactorization
extracts the session management out from `basicAuthAndSessionMiddleware`
so that `basicAuthAndSessionMiddleware` and `webauthnService` can both
use the same shared session management service to perform session
management logic.
### Testing
This is a refactorization intended to not change any externally
observable behaviour, so existing tests (e.g., `api_auth_test.go`)
should cover this where appropriate. I have manually verified that:
- Appending `+ "foo"` to the cookie name in `createSession` causes
`TestHtmlFormLogin/invalid_URL_returns_403_before_auth_and_404_after_auth`
and `TestHtmlFormLogin/UTF-8_auth_works` to fail
- Inverting the return value of `hasValidSession` cases a whole bunch of
tests in `TestHTTPLogin` and `TestHtmlFormLogin` to fail
- (Fixed) Changing the cookie to `MaxAge: 1000` in `destroySession` does
NOT cause any tests to fail!
- Added tests `TestHtmlFormLogin/Logout_removes_the_session_cookie`,
`TestHTTPLogin/*/Logout_removes_the_session_cookie`,
`TestHtmlFormLogin/Session_cookie_is_invalid_after_logout` and
`TestHTTPLogin/200_path#01/Session_cookie_is_invalid_after_logout` to
cover this.
- Manually verified that these tests pass both before and after the
changes in this PR, and that changing the cookie to `MaxAge: 1000` or
not calling `m.tokens.Delete(cookie.Value)` in `destroySession` makes
the respective pair of tests fail.
2024-03-21 12:09:47 +00:00
|
|
|
"strings"
|
2024-01-04 10:07:12 +00:00
|
|
|
"time"
|
|
|
|
|
lib/api: Extract session store (#9425)
This is an extract from PR #9175, which can be reviewed in isolation to
reduce the volume of changes to review all at once in #9175. There are
about to be several services and API handlers that read and set cookies
and session state, so this abstraction will prove helpful.
In particular a motivating cause for this is that with the current
architecture in PR #9175, in `api.go` the [`webauthnService` needs to
access the
session](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dR309-R310)
for authentication purposes but needs to be instantiated before the
`configMuxBuilder` for config purposes, because the WebAuthn additions
to config management need to perform WebAuthn registration ceremonies,
but currently the session management is embedded in the
`basicAuthAndSessionMiddleware` which is [instantiated much
later](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dL371-R380)
and only if authentication is enabled in `guiCfg`. This refactorization
extracts the session management out from `basicAuthAndSessionMiddleware`
so that `basicAuthAndSessionMiddleware` and `webauthnService` can both
use the same shared session management service to perform session
management logic.
### Testing
This is a refactorization intended to not change any externally
observable behaviour, so existing tests (e.g., `api_auth_test.go`)
should cover this where appropriate. I have manually verified that:
- Appending `+ "foo"` to the cookie name in `createSession` causes
`TestHtmlFormLogin/invalid_URL_returns_403_before_auth_and_404_after_auth`
and `TestHtmlFormLogin/UTF-8_auth_works` to fail
- Inverting the return value of `hasValidSession` cases a whole bunch of
tests in `TestHTTPLogin` and `TestHtmlFormLogin` to fail
- (Fixed) Changing the cookie to `MaxAge: 1000` in `destroySession` does
NOT cause any tests to fail!
- Added tests `TestHtmlFormLogin/Logout_removes_the_session_cookie`,
`TestHTTPLogin/*/Logout_removes_the_session_cookie`,
`TestHtmlFormLogin/Session_cookie_is_invalid_after_logout` and
`TestHTTPLogin/200_path#01/Session_cookie_is_invalid_after_logout` to
cover this.
- Manually verified that these tests pass both before and after the
changes in this PR, and that changing the cookie to `MaxAge: 1000` or
not calling `m.tokens.Delete(cookie.Value)` in `destroySession` makes
the respective pair of tests fail.
2024-03-21 12:09:47 +00:00
|
|
|
"github.com/syncthing/syncthing/lib/config"
|
2024-01-04 10:07:12 +00:00
|
|
|
"github.com/syncthing/syncthing/lib/db"
|
lib/api: Extract session store (#9425)
This is an extract from PR #9175, which can be reviewed in isolation to
reduce the volume of changes to review all at once in #9175. There are
about to be several services and API handlers that read and set cookies
and session state, so this abstraction will prove helpful.
In particular a motivating cause for this is that with the current
architecture in PR #9175, in `api.go` the [`webauthnService` needs to
access the
session](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dR309-R310)
for authentication purposes but needs to be instantiated before the
`configMuxBuilder` for config purposes, because the WebAuthn additions
to config management need to perform WebAuthn registration ceremonies,
but currently the session management is embedded in the
`basicAuthAndSessionMiddleware` which is [instantiated much
later](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dL371-R380)
and only if authentication is enabled in `guiCfg`. This refactorization
extracts the session management out from `basicAuthAndSessionMiddleware`
so that `basicAuthAndSessionMiddleware` and `webauthnService` can both
use the same shared session management service to perform session
management logic.
### Testing
This is a refactorization intended to not change any externally
observable behaviour, so existing tests (e.g., `api_auth_test.go`)
should cover this where appropriate. I have manually verified that:
- Appending `+ "foo"` to the cookie name in `createSession` causes
`TestHtmlFormLogin/invalid_URL_returns_403_before_auth_and_404_after_auth`
and `TestHtmlFormLogin/UTF-8_auth_works` to fail
- Inverting the return value of `hasValidSession` cases a whole bunch of
tests in `TestHTTPLogin` and `TestHtmlFormLogin` to fail
- (Fixed) Changing the cookie to `MaxAge: 1000` in `destroySession` does
NOT cause any tests to fail!
- Added tests `TestHtmlFormLogin/Logout_removes_the_session_cookie`,
`TestHTTPLogin/*/Logout_removes_the_session_cookie`,
`TestHtmlFormLogin/Session_cookie_is_invalid_after_logout` and
`TestHTTPLogin/200_path#01/Session_cookie_is_invalid_after_logout` to
cover this.
- Manually verified that these tests pass both before and after the
changes in this PR, and that changing the cookie to `MaxAge: 1000` or
not calling `m.tokens.Delete(cookie.Value)` in `destroySession` makes
the respective pair of tests fail.
2024-03-21 12:09:47 +00:00
|
|
|
"github.com/syncthing/syncthing/lib/events"
|
2024-01-04 10:07:12 +00:00
|
|
|
"github.com/syncthing/syncthing/lib/rand"
|
|
|
|
"github.com/syncthing/syncthing/lib/sync"
|
|
|
|
)
|
|
|
|
|
|
|
|
type tokenManager struct {
|
|
|
|
key string
|
|
|
|
miscDB *db.NamespacedKV
|
|
|
|
lifetime time.Duration
|
|
|
|
maxItems int
|
|
|
|
|
|
|
|
timeNow func() time.Time // can be overridden for testing
|
|
|
|
|
|
|
|
mut sync.Mutex
|
|
|
|
tokens *TokenSet
|
|
|
|
saveTimer *time.Timer
|
|
|
|
}
|
|
|
|
|
|
|
|
func newTokenManager(key string, miscDB *db.NamespacedKV, lifetime time.Duration, maxItems int) *tokenManager {
|
|
|
|
tokens := &TokenSet{
|
|
|
|
Tokens: make(map[string]int64),
|
|
|
|
}
|
|
|
|
if bs, ok, _ := miscDB.Bytes(key); ok {
|
|
|
|
_ = tokens.Unmarshal(bs) // best effort
|
|
|
|
}
|
|
|
|
return &tokenManager{
|
|
|
|
key: key,
|
|
|
|
miscDB: miscDB,
|
|
|
|
lifetime: lifetime,
|
|
|
|
maxItems: maxItems,
|
|
|
|
timeNow: time.Now,
|
|
|
|
mut: sync.NewMutex(),
|
|
|
|
tokens: tokens,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Check returns true if the token is valid, and updates the token's expiry
|
|
|
|
// time. The token is removed if it is expired.
|
|
|
|
func (m *tokenManager) Check(token string) bool {
|
|
|
|
m.mut.Lock()
|
|
|
|
defer m.mut.Unlock()
|
|
|
|
|
|
|
|
expires, ok := m.tokens.Tokens[token]
|
|
|
|
if ok {
|
|
|
|
if expires < m.timeNow().UnixNano() {
|
|
|
|
// The token is expired.
|
|
|
|
m.saveLocked() // removes expired tokens
|
|
|
|
return false
|
|
|
|
}
|
|
|
|
|
|
|
|
// Give the token further life.
|
|
|
|
m.tokens.Tokens[token] = m.timeNow().Add(m.lifetime).UnixNano()
|
|
|
|
m.saveLocked()
|
|
|
|
}
|
|
|
|
return ok
|
|
|
|
}
|
|
|
|
|
|
|
|
// New creates a new token and returns it.
|
|
|
|
func (m *tokenManager) New() string {
|
|
|
|
token := rand.String(randomTokenLength)
|
|
|
|
|
|
|
|
m.mut.Lock()
|
|
|
|
defer m.mut.Unlock()
|
|
|
|
|
|
|
|
m.tokens.Tokens[token] = m.timeNow().Add(m.lifetime).UnixNano()
|
|
|
|
m.saveLocked()
|
|
|
|
|
|
|
|
return token
|
|
|
|
}
|
|
|
|
|
|
|
|
// Delete removes a token.
|
|
|
|
func (m *tokenManager) Delete(token string) {
|
|
|
|
m.mut.Lock()
|
|
|
|
defer m.mut.Unlock()
|
|
|
|
|
|
|
|
delete(m.tokens.Tokens, token)
|
|
|
|
m.saveLocked()
|
|
|
|
}
|
|
|
|
|
|
|
|
func (m *tokenManager) saveLocked() {
|
|
|
|
// Remove expired tokens.
|
|
|
|
now := m.timeNow().UnixNano()
|
|
|
|
for token, expiry := range m.tokens.Tokens {
|
|
|
|
if expiry < now {
|
|
|
|
delete(m.tokens.Tokens, token)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// If we have a limit on the number of tokens, remove the oldest ones.
|
|
|
|
if m.maxItems > 0 && len(m.tokens.Tokens) > m.maxItems {
|
|
|
|
// Sort the tokens by expiry time, oldest first.
|
|
|
|
type tokenExpiry struct {
|
|
|
|
token string
|
|
|
|
expiry int64
|
|
|
|
}
|
|
|
|
var tokens []tokenExpiry
|
|
|
|
for token, expiry := range m.tokens.Tokens {
|
|
|
|
tokens = append(tokens, tokenExpiry{token, expiry})
|
|
|
|
}
|
|
|
|
slices.SortFunc(tokens, func(i, j tokenExpiry) int {
|
|
|
|
return int(i.expiry - j.expiry)
|
|
|
|
})
|
|
|
|
// Remove the oldest tokens.
|
|
|
|
for _, token := range tokens[:len(tokens)-m.maxItems] {
|
|
|
|
delete(m.tokens.Tokens, token.token)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Postpone saving until one second of inactivity.
|
|
|
|
if m.saveTimer == nil {
|
|
|
|
m.saveTimer = time.AfterFunc(time.Second, m.scheduledSave)
|
|
|
|
} else {
|
|
|
|
m.saveTimer.Reset(time.Second)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func (m *tokenManager) scheduledSave() {
|
|
|
|
m.mut.Lock()
|
|
|
|
defer m.mut.Unlock()
|
|
|
|
|
|
|
|
m.saveTimer = nil
|
|
|
|
|
|
|
|
bs, _ := m.tokens.Marshal() // can't fail
|
|
|
|
_ = m.miscDB.PutBytes(m.key, bs) // can fail, but what are we going to do?
|
|
|
|
}
|
lib/api: Extract session store (#9425)
This is an extract from PR #9175, which can be reviewed in isolation to
reduce the volume of changes to review all at once in #9175. There are
about to be several services and API handlers that read and set cookies
and session state, so this abstraction will prove helpful.
In particular a motivating cause for this is that with the current
architecture in PR #9175, in `api.go` the [`webauthnService` needs to
access the
session](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dR309-R310)
for authentication purposes but needs to be instantiated before the
`configMuxBuilder` for config purposes, because the WebAuthn additions
to config management need to perform WebAuthn registration ceremonies,
but currently the session management is embedded in the
`basicAuthAndSessionMiddleware` which is [instantiated much
later](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dL371-R380)
and only if authentication is enabled in `guiCfg`. This refactorization
extracts the session management out from `basicAuthAndSessionMiddleware`
so that `basicAuthAndSessionMiddleware` and `webauthnService` can both
use the same shared session management service to perform session
management logic.
### Testing
This is a refactorization intended to not change any externally
observable behaviour, so existing tests (e.g., `api_auth_test.go`)
should cover this where appropriate. I have manually verified that:
- Appending `+ "foo"` to the cookie name in `createSession` causes
`TestHtmlFormLogin/invalid_URL_returns_403_before_auth_and_404_after_auth`
and `TestHtmlFormLogin/UTF-8_auth_works` to fail
- Inverting the return value of `hasValidSession` cases a whole bunch of
tests in `TestHTTPLogin` and `TestHtmlFormLogin` to fail
- (Fixed) Changing the cookie to `MaxAge: 1000` in `destroySession` does
NOT cause any tests to fail!
- Added tests `TestHtmlFormLogin/Logout_removes_the_session_cookie`,
`TestHTTPLogin/*/Logout_removes_the_session_cookie`,
`TestHtmlFormLogin/Session_cookie_is_invalid_after_logout` and
`TestHTTPLogin/200_path#01/Session_cookie_is_invalid_after_logout` to
cover this.
- Manually verified that these tests pass both before and after the
changes in this PR, and that changing the cookie to `MaxAge: 1000` or
not calling `m.tokens.Delete(cookie.Value)` in `destroySession` makes
the respective pair of tests fail.
2024-03-21 12:09:47 +00:00
|
|
|
|
|
|
|
type tokenCookieManager struct {
|
|
|
|
cookieName string
|
|
|
|
shortID string
|
|
|
|
guiCfg config.GUIConfiguration
|
|
|
|
evLogger events.Logger
|
|
|
|
tokens *tokenManager
|
|
|
|
}
|
|
|
|
|
|
|
|
func newTokenCookieManager(shortID string, guiCfg config.GUIConfiguration, evLogger events.Logger, miscDB *db.NamespacedKV) *tokenCookieManager {
|
|
|
|
return &tokenCookieManager{
|
|
|
|
cookieName: "sessionid-" + shortID,
|
|
|
|
shortID: shortID,
|
|
|
|
guiCfg: guiCfg,
|
|
|
|
evLogger: evLogger,
|
|
|
|
tokens: newTokenManager("sessions", miscDB, maxSessionLifetime, maxActiveSessions),
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func (m *tokenCookieManager) createSession(username string, persistent bool, w http.ResponseWriter, r *http.Request) {
|
|
|
|
sessionid := m.tokens.New()
|
|
|
|
|
|
|
|
// Best effort detection of whether the connection is HTTPS --
|
|
|
|
// either directly to us, or as used by the client towards a reverse
|
|
|
|
// proxy who sends us headers.
|
|
|
|
connectionIsHTTPS := r.TLS != nil ||
|
|
|
|
strings.ToLower(r.Header.Get("x-forwarded-proto")) == "https" ||
|
|
|
|
strings.Contains(strings.ToLower(r.Header.Get("forwarded")), "proto=https")
|
|
|
|
// If the connection is HTTPS, or *should* be HTTPS, set the Secure
|
|
|
|
// bit in cookies.
|
|
|
|
useSecureCookie := connectionIsHTTPS || m.guiCfg.UseTLS()
|
|
|
|
|
|
|
|
maxAge := 0
|
|
|
|
if persistent {
|
|
|
|
maxAge = int(maxSessionLifetime.Seconds())
|
|
|
|
}
|
|
|
|
http.SetCookie(w, &http.Cookie{
|
|
|
|
Name: m.cookieName,
|
|
|
|
Value: sessionid,
|
|
|
|
// In HTTP spec Max-Age <= 0 means delete immediately,
|
|
|
|
// but in http.Cookie MaxAge = 0 means unspecified (session) and MaxAge < 0 means delete immediately
|
|
|
|
MaxAge: maxAge,
|
|
|
|
Secure: useSecureCookie,
|
|
|
|
Path: "/",
|
|
|
|
})
|
|
|
|
|
|
|
|
emitLoginAttempt(true, username, r.RemoteAddr, m.evLogger)
|
|
|
|
}
|
|
|
|
|
|
|
|
func (m *tokenCookieManager) hasValidSession(r *http.Request) bool {
|
|
|
|
for _, cookie := range r.Cookies() {
|
|
|
|
// We iterate here since there may, historically, be multiple
|
|
|
|
// cookies with the same name but different path. Any "old" ones
|
|
|
|
// won't match an existing session and will be ignored, then
|
|
|
|
// later removed on logout or when timing out.
|
|
|
|
if cookie.Name == m.cookieName {
|
|
|
|
if m.tokens.Check(cookie.Value) {
|
|
|
|
return true
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return false
|
|
|
|
}
|
|
|
|
|
|
|
|
func (m *tokenCookieManager) destroySession(w http.ResponseWriter, r *http.Request) {
|
|
|
|
for _, cookie := range r.Cookies() {
|
|
|
|
// We iterate here since there may, historically, be multiple
|
|
|
|
// cookies with the same name but different path. We drop them
|
|
|
|
// all.
|
|
|
|
if cookie.Name == m.cookieName {
|
|
|
|
m.tokens.Delete(cookie.Value)
|
|
|
|
|
|
|
|
// Create a cookie deletion command
|
|
|
|
http.SetCookie(w, &http.Cookie{
|
|
|
|
Name: m.cookieName,
|
|
|
|
Value: "",
|
|
|
|
MaxAge: -1,
|
|
|
|
Secure: cookie.Secure,
|
|
|
|
Path: cookie.Path,
|
|
|
|
})
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|