From b3cfbcb04e899d1ea2162e92e767e205f9d0ad4a Mon Sep 17 00:00:00 2001
From: Christian Pointner <equinox@helsinki.at>
Date: Tue, 23 Apr 2019 22:05:18 +0200
Subject: [PATCH] sessions.insert() vs session.new()
---
auth/auth_test.go | 20 ++++----------------
auth/oidc.go | 16 +++++++---------
auth/sessions.go | 26 +++++++++++---------------
auth/sessions_test.go | 6 +-----
4 files changed, 23 insertions(+), 45 deletions(-)
diff --git a/auth/auth_test.go b/auth/auth_test.go
index 0409b3b..aaf9824 100644
--- a/auth/auth_test.go
+++ b/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)
diff --git a/auth/oidc.go b/auth/oidc.go
index e0159d7..66fa6db 100644
--- a/auth/oidc.go
+++ b/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() {
diff --git a/auth/sessions.go b/auth/sessions.go
index d2d9ec2..5ed0e97 100644
--- a/auth/sessions.go
+++ b/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
diff --git a/auth/sessions_test.go b/auth/sessions_test.go
index eb9d6e4..de2558a 100644
--- a/auth/sessions_test.go
+++ b/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 {
--
GitLab