Skip to content

Commit 8f22fc9

Browse files
committed
session: add ShiftState method to session Store
And only allow legal state shifts.
1 parent ad38fcb commit 8f22fc9

File tree

3 files changed

+112
-2
lines changed

3 files changed

+112
-2
lines changed

session/interface.go

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ const (
2727
type State uint8
2828

2929
/*
30-
/---> StateExpired
30+
/---> StateExpired (terminal)
3131
StateCreated ---
32-
\---> StateRevoked
32+
\---> StateRevoked (terminal)
3333
*/
3434

3535
const (
@@ -59,6 +59,24 @@ const (
5959
StateReserved State = 4
6060
)
6161

62+
// Terminal returns true if the state is a terminal state.
63+
func (s State) Terminal() bool {
64+
return s == StateExpired || s == StateRevoked
65+
}
66+
67+
// legalStateShifts is a map that defines the legal State transitions that a
68+
// Session can be put through.
69+
var legalStateShifts = map[State]map[State]bool{
70+
StateCreated: {
71+
StateExpired: true,
72+
StateRevoked: true,
73+
},
74+
StateInUse: {
75+
StateRevoked: true,
76+
StateExpired: true,
77+
},
78+
}
79+
6280
// MacaroonRecipe defines the permissions and caveats that should be used
6381
// to bake a macaroon.
6482
type MacaroonRecipe struct {
@@ -225,5 +243,9 @@ type Store interface {
225243
// StateReserved state.
226244
DeleteReservedSessions() error
227245

246+
// ShiftState updates the state of the session with the given ID to the
247+
// "dest" state.
248+
ShiftState(id ID, dest State) error
249+
228250
IDToGroupIndex
229251
}

session/kvdb_store.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,47 @@ func (db *BoltStore) DeleteReservedSessions() error {
514514
})
515515
}
516516

517+
// ShiftState updates the state of the session with the given ID to the "dest"
518+
// state.
519+
//
520+
// NOTE: this is part of the Store interface.
521+
func (db *BoltStore) ShiftState(id ID, dest State) error {
522+
return db.Update(func(tx *bbolt.Tx) error {
523+
sessionBucket, err := getBucket(tx, sessionBucketKey)
524+
if err != nil {
525+
return err
526+
}
527+
528+
session, err := getSessionByID(sessionBucket, id)
529+
if err != nil {
530+
return err
531+
}
532+
533+
// If the session is already in the desired state, we return
534+
// with no error to maintain idempotency.
535+
if session.State == dest {
536+
return nil
537+
}
538+
539+
// Ensure that the wanted state change is allowed.
540+
allowedDestinations, ok := legalStateShifts[session.State]
541+
if !ok || !allowedDestinations[dest] {
542+
return fmt.Errorf("illegal session state transition "+
543+
"from %d to %d", session.State, dest)
544+
}
545+
546+
session.State = dest
547+
548+
// If the session is terminal, we set the revoked at time to the
549+
// current time.
550+
if dest.Terminal() {
551+
session.RevokedAt = db.clock.Now().UTC()
552+
}
553+
554+
return putSession(sessionBucket, session)
555+
})
556+
}
557+
517558
// RevokeSession updates the state of the session with the given local
518559
// public key to be revoked.
519560
//

session/store_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,53 @@ func TestCheckSessionGroupPredicate(t *testing.T) {
392392
require.True(t, ok)
393393
}
394394

395+
// TestStateShift tests that the ShiftState method works as expected.
396+
func TestStateShift(t *testing.T) {
397+
// Set up a new DB.
398+
clock := clock.NewTestClock(testTime)
399+
db, err := NewDB(t.TempDir(), "test.db", clock)
400+
require.NoError(t, err)
401+
t.Cleanup(func() {
402+
_ = db.Close()
403+
})
404+
405+
// Add a new session to the DB.
406+
s1 := newSession(t, db, clock, "label 1")
407+
require.NoError(t, db.CreateSession(s1))
408+
409+
// Check that the session is in the StateCreated state. Also check that
410+
// the "RevokedAt" time has not yet been set.
411+
s1, err = db.GetSession(s1.LocalPublicKey)
412+
require.NoError(t, err)
413+
require.Equal(t, StateCreated, s1.State)
414+
require.Equal(t, time.Time{}, s1.RevokedAt)
415+
416+
// Shift the state of the session to StateRevoked.
417+
err = db.ShiftState(s1.ID, StateRevoked)
418+
require.NoError(t, err)
419+
420+
// This should have worked. Since it is now in a terminal state, the
421+
// "RevokedAt" time should be set.
422+
s1, err = db.GetSession(s1.LocalPublicKey)
423+
require.NoError(t, err)
424+
require.Equal(t, StateRevoked, s1.State)
425+
require.True(t, clock.Now().Equal(s1.RevokedAt))
426+
427+
// Trying to do the same state shift again should succeed since the
428+
// session is already in the expected "dest" state. The revoked-at time
429+
// should not have changed though.
430+
prevTime := clock.Now()
431+
clock.SetTime(prevTime.Add(time.Second))
432+
err = db.ShiftState(s1.ID, StateRevoked)
433+
require.NoError(t, err)
434+
require.True(t, prevTime.Equal(s1.RevokedAt))
435+
436+
// Trying to shift the state from a terminal state back to StateCreated
437+
// should also fail since this is not a legal state transition.
438+
err = db.ShiftState(s1.ID, StateCreated)
439+
require.ErrorContains(t, err, "illegal session state transition")
440+
}
441+
395442
// testSessionModifier is a functional option that can be used to modify the
396443
// default test session created by newSession.
397444
type testSessionModifier func(*Session)

0 commit comments

Comments
 (0)