fix(sessions): a password change ends the sessions opened before it

go.mod

@@ -33,7 +33,7 @@ require (
 	github.com/go-openapi/jsonpointer v0.22.5 // indirect
 	github.com/go-openapi/swag/jsonname v0.25.5 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
-	github.com/golang-jwt/jwt/v4 v4.5.2 // indirect
+	github.com/golang-jwt/jwt/v4 v4.5.2
 	github.com/golang-jwt/jwt/v5 v5.3.0 // indirect
 	github.com/google/uuid v1.5.0 // indirect
 	github.com/gorilla/mux v1.8.0 // indirect

route/v1/user.go

@@ -236,26 +236,28 @@ func PostUserLogin(ctx echo.Context) error {
 	return issueTokens(ctx, user)
 }
 
-// issueTokens mints the access and refresh tokens and builds the login body;
-// shared by /login (2FA off) and /2fa/verify (2FA on).
-func issueTokens(ctx echo.Context, user model2.UserDBModel) error {
+// newTokens is the pair of a session of user: an access token, three hours, and the refresh
+// token of the session the row is at.
+func newTokens(user model2.UserDBModel) (system_model.VerifyInformation, error) {
 	privateKey, _ := service.MyService.User().GetKeyPair()
-
-	token := system_model.VerifyInformation{}
-
-	accessToken, err := jwt.GetAccessToken(user.Username, privateKey, user.Id)
+	access, err := jwt.GetAccessToken(user.Username, privateKey, user.Id)
 	if err != nil {
-		return ctx.JSON(http.StatusInternalServerError, model.Result{Success: common_err.SERVICE_ERROR, Message: err.Error()})
+		return system_model.VerifyInformation{}, err
 	}
-	token.AccessToken = accessToken
+	refresh, err := service.MyService.User().RefreshToken(user)
+	if err != nil {
+		return system_model.VerifyInformation{}, err
+	}
+	return system_model.VerifyInformation{AccessToken: access, RefreshToken: refresh, ExpiresAt: time.Now().Add(3 * time.Hour).Unix()}, nil
+}
 
-	refreshToken, err := jwt.GetRefreshToken(user.Username, privateKey, user.Id)
+// issueTokens mints the access and refresh tokens and builds the login body;
+// shared by /login (2FA off) and /2fa/verify (2FA on).
+func issueTokens(ctx echo.Context, user model2.UserDBModel) error {
+	token, err := newTokens(user)
 	if err != nil {
 		return ctx.JSON(http.StatusInternalServerError, model.Result{Success: common_err.SERVICE_ERROR, Message: err.Error()})
 	}
-	token.RefreshToken = refreshToken
-
-	token.ExpiresAt = time.Now().Add(3 * time.Hour * time.Duration(1)).Unix()
 	data := make(map[string]interface{}, 2)
 	user.Password = ""
 	data["token"] = token
@@ -431,8 +433,14 @@ func PutUserPassword(ctx echo.Context) error {
 	if err := service.MyService.User().UpdateUserPassword(user); err != nil {
 		return ctx.JSON(common_err.SERVICE_ERROR, model.Result{Success: common_err.SERVICE_ERROR, Message: common_err.GetMsg(common_err.SERVICE_ERROR), Data: err.Error()})
 	}
-	user.Password = ""
-	return ctx.JSON(common_err.SUCCESS, model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: user})
+	// The change ended every session of the account, this one with them: it goes on with the
+	// pair made here, of the session the row is at now. The body is the one of a login.
+	user = service.MyService.User().GetUserAllInfoById(id)
+	if user.Id == 0 { // gone since the change: no pair for an account that is not there
+		return ctx.JSON(common_err.SERVICE_ERROR,
+			model.Result{Success: common_err.USER_NOT_EXIST, Message: common_err.GetMsg(common_err.USER_NOT_EXIST)})
+	}
+	return issueTokens(ctx, user)
 }
 
 // @Summary edit user nick
@@ -827,8 +835,6 @@ func PostUserRefreshToken(ctx echo.Context) error {
 	ctx.Bind(&js)
 	refresh := js["refresh_token"]
 
-	privateKey, _ := service.MyService.User().GetKeyPair()
-
 	claims, err := jwt.ParseToken(
 		refresh,
 		func() (*ecdsa.PublicKey, error) {
@@ -842,22 +848,19 @@ func PostUserRefreshToken(ctx echo.Context) error {
 		return ctx.JSON(http.StatusUnauthorized, model.Result{Success: common_err.VERIFICATION_FAILURE, Message: common_err.GetMsg(common_err.VERIFICATION_FAILURE)})
 	}
 
-	newAccessToken, err := jwt.GetAccessToken(claims.Username, privateKey, claims.ID)
-	if err != nil {
-		return ctx.JSON(http.StatusInternalServerError, model.Result{Success: common_err.SERVICE_ERROR, Message: err.Error()})
+	// The token is only a signature: the account is read again. It may be gone, a password
+	// change since the token was made ended its session, and an account made since under the
+	// same id has sessions of its own.
+	user := service.MyService.User().GetUserAllInfoById(strconv.Itoa(claims.ID))
+	if user.Id == 0 || service.SessionOf(claims) != user.SessionVersion {
+		return ctx.JSON(http.StatusUnauthorized, model.Result{Success: common_err.VERIFICATION_FAILURE, Message: common_err.GetMsg(common_err.VERIFICATION_FAILURE)})
 	}
 
-	newRefreshToken, err := jwt.GetRefreshToken(claims.Username, privateKey, claims.ID)
+	verifyInfo, err := newTokens(user)
 	if err != nil {
 		return ctx.JSON(http.StatusInternalServerError, model.Result{Success: common_err.SERVICE_ERROR, Message: err.Error()})
 	}
 
-	verifyInfo := system_model.VerifyInformation{
-		AccessToken:  newAccessToken,
-		RefreshToken: newRefreshToken,
-		ExpiresAt:    time.Now().Add(3 * time.Hour).Unix(),
-	}
-
 	return ctx.JSON(common_err.SUCCESS, model.Result{Success: common_err.SUCCESS, Message: common_err.GetMsg(common_err.SUCCESS), Data: verifyInfo})
 }
 

route/v1_2fa_test.go

@@ -124,6 +124,13 @@ func wrongCode(t *testing.T, secret string) string {
 // the per-user limiters are process-wide, so the map is reset here: a
 // budget spent on an id in one test is not inherited by the next.
 func newRig(t *testing.T) (service.UserService, client) {
+	t.Helper()
+	users, c, _ := newRigDB(t)
+	return users, c
+}
+
+// newRigDB is newRig, with the database for a test that needs a row as an older version left it.
+func newRigDB(t *testing.T) (service.UserService, client, *gorm.DB) {
 	t.Helper()
 	logger.LogInitConsoleOnly() // the handlers log; a test run on its own has no logger otherwise
 	db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
@@ -141,7 +148,7 @@ func newRig(t *testing.T) (service.UserService, client) {
 	v1.ResetLoginLimiters()
 	v1.UserLimit = rate.Inf
 	v1.ResetUserLimiters()
-	return users, client{t: t, h: route.InitRouter()}
+	return users, client{t: t, h: route.InitRouter()}, db
 }
 
 // TestLimiterResetPerRig: a budget exhausted on user id 1 in one rig is not

route/v1_sessions_test.go

@@ -0,0 +1,176 @@
+package route_test
+
+import (
+	"net/http"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/ReCasaOS/CasaOS-Common/utils/jwt"
+	"github.com/ReCasaOS/CasaOS-UserService/pkg/utils/encryption"
+	model2 "github.com/ReCasaOS/CasaOS-UserService/service/model"
+	gojwt "github.com/golang-jwt/jwt/v4"
+)
+
+func refreshStatus(c client, refresh string) (int, result) {
+	return c.do("POST", "/v1/users/refresh", "", map[string]string{"refresh_token": refresh})
+}
+
+// A refresh token is good for seven days and for a new pair each time, so a copy of one outlived
+// everything its owner could do. Changing the password ends every session opened before it; the
+// one that changed it goes on with the pair the change hands back.
+func TestARefreshTokenStopsWorkingWhenThePasswordChanges(t *testing.T) {
+	users, c := newRig(t)
+	const password, changed = "correct horse", "battery staple"
+	users.CreateUser(model2.UserDBModel{Username: "admin", Password: encryption.HashPassword(password), Role: "admin"})
+
+	access, copied := tokens(t, c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "admin", "password": password}))
+	// the copy is used once: the pair it gives is as old as the session
+	status, res := refreshStatus(c, copied)
+	if status != http.StatusOK {
+		t.Fatalf("a refresh before the change: %d", status)
+	}
+	renewed, _ := res.Data["refresh_token"].(string)
+	if renewed == "" {
+		t.Fatalf("no refresh token in %v", res.Data)
+	}
+
+	data := c.expect(200, 200, "PUT", "/v1/users/current/password", access, map[string]string{"old_password": password, "password": changed})
+	_, mine := tokens(t, data)
+	if user, _ := data["user"].(map[string]interface{}); user["username"] != "admin" || user["password"] != nil {
+		t.Fatalf("the user of the answer: %v", data["user"])
+	}
+
+	for name, token := range map[string]string{"the copied token": copied, "the one it was renewed into": renewed} {
+		if status, _ := refreshStatus(c, token); status != http.StatusUnauthorized {
+			t.Errorf("%s after the password changed: %d, want 401", name, status)
+		}
+	}
+	status, res = refreshStatus(c, mine)
+	if status != http.StatusOK {
+		t.Fatalf("the session that changed the password: %d, want 200", status)
+	}
+	// and what it is renewed into goes on working
+	if next, _ := res.Data["refresh_token"].(string); next == "" {
+		t.Fatalf("no refresh token in %v", res.Data)
+	} else if status, _ := refreshStatus(c, next); status != http.StatusOK {
+		t.Fatalf("the renewed token of that session: %d, want 200", status)
+	}
+	c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "admin", "password": changed})
+}
+
+func TestARefreshTokenStopsWorkingWhenTheAccountIsGone(t *testing.T) {
+	users, c := newRig(t)
+	const password = "correct horse"
+	user := users.CreateUser(model2.UserDBModel{Username: "guest", Password: encryption.HashPassword(password), Role: "admin"})
+	_, refresh := tokens(t, c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "guest", "password": password}))
+
+	users.DeleteUserById(strconv.Itoa(user.Id))
+
+	if status, _ := refreshStatus(c, refresh); status != http.StatusUnauthorized {
+		t.Fatalf("the token of an account that is gone: %d, want 401", status)
+	}
+}
+
+// A box that updates keeps its sessions: a token made before this change says nothing of the
+// session it belongs to, and is one of the sessions opened before any password change.
+func TestARefreshTokenMadeBeforeSessionsWereCountedStillWorks(t *testing.T) {
+	users, c, db := newRigDB(t)
+	const password = "correct horse"
+	user := users.CreateUser(model2.UserDBModel{Username: "admin", Password: encryption.HashPassword(password), Role: "admin"})
+	// the row as the update finds it: the column is added with 0 in it
+	if err := db.Exec("UPDATE o_users SET session_version = 0").Error; err != nil {
+		t.Fatal(err)
+	}
+	private, _ := users.GetKeyPair()
+	old, err := jwt.GetRefreshToken("admin", private, user.Id)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if status, _ := refreshStatus(c, old); status != http.StatusOK {
+		t.Fatalf("a token of before the update: %d, want 200", status)
+	}
+
+	access, _ := tokens(t, c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "admin", "password": password}))
+	c.expect(200, 200, "PUT", "/v1/users/current/password", access, map[string]string{"old_password": password, "password": "battery staple"})
+	if status, _ := refreshStatus(c, old); status != http.StatusUnauthorized {
+		t.Fatalf("a token of before the update, after the password changed: %d, want 401", status)
+	}
+}
+
+// The reset from the command line is what an owner runs when the account is not theirs any more:
+// it ends the sessions too.
+func TestARefreshTokenStopsWorkingWhenTheCredentialsAreReset(t *testing.T) {
+	users, c := newRig(t)
+	const password = "correct horse"
+	user := users.CreateUser(model2.UserDBModel{Username: "admin", Password: encryption.HashPassword(password), Role: "admin"})
+	_, refresh := tokens(t, c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "admin", "password": password}))
+
+	user.Password = encryption.HashPassword("battery staple")
+	if err := users.ResetUserCredentials(user); err != nil {
+		t.Fatal(err)
+	}
+
+	if status, _ := refreshStatus(c, refresh); status != http.StatusUnauthorized {
+		t.Fatalf("a token of before the reset: %d, want 401", status)
+	}
+	c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "admin", "password": "battery staple"})
+}
+
+// The database gives the number of an account that is gone to the next one that is made. A
+// token of the first, an hour old, must not open the second: a token is never older than its
+// account.
+func TestARefreshTokenOfAnAccountThatIsGoneDoesNotOpenTheNextOne(t *testing.T) {
+	users, c := newRig(t)
+	gone := users.CreateUser(model2.UserDBModel{Username: "admin", Password: encryption.HashPassword("correct horse"), Role: "admin"})
+	private, _ := users.GetKeyPair()
+	issued := time.Now().Add(-time.Hour)
+	old, err := gojwt.NewWithClaims(gojwt.SigningMethodES256, jwt.Claims{Username: "admin", ID: gone.Id, RegisteredClaims: gojwt.RegisteredClaims{
+		ExpiresAt: gojwt.NewNumericDate(issued.Add(7 * 24 * time.Hour)),
+		IssuedAt:  gojwt.NewNumericDate(issued),
+		NotBefore: gojwt.NewNumericDate(issued),
+		Issuer:    "refresh",
+		ID:        "0",
+	}}).SignedString(private)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	users.DeleteUserById(strconv.Itoa(gone.Id))
+	next := users.CreateUser(model2.UserDBModel{Username: "owner", Password: encryption.HashPassword("battery staple"), Role: "admin"})
+	if next.Id != gone.Id {
+		t.Skipf("the next account got another number (%d after %d): nothing to tell apart", next.Id, gone.Id)
+	}
+
+	if status, _ := refreshStatus(c, old); status != http.StatusUnauthorized {
+		t.Fatalf("the token of the account that was there before: %d, want 401", status)
+	}
+	// and the next account's own session works
+	_, refresh := tokens(t, c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "owner", "password": "battery staple"}))
+	if status, _ := refreshStatus(c, refresh); status != http.StatusOK {
+		t.Fatalf("the session of the next account: %d, want 200", status)
+	}
+}
+
+// Saving the profile from a row read earlier must not put the sessions back to where they were.
+func TestSavingAProfileDoesNotBringEndedSessionsBack(t *testing.T) {
+	users, c := newRig(t)
+	const password = "correct horse"
+	user := users.CreateUser(model2.UserDBModel{Username: "admin", Password: encryption.HashPassword(password), Role: "admin"})
+	if err := users.UpdateUserPassword(user); err != nil { // the row is at session 1
+		t.Fatal(err)
+	}
+	_, copied := tokens(t, c.expect(200, 200, "POST", "/v1/users/login", "", map[string]string{"username": "admin", "password": password}))
+	read := users.GetUserAllInfoById(strconv.Itoa(user.Id)) // read while the copied token is good
+	if err := users.UpdateUserPassword(read); err != nil {  // session 2: the copied token is ended
+		t.Fatal(err)
+	}
+
+	read.Nickname = "Ada"
+	users.UpdateUser(read)
+
+	if status, _ := refreshStatus(c, copied); status != http.StatusUnauthorized {
+		t.Fatalf("an ended session, after the profile was saved from an older read: %d, want 401", status)
+	}
+}

service/model/o_user.go

@@ -33,6 +33,14 @@ type UserDBModel struct {
 	TotpEnabled   bool     `gorm:"default:false" json:"totp_enabled"`
 	TotpLastStep  int64    `gorm:"default:0" json:"-"`       // last accepted 30 s step (replay guard)
 	RecoveryCodes []string `gorm:"serializer:json" json:"-"` // bcrypt hashes, removed as consumed
+
+	// SessionVersion numbers the sessions of the account: a refresh token carries the number the
+	// row had when it was made, and is refused once the row has another. A password change adds
+	// one, which ends every session opened before it. A new account starts at the instant it is
+	// made (CreateUser), not at 0: the database gives the id of an account that is gone to the
+	// next one, and a token of the first must not be of the second's session. An account from
+	// before sessions were numbered is at 0, like its tokens, which say nothing.
+	SessionVersion int64 `gorm:"default:0" json:"-"`
 }
 
 func (p *UserDBModel) TableName() string {