From e886da24a3ccfa7e64a9bbccbc4142e0442fa419 Mon Sep 17 00:00:00 2001 From: Christian Pointner <equinox@helsinki.at> Date: Wed, 17 Apr 2019 00:59:42 +0200 Subject: [PATCH] added login timeout for oidc --- auth/auth.go | 4 +-- auth/config.go | 9 +++--- auth/oidc.go | 72 ++++++++++++++++++++++++++++------------- auth/sessions.go | 39 +++++++++++++++------- contrib/sample-cfg.yaml | 1 + 5 files changed, 85 insertions(+), 40 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 5409ac4..cf57df0 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -74,7 +74,7 @@ func newSession() http.Handler { sendHTTPErrorResponse(w, http.StatusBadRequest, "OIDC authentication is not configured") return } - s, err := NewOIDCSession() + s, err := auth.oidc.NewOIDCSession() if err != nil { sendHTTPErrorResponse(w, http.StatusBadRequest, "Error creating session: "+err.Error()) return @@ -109,7 +109,7 @@ func getSession() http.Handler { return } st := s.State() - if st == SessionStateLoggedIn || st == SessionStateLoginFailed { + if st == SessionStateLoggedIn || st == SessionStateLoginFailed || st == SessionStateLoginTimeout { sendHTTPResponse(w, http.StatusOK, s) return } diff --git a/auth/config.go b/auth/config.go index 9213f71..294488c 100644 --- a/auth/config.go +++ b/auth/config.go @@ -34,10 +34,11 @@ type SessionsConfig struct { } type OIDCConfig struct { - IssuerURL string `json:"issuer-url" yaml:"issuer-url" toml:"issuer-url"` - ClientID string `json:"client-id" yaml:"client-id" toml:"client-id"` - ClientSecret string `json:"client-secret" yaml:"client-secret" toml:"client-secret"` - CallbackURL string `json:"callback-url" yaml:"callback-url" toml:"callback-url"` + IssuerURL string `json:"issuer-url" yaml:"issuer-url" toml:"issuer-url"` + ClientID string `json:"client-id" yaml:"client-id" toml:"client-id"` + ClientSecret string `json:"client-secret" yaml:"client-secret" toml:"client-secret"` + CallbackURL string `json:"callback-url" yaml:"callback-url" toml:"callback-url"` + LoginTimeout time.Duration `json:"login-timeout" yaml:"login-timeout" toml:"login-timeout"` } type Config struct { diff --git a/auth/oidc.go b/auth/oidc.go index 7a40787..6095531 100644 --- a/auth/oidc.go +++ b/auth/oidc.go @@ -27,33 +27,18 @@ package auth import ( "context" "net/http" + "time" oidc "github.com/coreos/go-oidc" "golang.org/x/oauth2" ) -type OIDCSession struct { - Nonce string - token *oauth2.Token -} - -func NewOIDCSession() (s *Session, err error) { - os := &OIDCSession{} - if os.Nonce, err = generateRandomString(16); err != nil { - return - } - - if s, err = NewSession(); err != nil { - return - } - s.oidc = os - if err = auth.sessions.insert(s); err != nil { - return nil, err - } - return -} +const ( + defaultLoginTimeout = 5 * time.Minute +) type OIDCBackend struct { + loginTimeout time.Duration issuerURL string provider *oidc.Provider verifier *oidc.IDTokenVerifier @@ -63,7 +48,11 @@ type OIDCBackend struct { func NewOIDCBackend(cfg *OIDCConfig) (b *OIDCBackend, err error) { // TODO: make ctx a parameter? ctx := context.Background() - b = &OIDCBackend{issuerURL: cfg.IssuerURL} + b = &OIDCBackend{issuerURL: cfg.IssuerURL, loginTimeout: cfg.LoginTimeout} + if b.loginTimeout <= 0 { + b.loginTimeout = defaultLoginTimeout + } + if b.provider, err = oidc.NewProvider(ctx, cfg.IssuerURL); err != nil { return } @@ -91,6 +80,39 @@ func (b *OIDCBackend) String() string { // TODO: implement session refresh go-routine // see: https://github.com/golang/oauth2/issues/84 +type OIDCSession struct { + Nonce string + token *oauth2.Token +} + +func (b *OIDCBackend) NewOIDCSession() (s *Session, err error) { + os := &OIDCSession{} + if os.Nonce, err = generateRandomString(16); err != nil { + return + } + + if s, err = NewSession(); err != nil { + return + } + s.oidc = os + if err = auth.sessions.insert(s); err != nil { + return nil, err + } + + time.AfterFunc(b.loginTimeout, func() { + if s.updateState(SessionStateNew, SessionStateLoginTimeout) { + return + } + if s.updateState(SessionStateLoginStarted, SessionStateLoginTimeout) { + return + } + if s.updateState(SessionStateLoginFinalizing, SessionStateLoginTimeout) { + return + } + }) + return +} + func (b *OIDCBackend) LoginHandler() http.Handler { return &oidcLoginHandler{backend: b} } @@ -118,7 +140,7 @@ func (h *oidcLoginHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { sendHTTPErrorResponse(w, http.StatusConflict, "this session is already logged in or in an invalid state") return } - // TODO start login timeout go-routine + http.Redirect(w, r, h.backend.oauth2Config.AuthCodeURL(s.ID(), oidc.Nonce(s.oidc.Nonce)), http.StatusFound) } @@ -188,6 +210,10 @@ func (h *oidcCallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) sendHTTPErrorResponse(w, http.StatusInternalServerError, "updating session failed: "+err.Error()) return } - newS.setState(SessionStateLoggedIn) + if !newS.updateState(SessionStateLoginFinalizing, SessionStateLoggedIn) { + sendHTTPErrorResponse(w, http.StatusGone, "session login timeout") + return + } + sendHTTPResponse(w, http.StatusOK, "You are now logged in as: "+newS.Username) } diff --git a/auth/sessions.go b/auth/sessions.go index f675ad4..85141ed 100644 --- a/auth/sessions.go +++ b/auth/sessions.go @@ -48,7 +48,8 @@ const ( SessionStateLoginFinalizing SessionStateLoggedIn SessionStateLoginFailed - SessionStateOutdated + SessionStateLoginTimeout + SessionStateStale SessionStateRemoved ) @@ -64,8 +65,10 @@ func (s SessionState) String() string { return "logged-in" case SessionStateLoginFailed: return "login-failed" - case SessionStateOutdated: - return "outdated" + case SessionStateLoginTimeout: + return "login-timeout" + case SessionStateStale: + return "stale" case SessionStateRemoved: return "removed" } @@ -120,13 +123,26 @@ func (s *Session) State() SessionState { } func (s *Session) setState(st SessionState) { - // TODO: notify subscribers! Check for subscriber channel will be racy... - atomic.StoreUint32((*uint32)(&s.state), uint32(st)) + old := atomic.SwapUint32((*uint32)(&s.state), uint32(st)) + if old != uint32(st) { + // TODO: this is racy!! + if s.subscribe != nil { + close(s.subscribe) + s.subscribe = nil + } + } } func (s *Session) updateState(old, new SessionState) bool { - // TODO: notify subscribers! Check for subscriber channel will be racy... - return atomic.CompareAndSwapUint32((*uint32)(&s.state), uint32(old), uint32(new)) + ok := atomic.CompareAndSwapUint32((*uint32)(&s.state), uint32(old), uint32(new)) + if ok && old != new { + // TODO: this is racy!! + if s.subscribe != nil { + close(s.subscribe) + s.subscribe = nil + } + } + return ok } func (s *Session) Expired() bool { @@ -265,7 +281,7 @@ func (sm *SessionManager) getAndSubscribe(id string) (*Session, <-chan struct{}) if !ok { return nil, nil } - // TODO: this is racy + // TODO: this is racy!! if s.subscribe == nil { s.subscribe = make(chan struct{}) } @@ -285,13 +301,14 @@ func (sm *SessionManager) update(id string, s *Session) error { s.state = old.state s.expires = old.expires - // TODO: this is racy + // TODO: this is racy!! if old.subscribe != nil { close(old.subscribe) + s.subscribe = nil } sm.sessions[id] = s - old.setState(SessionStateOutdated) + old.setState(SessionStateStale) return nil } @@ -312,7 +329,7 @@ func (sm *SessionManager) cleanup() { defer sm.mutex.Unlock() for id, s := range sm.sessions { - if s.Expired() || s.State() == SessionStateLoginFailed { + if s.Expired() || s.State() == SessionStateLoginFailed || s.State() == SessionStateLoginTimeout { s.setState(SessionStateRemoved) delete(sm.sessions, id) } diff --git a/contrib/sample-cfg.yaml b/contrib/sample-cfg.yaml index 4a9f1bd..bf20e12 100644 --- a/contrib/sample-cfg.yaml +++ b/contrib/sample-cfg.yaml @@ -38,6 +38,7 @@ importer: # client-id: ${OIDC_CLIENT_ID} # client-secret: ${OIDC_CLIENT_SECRET} # callback-url: http://localhost:8080/auth/oidc/callback +# login-timeout: 1m # defaults to 5 Minutes ### uncomment to enable CORS headers ### see: https://godoc.org/github.com/rs/cors#Options -- GitLab