1
0
mirror of https://github.com/volatiletech/authboss.git synced 2025-09-16 09:06:20 +02:00

Add EventAuthHijack to work around ordering issue

Lock/Confirm and possibly other authentication preemption mechanisms
hook into 'Before(EventAuth)', but the ordering of these rejection
mechanisms mixed with the 2fa acceptance response could result in a
dual response.
This commit is contained in:
Aaron L
2018-12-16 22:47:31 -08:00
parent 019073081f
commit f70bdd5eeb
9 changed files with 40 additions and 18 deletions

View File

@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fix a bug where if you were using lock/remember modules with 2fa they
would fail since the events didn't contain the current user in the context
as the auth module delivers them.
- Fix a bug with 2fa where a locked account could get a double response
### Deprecated

View File

@@ -100,6 +100,13 @@ func (a *Auth) LoginPost(w http.ResponseWriter, r *http.Request) error {
return nil
}
handled, err = a.Events.FireBefore(authboss.EventAuthHijack, w, r)
if err != nil {
return err
} else if handled {
return nil
}
logger.Infof("user %s logged in", pid)
authboss.PutSession(w, authboss.SessionKey, pid)
authboss.DelSession(w, authboss.SessionHalfAuthKey)

View File

@@ -13,6 +13,13 @@ type Event int
const (
EventRegister Event = iota
EventAuth
// EventAuthHijack is used to steal the authentication process after a
// successful auth but before any session variable has been put in.
// Most useful for defining an additional step for authentication
// (like 2fa). It needs to be separate to EventAuth because other modules
// do checks that would also interrupt event handlers with an authentication
// failure so there's an ordering problem.
EventAuthHijack
EventOAuth2
EventAuthFail
EventOAuth2Fail

View File

@@ -174,6 +174,13 @@ func (o *OTP) LoginPost(w http.ResponseWriter, r *http.Request) error {
return nil
}
handled, err = o.Events.FireBefore(authboss.EventAuthHijack, w, r)
if err != nil {
return err
} else if handled {
return nil
}
logger.Infof("user %s logged in via otp", pid)
authboss.PutSession(w, authboss.SessionKey, pid)
authboss.DelSession(w, authboss.SessionHalfAuthKey)

View File

@@ -139,7 +139,7 @@ func (s *SMS) Setup() error {
s.Authboss.Core.Router.Get("/2fa/sms/validate", s.Core.ErrorHandler.Wrap(validate.Get))
s.Authboss.Core.Router.Post("/2fa/sms/validate", s.Core.ErrorHandler.Wrap(validate.Post))
s.Authboss.Events.Before(authboss.EventAuth, s.BeforeAuth)
s.Authboss.Events.Before(authboss.EventAuthHijack, s.HijackAuth)
return s.Authboss.Core.ViewRenderer.Load(
PageSMSConfirm,
@@ -151,9 +151,9 @@ func (s *SMS) Setup() error {
)
}
// BeforeAuth stores the user's pid in a special temporary session variable
// HijackAuth stores the user's pid in a special temporary session variable
// and redirects them to the validation endpoint.
func (s *SMS) BeforeAuth(w http.ResponseWriter, r *http.Request, handled bool) (bool, error) {
func (s *SMS) HijackAuth(w http.ResponseWriter, r *http.Request, handled bool) (bool, error) {
if handled {
return false, nil
}

View File

@@ -110,13 +110,13 @@ func (h *testHarness) setSession(key, value string) {
h.session.ClientValues[key] = value
}
func TestBeforeAuth(t *testing.T) {
func TestHijackAuth(t *testing.T) {
t.Parallel()
t.Run("Handled", func(t *testing.T) {
harness := testSetup()
handled, err := harness.sms.BeforeAuth(nil, nil, true)
handled, err := harness.sms.HijackAuth(nil, nil, true)
if handled {
t.Error("should not be handled")
}
@@ -135,7 +135,7 @@ func TestBeforeAuth(t *testing.T) {
harness.putUserInCtx(user, &r)
harness.loadClientState(w, &r)
handled, err := harness.sms.BeforeAuth(w, r, false)
handled, err := harness.sms.HijackAuth(w, r, false)
if handled {
t.Error("should not be handled")
}
@@ -147,7 +147,7 @@ func TestBeforeAuth(t *testing.T) {
t.Run("Ok", func(t *testing.T) {
harness := testSetup()
handled, err := harness.sms.BeforeAuth(nil, nil, true)
handled, err := harness.sms.HijackAuth(nil, nil, true)
if handled {
t.Error("should not be handled")
}
@@ -162,7 +162,7 @@ func TestBeforeAuth(t *testing.T) {
harness.putUserInCtx(user, &r)
harness.loadClientState(w, &r)
handled, err = harness.sms.BeforeAuth(w, r, false)
handled, err = harness.sms.HijackAuth(w, r, false)
if !handled {
t.Error("should be handled")
}

View File

@@ -108,7 +108,7 @@ func (t *TOTP) Setup() error {
t.Authboss.Core.Router.Get("/2fa/totp/validate", t.Core.ErrorHandler.Wrap(t.GetValidate))
t.Authboss.Core.Router.Post("/2fa/totp/validate", t.Core.ErrorHandler.Wrap(t.PostValidate))
t.Authboss.Events.Before(authboss.EventAuth, t.BeforeAuth)
t.Authboss.Events.Before(authboss.EventAuthHijack, t.HijackAuth)
return t.Authboss.Core.ViewRenderer.Load(
PageTOTPSetup,
@@ -120,9 +120,9 @@ func (t *TOTP) Setup() error {
)
}
// BeforeAuth stores the user's pid in a special temporary session variable
// HijackAuth stores the user's pid in a special temporary session variable
// and redirects them to the validation endpoint.
func (t *TOTP) BeforeAuth(w http.ResponseWriter, r *http.Request, handled bool) (bool, error) {
func (t *TOTP) HijackAuth(w http.ResponseWriter, r *http.Request, handled bool) (bool, error) {
if handled {
return false, nil
}

View File

@@ -105,13 +105,13 @@ func (h *testHarness) setSession(key, value string) {
h.session.ClientValues[key] = value
}
func TestBeforeAuth(t *testing.T) {
func TestHijackAuth(t *testing.T) {
t.Parallel()
t.Run("Handled", func(t *testing.T) {
harness := testSetup()
handled, err := harness.totp.BeforeAuth(nil, nil, true)
handled, err := harness.totp.HijackAuth(nil, nil, true)
if handled {
t.Error("should not be handled")
}
@@ -130,7 +130,7 @@ func TestBeforeAuth(t *testing.T) {
harness.putUserInCtx(user, &r)
harness.loadClientState(w, &r)
handled, err := harness.totp.BeforeAuth(w, r, false)
handled, err := harness.totp.HijackAuth(w, r, false)
if handled {
t.Error("should not be handled")
}
@@ -142,7 +142,7 @@ func TestBeforeAuth(t *testing.T) {
t.Run("Ok", func(t *testing.T) {
harness := testSetup()
handled, err := harness.totp.BeforeAuth(nil, nil, true)
handled, err := harness.totp.HijackAuth(nil, nil, true)
if handled {
t.Error("should not be handled")
}
@@ -157,7 +157,7 @@ func TestBeforeAuth(t *testing.T) {
harness.putUserInCtx(user, &r)
harness.loadClientState(w, &r)
handled, err = harness.totp.BeforeAuth(w, r, false)
handled, err = harness.totp.HijackAuth(w, r, false)
if !handled {
t.Error("should be handled")
}

View File

@@ -4,9 +4,9 @@ package authboss
import "strconv"
const _Event_name = "EventRegisterEventAuthEventOAuth2EventAuthFailEventOAuth2FailEventRecoverStartEventRecoverEndEventGetUserEventGetUserSessionEventPasswordReset"
const _Event_name = "EventRegisterEventAuthEventAuthHijackEventOAuth2EventAuthFailEventOAuth2FailEventRecoverStartEventRecoverEndEventGetUserEventGetUserSessionEventPasswordReset"
var _Event_index = [...]uint8{0, 13, 22, 33, 46, 61, 78, 93, 105, 124, 142}
var _Event_index = [...]uint8{0, 13, 22, 37, 48, 61, 76, 93, 108, 120, 139, 157}
func (i Event) String() string {
if i < 0 || i >= Event(len(_Event_index)-1) {