diff --git a/session/sql_migration.go b/session/sql_migration.go index 428cc0fc..593ceac3 100644 --- a/session/sql_migration.go +++ b/session/sql_migration.go @@ -40,35 +40,16 @@ func MigrateSessionStoreToSQL(ctx context.Context, kvStore *bbolt.DB, return err } - // If sessions are linked to a group, we must insert the initial session - // of each group before the other sessions in that group. This ensures - // we can retrieve the SQL group ID when inserting the remaining - // sessions. Therefore, we first insert all initial group sessions, - // allowing us to fetch the group IDs and insert the rest of the - // sessions afterward. - // We therefore filter out the initial sessions first, and then migrate - // them prior to the rest of the sessions. - var ( - initialGroupSessions []*Session - linkedSessions []*Session - ) - - for _, kvSession := range kvSessions { - if kvSession.GroupID == kvSession.ID { - initialGroupSessions = append( - initialGroupSessions, kvSession, - ) - } else { - linkedSessions = append(linkedSessions, kvSession) - } - } + initialGroupSessions, linkedSessions := filterSessions(kvSessions) + // Migrate the non-linked sessions first. err = migrateSessionsToSQLAndValidate(ctx, tx, initialGroupSessions) if err != nil { return fmt.Errorf("migration of non-linked session failed: %w", err) } + // Then migrate the linked sessions. err = migrateSessionsToSQLAndValidate(ctx, tx, linkedSessions) if err != nil { return fmt.Errorf("migration of linked session failed: %w", err) @@ -81,6 +62,73 @@ func MigrateSessionStoreToSQL(ctx context.Context, kvStore *bbolt.DB, return nil } +// filterSessions categorizes the sessions into two groups: initial group +// sessions and linked sessions. The initial group sessions are the first +// sessions in a session group, while the linked sessions are those that have a +// linked parent session. These are separated to ensure that we can insert the +// initial group sessions first, which allows us to fetch the SQL group ID when +// inserting the rest of the linked sessions afterward. +// +// Additionally, it checks for duplicate session IDs and drops all but +// one session with the same ID, keeping the one with the latest CreatedAt +// timestamp. Note that users with duplicate session IDs should be extremely +// rare, as it could only occur if colliding session IDs were created prior to +// the introduction of the session linking functionality. +func filterSessions(kvSessions []*Session) ([]*Session, []*Session) { + // First map sessions by their ID. + sessionsByID := make(map[ID][]*Session) + for _, s := range kvSessions { + sessionsByID[s.ID] = append(sessionsByID[s.ID], s) + } + + var ( + initialGroupSessions []*Session + linkedSessions []*Session + ) + + // Process the mapped sessions. If there are duplicate sessions with the + // same ID, we will only iterate the session with the latest CreatedAt + // timestamp, and drop the other sessions. This is to ensure that we can + // keep a UNIQUE constraint for the session ID (alias) in the SQL db. + for id, sessions := range sessionsByID { + sessionToKeep := sessions[0] + if len(sessions) > 1 { + log.Warnf("Found %d sessions with duplicate ID %x, "+ + "keeping only the latest one", len(sessions), + id) + + // Find the session with the latest timestamp. + latestSession := sessions[0] + for _, s := range sessions[1:] { + if s.CreatedAt.After(latestSession.CreatedAt) { + latestSession = s + } + } + sessionToKeep = latestSession + + // Log the sessions that will be dropped. + for _, s := range sessions { + if s == sessionToKeep { + continue + } + log.Warnf("Dropping duplicate session with ID "+ + "%x created at %v", id, s.CreatedAt) + } + } + + // Categorize the session that we are keeping. + if sessionToKeep.GroupID == sessionToKeep.ID { + initialGroupSessions = append( + initialGroupSessions, sessionToKeep, + ) + } else { + linkedSessions = append(linkedSessions, sessionToKeep) + } + } + + return initialGroupSessions, linkedSessions +} + // getBBoltSessions is a helper function that fetches all sessions from the // Bbolt store, by iterating directly over the buckets, without needing to // use any public functions of the BoltStore struct. diff --git a/session/sql_migration_test.go b/session/sql_migration_test.go index 77e003fe..d9244b9e 100644 --- a/session/sql_migration_test.go +++ b/session/sql_migration_test.go @@ -352,6 +352,115 @@ func TestSessionsStoreMigration(t *testing.T) { return getBoltStoreSessions(t, store) }, }, + { + name: "multiple sessions with the same ID", + populateDB: func(t *testing.T, store *BoltStore, + _ accounts.Store) []*Session { + + // We first add one session which has no other + // session with same ID, to test that this is + // correctly migrated, and included in the + // migration result. + sess1, err := store.NewSession( + ctx, "session1", TypeMacaroonAdmin, + time.Unix(1000, 0), "", + ) + require.NoError(t, err) + + sess2, err := store.NewSession( + ctx, "session2", TypeMacaroonAdmin, + time.Unix(1000, 0), "", + ) + require.NoError(t, err) + + // Then add two sessions with the same ID, to + // test that only the latest session is included + // in the migration result. + sess3, err := store.NewSession( + ctx, "session3", TypeMacaroonAdmin, + time.Unix(1000, 0), "", + ) + require.NoError(t, err) + + // During the addition of the session linking + // functionality, logic was added in the + // NewSession function to ensure we can't create + // multiple sessions with the same ID. Therefore + // we need to manually override the ID of + // the second session to match the first + // session, to simulate such a scenario that + // could occur prior to the addition of that + // logic. + // We also need to update the CreatedAt time + // as the execution of this function is too + // fast for the CreatedAt time of sess2 and + // sess3 to differ. + err = updateSessionIDAndCreatedAt( + store, sess3.ID, sess2.MacaroonRootKey, + sess2.CreatedAt.Add(time.Minute), + ) + require.NoError(t, err) + + // Finally, we add three sessions with the same + // ID, to test we can handle more than two + // sessions with the same ID. + sess4, err := store.NewSession( + ctx, "session4", TypeMacaroonAdmin, + time.Unix(1000, 0), "", + ) + require.NoError(t, err) + + sess5, err := store.NewSession( + ctx, "session5", TypeMacaroonAdmin, + time.Unix(1000, 0), "", + ) + require.NoError(t, err) + + sess6, err := store.NewSession( + ctx, "session6", TypeMacaroonAdmin, + time.Unix(1000, 0), "", + ) + require.NoError(t, err) + + err = updateSessionIDAndCreatedAt( + store, sess5.ID, sess4.MacaroonRootKey, + sess4.CreatedAt.Add(time.Minute), + ) + require.NoError(t, err) + + err = updateSessionIDAndCreatedAt( + store, sess6.ID, sess4.MacaroonRootKey, + sess4.CreatedAt.Add(time.Minute*2), + ) + require.NoError(t, err) + + // Now fetch the updated sessions from the kv + // store, so that we are sure that the new IDs + // have really been persisted in the DB. + kvSessions := getBoltStoreSessions(t, store) + require.Len(t, kvSessions, 6) + + getSessionByName := func(name string) *Session { + for _, session := range kvSessions { + if session.Label == name { + return session + } + } + + t.Fatalf("session %s not found", name) + return nil + } + + // When multiple sessions with the same ID + // exist, we expect only the session with the + // latest creation time to be migrated. + return []*Session{ + getSessionByName(sess1.Label), + getSessionByName(sess3.Label), + getSessionByName(sess6.Label), + } + }, + }, { name: "randomized sessions", populateDB: randomizedSessions, @@ -803,3 +912,44 @@ func shiftStateUnsafe(db *BoltStore, id ID, dest State) error { return putSession(sessionBucket, session) }) } + +// updateSessionIDAndCreatedAt can be used to update the ID, the GroupID, +// the MacaroonRootKey and the CreatedAt time a session in the BoltStore. +// +// NOTE: this function should only be used for testing purposes. Also note that +// we pass the macaroon root key to set the new session ID, as the +// DeserializeSession function derives the session ID from the +// session.MacaroonRootKey. +func updateSessionIDAndCreatedAt(db *BoltStore, oldID ID, newIdRootKey uint64, + newCreatedAt time.Time) error { + + newId := IDFromMacRootKeyID(newIdRootKey) + + if oldID == newId { + return fmt.Errorf("can't update session ID to the same ID: %s", + oldID) + } + + return db.Update(func(tx *bbolt.Tx) error { + // Get the main session bucket. + sessionBkt, err := getBucket(tx, sessionBucketKey) + if err != nil { + return err + } + + // Look up the session using the old ID. + sess, err := getSessionByID(sessionBkt, oldID) + if err != nil { + return err + } + + // Update the session. + sess.ID = newId + sess.GroupID = newId + sess.MacaroonRootKey = newIdRootKey + sess.CreatedAt = newCreatedAt + + // Write it back under the same key (local pubkey). + return putSession(sessionBkt, sess) + }) +}