[3.2.7 backport] CBG-4694 do not return invalid sessions with GET (#7595)

Author: torcolvin

auth/session.go

@@ -114,15 +114,24 @@ func (auth *Authenticator) CreateSession(ctx context.Context, user User, ttl tim
 	return session, nil
 }
 
+// GetSession returns a session by ID. Return a not found error if the session is not found, or is invalid.
 func (auth *Authenticator) GetSession(sessionID string) (*LoginSession, error) {
 	var session LoginSession
 	_, err := auth.datastore.Get(auth.DocIDForSession(sessionID), &session)
 	if err != nil {
-		if base.IsDocNotFoundError(err) {
-			err = nil
-		}
 		return nil, err
 	}
+	user, err := auth.GetUser(session.Username)
+	if err != nil {
+		return nil, err
+	}
+	if user == nil {
+		return nil, base.ErrNotFound
+	}
+	if session.SessionUUID != user.GetSessionUUID() {
+		return nil, base.ErrNotFound
+	}
+
 	return &session, nil
 }
 

auth/session_test.go

@@ -91,10 +91,9 @@ func TestDeleteSession(t *testing.T) {
 	assert.NoError(t, dataStore.Set(auth.DocIDForSession(mockSession.ID), noSessionExpiry, nil, mockSession))
 	assert.NoError(t, auth.DeleteSession(ctx, mockSession.ID, ""))
 
-	// Just to verify the session has been deleted gracefully.
 	session, err := auth.GetSession(mockSession.ID)
 	assert.Nil(t, session)
-	assert.NoError(t, err)
+	base.RequireDocNotFoundError(t, err)
 }
 
 // Coverage for MakeSessionCookie. The MakeSessionCookie should create a cookie

docs/api/paths/admin/db-_session.yaml

@@ -40,7 +40,7 @@ post:
               description: User name to generate the session for.
               type: string
             ttl:
-              description: Time until the session expires. Uses default value of 24 hours if left blank.
+              description: Time until the session expires. Uses default value of 24 hours if left blank. This value must be greater or equal to 1.
               type: integer
   responses:
     '200':

rest/session_api.go

@@ -242,14 +242,9 @@ func (h *handler) getUserSession() error {
 
 	h.assertAdminOnly()
 	session, err := h.db.Authenticator(h.ctx()).GetSession(h.PathVar("sessionid"))
-
-	if session == nil {
-		if err == nil {
-			err = kNotFoundError
-		}
+	if err != nil {
 		return err
 	}
-
 	return h.respondWithSessionInfoForSession(session)
 }
 
@@ -292,22 +287,17 @@ func (h *handler) deleteUserSessionWithValidation(sessionId string, userName str
 	// Validate that the session being deleted belongs to the user.  This adds some
 	// overhead - for user-agnostic session deletion should use deleteSession
 	session, getErr := h.db.Authenticator(h.ctx()).GetSession(sessionId)
-	if session == nil {
-		if getErr == nil {
-			getErr = kNotFoundError
-		}
+	if getErr != nil {
 		return getErr
 	}
 
-	if getErr == nil {
-		if session.Username == userName {
-			delErr := h.db.Authenticator(h.ctx()).DeleteSession(h.ctx(), sessionId, userName)
-			if delErr != nil {
-				return delErr
-			}
-		} else {
-			return kNotFoundError
+	if session.Username == userName {
+		delErr := h.db.Authenticator(h.ctx()).DeleteSession(h.ctx(), sessionId, userName)
+		if delErr != nil {
+			return delErr
 		}
+	} else {
+		return kNotFoundError
 	}
 	return nil
 }

rest/session_test.go

@@ -447,19 +447,19 @@ func TestSessionAPI(t *testing.T) {
 
 	// Attempt to GET the deleted session and make sure it's not found
 	response = rt.SendAdminRequest("GET", fmt.Sprintf("/db/_session/%s", user1sessions[0]), "")
-	RequireStatus(t, response, 404)
+	RequireNotFoundError(t, response)
 
 	// 2. DELETE a session with user validation
 	response = rt.SendAdminRequest("DELETE", fmt.Sprintf("/db/_user/%s/_session/%s", "user1", user1sessions[1]), "")
 	RequireStatus(t, response, 200)
 
 	// Attempt to GET the deleted session and make sure it's not found
 	response = rt.SendAdminRequest("GET", fmt.Sprintf("/db/_session/%s", user1sessions[1]), "")
-	RequireStatus(t, response, 404)
+	RequireNotFoundError(t, response)
 
 	// 3. DELETE a session not belonging to the user (should fail)
 	response = rt.SendAdminRequest("DELETE", fmt.Sprintf("/db/_user/%s/_session/%s", "user1", user2sessions[0]), "")
-	RequireStatus(t, response, 404)
+	RequireNotFoundError(t, response)
 
 	// GET the session and make sure it still exists
 	response = rt.SendAdminRequest("GET", fmt.Sprintf("/db/_session/%s", user2sessions[0]), "")
@@ -469,6 +469,23 @@ func TestSessionAPI(t *testing.T) {
 	response = rt.SendAdminRequest("DELETE", "/db/_user/user2/_session", "")
 	RequireStatus(t, response, 200)
 
+	// Validate that all sessions were deleted
+	for i := range 5 {
+		response = rt.SendAdminRequest("GET", fmt.Sprintf("/db/_session/%s", user2sessions[i]), "")
+		RequireNotFoundError(t, response)
+	}
+
+	// 5. DELETE sessions when password is changed
+	// Change password for user3
+	response = rt.SendAdminRequest("PUT", "/db/_user/user3", `{"password":"5678"}`)
+	RequireStatus(t, response, 200)
+
+	// Validate that all sessions were deleted
+	for i := range 5 {
+		response = rt.SendAdminRequest("GET", fmt.Sprintf("/db/_session/%s", user3sessions[i]), "")
+		RequireNotFoundError(t, response)
+	}
+
 	// DELETE the users
 	RequireStatus(t, rt.SendAdminRequest("DELETE", "/db/_user/user1", ""), 200)
 	RequireStatus(t, rt.SendAdminRequest("GET", "/db/_user/user1", ""), 404)

rest/utilities_testing.go

@@ -2827,3 +2827,10 @@ func (sc *ServerContext) reloadDatabaseWithConfigLoadFromBucket(nonContextStruct
 	defer sc.lock.Unlock()
 	return sc._reloadDatabaseWithConfig(nonContextStruct.Ctx, config, true, true)
 }
+
+func RequireNotFoundError(t *testing.T, response *TestResponse) {
+	RequireStatus(t, response, http.StatusNotFound)
+	var body db.Body
+	require.NoError(t, base.JSONUnmarshal(response.Body.Bytes(), &body))
+	require.Equal(t, db.Body{"error": "not_found", "reason": "missing"}, body)
+}