sessions.insert() vs session.new()

auth/auth_test.go

@@ -192,13 +192,10 @@ func TestAuthBearerToken(t *testing.T) {
 		t.Fatalf("authentication should be enabled but Init created no session manager")
 	}
 
-	s, err := NewSession()
+	s, err := auth.sessions.new()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err = auth.sessions.insert(s); err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
 
 	router := mux.NewRouter()
 	InstallHTTPHandler(router)
@@ -242,13 +239,10 @@ func TestAuthWaitForLogin(t *testing.T) {
 		t.Fatalf("authentication should be enabled but Init created no session manager")
 	}
 
-	s, err := NewSession()
+	s, err := auth.sessions.new()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err = auth.sessions.insert(s); err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
 
 	router := mux.NewRouter()
 	InstallHTTPHandler(router)
@@ -313,13 +307,10 @@ func TestAuthDeleteSession(t *testing.T) {
 		t.Fatalf("authentication should be enabled but Init created no session manager")
 	}
 
-	s, err := NewSession()
+	s, err := auth.sessions.new()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err = auth.sessions.insert(s); err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
 
 	router := mux.NewRouter()
 	InstallHTTPHandler(router)
@@ -358,13 +349,10 @@ func TestAuthMiddleware(t *testing.T) {
 		t.Fatalf("authentication should be enabled but Init created no session manager")
 	}
 
-	s, err := NewSession()
+	s, err := auth.sessions.new()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err = auth.sessions.insert(s); err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
 
 	router := mux.NewRouter()
 	InstallHTTPHandler(router)

auth/oidc.go

@@ -51,9 +51,7 @@ func (s *OIDCSession) refresh(ctx context.Context) (*Session, error) {
 		return nil, errors.New("fetching OIDC UserInfo failed: " + err.Error())
 	}
 
-	// TOOD: if we later use sessions.update() it is overkill generate
-	//       a whole new session...
-	newS, err := NewSession()
+	newS, err := &Session{}, nil // NewSession()
 	if err != nil {
 		return nil, err
 	}
@@ -138,15 +136,15 @@ func (b *OIDCBackend) NewOIDCSession(ctx context.Context, arguments json.RawMess
 		}
 		s.setState(SessionStateLoggedIn)
 	} else {
-		if s, err = NewSession(); err != nil {
-			return
-		}
+		// if s, err = NewSession(); err != nil {
+		// 	return
+		// }
 	}
 
 	s.oidc = os
-	if err = auth.sessions.insert(s); err != nil {
-		return nil, err
-	}
+	// if err = auth.sessions.insert(s); err != nil {
+	// 	return nil, err
+	// }
 
 	if s.State() != SessionStateLoggedIn {
 		time.AfterFunc(b.loginTimeout, func() {

auth/sessions.go

@@ -97,19 +97,6 @@ type Session struct {
 	Shows    []string `json:"shows"`
 }
 
-func NewSession() (s *Session, err error) {
-	s = &Session{}
-	if s.id, err = generateRandomString(16); err != nil {
-		return
-	}
-	if s.secret, err = generateRandomString(32); err != nil {
-		return
-	}
-	s.state = SessionStateNew
-	s.ctx = context.Background()
-	return
-}
-
 var (
 	anonAllowNone  = &Session{Username: "anonymous", ReadOnly: false, AllShows: false, Shows: []string{}}
 	anonAllowAll   = &Session{Username: "anonymous", ReadOnly: false, AllShows: true, Shows: []string{}}
@@ -280,10 +267,19 @@ func (sm *SessionManager) runMaintenance() {
 	}
 }
 
-func (sm *SessionManager) insert(s *Session) (err error) {
+func (sm *SessionManager) new() (s *Session, err error) {
 	sm.mutex.Lock()
 	defer sm.mutex.Unlock()
-	s.ctx, s.cancel = context.WithTimeout(s.ctx, sm.maxAge)
+
+	s = &Session{}
+	if s.id, err = generateRandomString(16); err != nil {
+		return
+	}
+	if s.secret, err = generateRandomString(32); err != nil {
+		return
+	}
+	s.state = SessionStateNew
+	s.ctx, s.cancel = context.WithTimeout(context.Background(), sm.maxAge)
 	sm.sessions[s.id] = s
 	auth.dbgLog.Printf("authentication: added new session %s", s.id)
 	return

auth/sessions_test.go

@@ -52,14 +52,10 @@ func TestSessionExpiry(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
-	s1, err := NewSession()
+	s1, err := sm.new()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	s1.Username = "test"
-	if err = sm.insert(s1); err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
 
 	s2 := sm.get(s1.ID())
 	if s2 == nil {