From 8ad0bc5e612f11b5e5da6c4bf59566cd3e54c9be Mon Sep 17 00:00:00 2001 From: Wilfred Asomani Date: Thu, 14 Sep 2023 12:35:03 +0000 Subject: [PATCH] satellite/console: add alt code protected MFA recovery endpoint This change adds an alternate MFA code recovery endpoint that requires MFA code to generate codes. Issue: https://github.com/storj/storj-private/issues/433 Change-Id: I10d922e9ad1ace4300d4bcfea7f48494227f1ff8 --- .../console/consoleweb/consoleapi/auth.go | 32 ++++++++++++++- .../consoleweb/consoleapi/auth_test.go | 12 +++++- satellite/console/consoleweb/server.go | 1 + satellite/console/mfa.go | 32 ++++++++++++++- satellite/console/service_test.go | 39 +++++++++++-------- 5 files changed, 95 insertions(+), 21 deletions(-) diff --git a/satellite/console/consoleweb/consoleapi/auth.go b/satellite/console/consoleweb/consoleapi/auth.go index 54e27fdbf..eb7f336b3 100644 --- a/satellite/console/consoleweb/consoleapi/auth.go +++ b/satellite/console/consoleweb/consoleapi/auth.go @@ -822,7 +822,37 @@ func (a *Auth) GenerateMFARecoveryCodes(w http.ResponseWriter, r *http.Request) var err error defer mon.Task()(&ctx)(&err) - codes, err := a.service.ResetMFARecoveryCodes(ctx) + codes, err := a.service.ResetMFARecoveryCodes(ctx, false, "", "") + if err != nil { + a.serveJSONError(ctx, w, err) + return + } + + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode(codes) + if err != nil { + a.log.Error("could not encode MFA recovery codes", zap.Error(ErrAuthAPI.Wrap(err))) + return + } +} + +// RegenerateMFARecoveryCodes requires MFA code to create a new set of MFA recovery codes for the user. +func (a *Auth) RegenerateMFARecoveryCodes(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + var err error + defer mon.Task()(&ctx)(&err) + + var data struct { + Passcode string `json:"passcode"` + RecoveryCode string `json:"recoveryCode"` + } + err = json.NewDecoder(r.Body).Decode(&data) + if err != nil { + a.serveJSONError(ctx, w, err) + return + } + + codes, err := a.service.ResetMFARecoveryCodes(ctx, true, data.Passcode, data.RecoveryCode) if err != nil { a.serveJSONError(ctx, w, err) return diff --git a/satellite/console/consoleweb/consoleapi/auth_test.go b/satellite/console/consoleweb/consoleapi/auth_test.go index 29b8b668b..87f9c2114 100644 --- a/satellite/console/consoleweb/consoleapi/auth_test.go +++ b/satellite/console/consoleweb/consoleapi/auth_test.go @@ -437,11 +437,19 @@ func TestMFAEndpoints(t *testing.T) { _, status = doRequest("/disable", badCode, "") require.Equal(t, http.StatusBadRequest, status) - // Expect failure when disabling due to providing both passcode and recovery code. - body, _ = doRequest("/generate-recovery-codes", "", "") + // Expect failure when regenerating without providing either passcode or recovery code. + _, status = doRequest("/regenerate-recovery-codes", "", "") + require.Equal(t, http.StatusBadRequest, status) + + // Expect failure when regenerating when providing both passcode and recovery code. + _, status = doRequest("/regenerate-recovery-codes", goodCode, codes[0]) + require.Equal(t, http.StatusConflict, status) + + body, _ = doRequest("/regenerate-recovery-codes", goodCode, "") err = json.Unmarshal(body, &codes) require.NoError(t, err) + // Expect failure when disabling due to providing both passcode and recovery code. _, status = doRequest("/disable", goodCode, codes[0]) require.Equal(t, http.StatusConflict, status) diff --git a/satellite/console/consoleweb/server.go b/satellite/console/consoleweb/server.go index 09c8c6c0b..155c28977 100644 --- a/satellite/console/consoleweb/server.go +++ b/satellite/console/consoleweb/server.go @@ -313,6 +313,7 @@ func NewServer(logger *zap.Logger, config Config, service *console.Service, oidc authRouter.Handle("/mfa/disable", server.withAuth(http.HandlerFunc(authController.DisableUserMFA))).Methods(http.MethodPost, http.MethodOptions) authRouter.Handle("/mfa/generate-secret-key", server.withAuth(http.HandlerFunc(authController.GenerateMFASecretKey))).Methods(http.MethodPost, http.MethodOptions) authRouter.Handle("/mfa/generate-recovery-codes", server.withAuth(http.HandlerFunc(authController.GenerateMFARecoveryCodes))).Methods(http.MethodPost, http.MethodOptions) + authRouter.Handle("/mfa/regenerate-recovery-codes", server.withAuth(http.HandlerFunc(authController.RegenerateMFARecoveryCodes))).Methods(http.MethodPost, http.MethodOptions) authRouter.Handle("/logout", server.withAuth(http.HandlerFunc(authController.Logout))).Methods(http.MethodPost, http.MethodOptions) authRouter.Handle("/token", server.ipRateLimiter.Limit(http.HandlerFunc(authController.Token))).Methods(http.MethodPost, http.MethodOptions) authRouter.Handle("/token-by-api-key", server.ipRateLimiter.Limit(http.HandlerFunc(authController.TokenByAPIKey))).Methods(http.MethodPost, http.MethodOptions) diff --git a/satellite/console/mfa.go b/satellite/console/mfa.go index 41207cc36..fa9f05cfe 100644 --- a/satellite/console/mfa.go +++ b/satellite/console/mfa.go @@ -216,7 +216,7 @@ func (s *Service) ResetMFASecretKey(ctx context.Context) (key string, err error) } // ResetMFARecoveryCodes creates a new set of MFA recovery codes for the user. -func (s *Service) ResetMFARecoveryCodes(ctx context.Context) (codes []string, err error) { +func (s *Service) ResetMFARecoveryCodes(ctx context.Context, requireCode bool, passcode string, recoveryCode string) (codes []string, err error) { defer mon.Task()(&ctx)(&err) user, err := s.getUserAndAuditLog(ctx, "reset MFA recovery codes") @@ -228,6 +228,36 @@ func (s *Service) ResetMFARecoveryCodes(ctx context.Context) (codes []string, er return nil, ErrUnauthorized.New(mfaRecoveryGenerationErrMsg) } + if requireCode { + t := time.Now() + if recoveryCode != "" && passcode != "" { + return nil, ErrMFAConflict.New(mfaConflictErrMsg) + } + + if recoveryCode != "" { + found := false + for _, code := range user.MFARecoveryCodes { + if code == recoveryCode { + found = true + break + } + } + if !found { + return nil, ErrMFARecoveryCode.New(mfaRecoveryInvalidErrMsg) + } + } else if passcode != "" { + valid, err := ValidateMFAPasscode(passcode, user.MFASecretKey, t) + if err != nil { + return nil, ErrValidation.Wrap(ErrMFAPasscode.Wrap(err)) + } + if !valid { + return nil, ErrValidation.Wrap(ErrMFAPasscode.New(mfaPasscodeInvalidErrMsg)) + } + } else { + return nil, ErrMFAMissing.New(mfaRequiredErrMsg) + } + } + codes = make([]string, MFARecoveryCodeCount) for i := 0; i < MFARecoveryCodeCount; i++ { code, err := NewMFARecoveryCode() diff --git a/satellite/console/service_test.go b/satellite/console/service_test.go index 7b6ffd521..ff4e450ed 100644 --- a/satellite/console/service_test.go +++ b/satellite/console/service_test.go @@ -870,6 +870,7 @@ func TestMFA(t *testing.T) { } userCtx, user := updateContext() + mfaTime := time.Now() var key string t.Run("ResetMFASecretKey", func(t *testing.T) { key, err = service.ResetMFASecretKey(userCtx) @@ -881,14 +882,14 @@ func TestMFA(t *testing.T) { t.Run("EnableUserMFABadPasscode", func(t *testing.T) { // Expect MFA-enabling attempt to be rejected when providing stale passcode. - badCode, err := console.NewMFAPasscode(key, time.Time{}.Add(time.Hour)) + badCode, err := console.NewMFAPasscode(key, mfaTime.Add(time.Hour)) require.NoError(t, err) - err = service.EnableUserMFA(userCtx, badCode, time.Time{}) + err = service.EnableUserMFA(userCtx, badCode, mfaTime) require.True(t, console.ErrValidation.Has(err)) userCtx, _ = updateContext() - _, err = service.ResetMFARecoveryCodes(userCtx) + _, err = service.ResetMFARecoveryCodes(userCtx, false, "", "") require.True(t, console.ErrUnauthorized.Has(err)) _, user = updateContext() @@ -897,11 +898,11 @@ func TestMFA(t *testing.T) { t.Run("EnableUserMFAGoodPasscode", func(t *testing.T) { // Expect MFA-enabling attempt to succeed when providing valid passcode. - goodCode, err := console.NewMFAPasscode(key, time.Time{}) + goodCode, err := console.NewMFAPasscode(key, mfaTime) require.NoError(t, err) userCtx, _ = updateContext() - err = service.EnableUserMFA(userCtx, goodCode, time.Time{}) + err = service.EnableUserMFA(userCtx, goodCode, mfaTime) require.NoError(t, err) _, user = updateContext() @@ -937,7 +938,7 @@ func TestMFA(t *testing.T) { }) t.Run("MFARecoveryCodes", func(t *testing.T) { - _, err = service.ResetMFARecoveryCodes(userCtx) + _, err = service.ResetMFARecoveryCodes(userCtx, false, "", "") require.NoError(t, err) _, user = updateContext() @@ -962,17 +963,21 @@ func TestMFA(t *testing.T) { } userCtx, _ = updateContext() - _, err = service.ResetMFARecoveryCodes(userCtx) + + // requiring MFA code to reset recovery codes should work + code, err := console.NewMFAPasscode(key, mfaTime) + require.NoError(t, err) + _, err = service.ResetMFARecoveryCodes(userCtx, true, code, "") require.NoError(t, err) }) t.Run("DisableUserMFABadPasscode", func(t *testing.T) { // Expect MFA-disabling attempt to fail when providing valid passcode. - badCode, err := console.NewMFAPasscode(key, time.Time{}.Add(time.Hour)) + badCode, err := console.NewMFAPasscode(key, mfaTime.Add(time.Hour)) require.NoError(t, err) userCtx, _ = updateContext() - err = service.DisableUserMFA(userCtx, badCode, time.Time{}, "") + err = service.DisableUserMFA(userCtx, badCode, mfaTime, "") require.True(t, console.ErrValidation.Has(err)) _, user = updateContext() @@ -983,11 +988,11 @@ func TestMFA(t *testing.T) { t.Run("DisableUserMFAConflict", func(t *testing.T) { // Expect MFA-disabling attempt to fail when providing both recovery code and passcode. - goodCode, err := console.NewMFAPasscode(key, time.Time{}) + goodCode, err := console.NewMFAPasscode(key, mfaTime) require.NoError(t, err) userCtx, user = updateContext() - err = service.DisableUserMFA(userCtx, goodCode, time.Time{}, user.MFARecoveryCodes[0]) + err = service.DisableUserMFA(userCtx, goodCode, mfaTime, user.MFARecoveryCodes[0]) require.True(t, console.ErrMFAConflict.Has(err)) _, user = updateContext() @@ -998,11 +1003,11 @@ func TestMFA(t *testing.T) { t.Run("DisableUserMFAGoodPasscode", func(t *testing.T) { // Expect MFA-disabling attempt to succeed when providing valid passcode. - goodCode, err := console.NewMFAPasscode(key, time.Time{}) + goodCode, err := console.NewMFAPasscode(key, mfaTime) require.NoError(t, err) userCtx, _ = updateContext() - err = service.DisableUserMFA(userCtx, goodCode, time.Time{}, "") + err = service.DisableUserMFA(userCtx, goodCode, mfaTime, "") require.NoError(t, err) userCtx, user = updateContext() @@ -1017,15 +1022,15 @@ func TestMFA(t *testing.T) { key, err = service.ResetMFASecretKey(userCtx) require.NoError(t, err) - goodCode, err := console.NewMFAPasscode(key, time.Time{}) + goodCode, err := console.NewMFAPasscode(key, mfaTime) require.NoError(t, err) userCtx, _ = updateContext() - err = service.EnableUserMFA(userCtx, goodCode, time.Time{}) + err = service.EnableUserMFA(userCtx, goodCode, mfaTime) require.NoError(t, err) userCtx, _ = updateContext() - _, err = service.ResetMFARecoveryCodes(userCtx) + _, err = service.ResetMFARecoveryCodes(userCtx, false, "", "") require.NoError(t, err) userCtx, user = updateContext() @@ -1034,7 +1039,7 @@ func TestMFA(t *testing.T) { require.NotEmpty(t, user.MFARecoveryCodes) // Disable MFA - err = service.DisableUserMFA(userCtx, "", time.Time{}, user.MFARecoveryCodes[0]) + err = service.DisableUserMFA(userCtx, "", mfaTime, user.MFARecoveryCodes[0]) require.NoError(t, err) _, user = updateContext()