JakeHillion/storj
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

satellite/console,web/satellite: Allow disabling MFA with recovery code

Browse Source 
This change allows users to disable multi-factor authentication
with a recovery code. Previously, users could only disable MFA
with a passcode.

Change-Id: Iec20bf7d8f6781182b81d1f17d9641491dbc8460
This commit is contained in:
Jeremy Wharton 2021-08-16 16:23:06 -05:00 committed by Jeremy Wharton
parent ef9a5210a4
commit 96e39018c7
10 changed files with 235 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
satellite/console/consoleweb/consoleapi/auth.go
View File

@ -84,7 +84,7 @@ func (a *Auth) Token(w http.ResponseWriter, r *http.Request) {
token, err := a.service.Token(ctx, tokenRequest)
if err != nil {
if !console.ErrMFAPasscodeRequired.Has(err) {
if !console.ErrMFAMissing.Has(err) {
a.log.Info("Error authenticating token request", zap.String("email", tokenRequest.Email), zap.Error(ErrAuthAPI.Wrap(err)))
}
a.serveJSONError(w, err)
@ -515,7 +515,8 @@ func (a *Auth) DisableUserMFA(w http.ResponseWriter, r *http.Request) {
defer mon.Task()(&ctx)(&err)
var data struct {
Passcode string `json:"passcode"`
Passcode string `json:"passcode"`
RecoveryCode string `json:"recoveryCode"`
}
err = json.NewDecoder(r.Body).Decode(&data)
if err != nil {
@ -523,7 +524,7 @@ func (a *Auth) DisableUserMFA(w http.ResponseWriter, r *http.Request) {
return
}
err = a.service.DisableUserMFA(ctx, data.Passcode, time.Now())
err = a.service.DisableUserMFA(ctx, data.Passcode, time.Now(), data.RecoveryCode)
if err != nil {
a.serveJSONError(w, err)
return
@ -605,12 +606,15 @@ func (a *Auth) getStatusCode(err error) int {
return http.StatusBadRequest
case console.ErrUnauthorized.Has(err), console.ErrRecoveryToken.Has(err):
return http.StatusUnauthorized
case console.ErrEmailUsed.Has(err):
case console.ErrEmailUsed.Has(err), console.ErrMFAConflict.Has(err):
return http.StatusConflict
case errors.Is(err, errNotImplemented):
return http.StatusNotImplemented
case console.ErrMFAPasscodeRequired.Has(err):
return http.StatusOK
case console.ErrMFAMissing.Has(err), console.ErrMFAPasscode.Has(err), console.ErrMFARecoveryCode.Has(err):
if console.ErrMFALogin.Has(err) {
return http.StatusOK
}
return http.StatusBadRequest
default:
return http.StatusInternalServerError
}
@ -630,6 +634,14 @@ func (a *Auth) getUserErrorMessage(err error) string {
return "The recovery token has expired"
}
return "The recovery token is invalid"
case console.ErrMFAMissing.Has(err):
return "A MFA passcode or recovery code is required"
case console.ErrMFAConflict.Has(err):
return "Expected either passcode or recovery code, but got both"
case console.ErrMFAPasscode.Has(err):
return "The MFA passcode is not valid or has expired"
case console.ErrMFARecoveryCode.Has(err):
return "The MFA recovery code is not valid or has been previously used"
case errors.Is(err, errNotImplemented):
return "The server is incapable of fulfilling the request"
default:

60
satellite/console/consoleweb/consoleapi/auth_test.go
View File

@ -257,15 +257,17 @@ func TestMFAEndpoints(t *testing.T) {
require.NotEmpty(t, token)
type data struct {
Passcode string `json:"passcode"`
Passcode string `json:"passcode"`
RecoveryCode string `json:"recoveryCode"`
}
doRequest := func(urlSuffix string, passcode string) *http.Response {
doRequest := func(urlSuffix string, passcode string, recoveryCode string) *http.Response {
urlLink := "http://" + sat.API.Console.Listener.Addr().String() + "/api/v0/auth/mfa" + urlSuffix
var buf io.Reader
body := &data{
Passcode: passcode,
Passcode: passcode,
RecoveryCode: recoveryCode,
}
bodyBytes, err := json.Marshal(body)
@ -291,17 +293,17 @@ func TestMFAEndpoints(t *testing.T) {
}
// Expect failure because MFA is not enabled.
result := doRequest("/generate-recovery-codes", "")
result := doRequest("/generate-recovery-codes", "", "")
require.Equal(t, http.StatusUnauthorized, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect failure due to not having generated a secret key.
result = doRequest("/enable", "123456")
result = doRequest("/enable", "123456", "")
require.Equal(t, http.StatusBadRequest, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect success when generating a secret key.
result = doRequest("/generate-secret-key", "")
result = doRequest("/generate-secret-key", "", "")
require.Equal(t, http.StatusOK, result.StatusCode)
var key string
@ -311,26 +313,26 @@ func TestMFAEndpoints(t *testing.T) {
require.NoError(t, result.Body.Close())
// Expect failure due to prodiving empty passcode.
result = doRequest("/enable", "")
result = doRequest("/enable", "", "")
require.Equal(t, http.StatusBadRequest, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect failure due to providing invalid passcode.
badCode, err := console.NewMFAPasscode(key, time.Now().Add(time.Hour))
require.NoError(t, err)
result = doRequest("/enable", badCode)
result = doRequest("/enable", badCode, "")
require.Equal(t, http.StatusBadRequest, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect success when providing valid passcode.
goodCode, err := console.NewMFAPasscode(key, time.Now())
require.NoError(t, err)
result = doRequest("/enable", goodCode)
result = doRequest("/enable", goodCode, "")
require.Equal(t, http.StatusOK, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect 10 recovery codes to be generated.
result = doRequest("/generate-recovery-codes", "")
result = doRequest("/generate-recovery-codes", "", "")
require.Equal(t, http.StatusOK, result.StatusCode)
var codes []string
@ -341,7 +343,7 @@ func TestMFAEndpoints(t *testing.T) {
// Expect no token due to missing passcode.
newToken, err := sat.API.Console.Service.Token(ctx, console.AuthUser{Email: user.Email, Password: user.FullName})
require.True(t, console.ErrMFAPasscodeRequired.Has(err))
require.True(t, console.ErrMFAMissing.Has(err))
require.Empty(t, newToken)
// Expect token when providing valid passcode.
@ -381,17 +383,47 @@ func TestMFAEndpoints(t *testing.T) {
}
// Expect failure due to disabling MFA with no passcode.
result = doRequest("/disable", "")
result = doRequest("/disable", "", "")
require.Equal(t, http.StatusBadRequest, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect failure due to disabling MFA with invalid passcode.
result = doRequest("/disable", badCode)
result = doRequest("/disable", badCode, "")
require.Equal(t, http.StatusBadRequest, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect failure when disabling due to providing both passcode and recovery code.
result = doRequest("/generate-recovery-codes", "", "")
err = json.NewDecoder(result.Body).Decode(&codes)
require.NoError(t, err)
require.NoError(t, result.Body.Close())
result = doRequest("/disable", goodCode, codes[0])
require.Equal(t, http.StatusConflict, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect success when disabling MFA with valid passcode.
result = doRequest("/disable", goodCode)
result = doRequest("/disable", goodCode, "")
require.Equal(t, http.StatusOK, result.StatusCode)
require.NoError(t, result.Body.Close())
// Expect success when disabling MFA with valid recovery code.
result = doRequest("/generate-secret-key", "", "")
err = json.NewDecoder(result.Body).Decode(&key)
require.NoError(t, err)
require.NoError(t, result.Body.Close())
goodCode, err = console.NewMFAPasscode(key, time.Now())
require.NoError(t, err)
result = doRequest("/enable", goodCode, "")
require.NoError(t, result.Body.Close())
result = doRequest("/generate-recovery-codes", "", "")
err = json.NewDecoder(result.Body).Decode(&codes)
require.NoError(t, err)
require.NoError(t, result.Body.Close())
result = doRequest("/disable", "", codes[0])
require.Equal(t, http.StatusOK, result.StatusCode)
require.NoError(t, result.Body.Close())
})

60
satellite/console/mfa.go
View File

@ -22,15 +22,28 @@ const (
// Error messages.
const (
mfaPasscodeInvalidErrMsg = "The MFA passcode is not valid or has expired"
mfaPasscodeRequiredErrMsg = "A MFA passcode or recovery code is required"
mfaRequiredErrMsg = "A MFA passcode or recovery code is required"
mfaRecoveryInvalidErrMsg = "The MFA recovery code is not valid or has been previously used"
mfaRecoveryGenerationErrMsg = "MFA recovery codes cannot be generated while MFA is disabled."
mfaConflictErrMsg = "Expected either passcode or recovery code, but got both"
)
var (
// ErrMFAPasscodeRequired is error type that occurs when a token request is incomplete
// ErrMFAMissing is error type that occurs when a request is incomplete
// due to missing MFA passcode and recovery code.
ErrMFAPasscodeRequired = errs.Class("MFA passcode required")
ErrMFAMissing = errs.Class("MFA code required")
// ErrMFAConflict is error type that occurs when both a passcode and recovery code are given.
ErrMFAConflict = errs.Class("MFA conflict")
// ErrMFALogin is error type caused by MFA that occurs when logging in / retrieving token.
ErrMFALogin = errs.Class("MFA login")
// ErrMFARecoveryCode is error type that represents usage of invalid MFA recovery code.
ErrMFARecoveryCode = errs.Class("MFA recovery code")
// ErrMFAPasscode is error type that represents usage of invalid MFA passcode.
ErrMFAPasscode = errs.Class("MFA passcode")
)
// NewMFAValidationOpts returns the options used to validate TOTP passcodes.
@ -83,10 +96,10 @@ func (s *Service) EnableUserMFA(ctx context.Context, passcode string, t time.Tim
valid, err := ValidateMFAPasscode(passcode, auth.User.MFASecretKey, t)
if err != nil {
return ErrValidation.Wrap(err)
return ErrValidation.Wrap(ErrMFAPasscode.Wrap(err))
}
if !valid {
return ErrValidation.New(mfaPasscodeInvalidErrMsg)
return ErrValidation.Wrap(ErrMFAPasscode.New(mfaPasscodeInvalidErrMsg))
}
auth.User.MFAEnabled = true
@ -99,7 +112,7 @@ func (s *Service) EnableUserMFA(ctx context.Context, passcode string, t time.Tim
}
// DisableUserMFA disables multi-factor authentication for the user if the given secret key and password are valid.
func (s *Service) DisableUserMFA(ctx context.Context, passcode string, t time.Time) (err error) {
func (s *Service) DisableUserMFA(ctx context.Context, passcode string, t time.Time, recoveryCode string) (err error) {
defer mon.Task()(&ctx)(&err)
auth, err := s.getAuthAndAuditLog(ctx, "disable MFA")
@ -107,12 +120,37 @@ func (s *Service) DisableUserMFA(ctx context.Context, passcode string, t time.Ti
return Error.Wrap(err)
}
valid, err := ValidateMFAPasscode(passcode, auth.User.MFASecretKey, t)
if err != nil {
return ErrValidation.Wrap(err)
user := &auth.User
if !user.MFAEnabled {
return nil
}
if !valid {
return ErrValidation.New(mfaPasscodeInvalidErrMsg)
if recoveryCode != "" && passcode != "" {
return ErrMFAConflict.New(mfaConflictErrMsg)
}
if recoveryCode != "" {
found := false
for _, code := range user.MFARecoveryCodes {
if code == recoveryCode {
found = true
break
}
}
if !found {
return ErrUnauthorized.Wrap(ErrMFARecoveryCode.New(mfaRecoveryInvalidErrMsg))
}
} else if passcode != "" {
valid, err := ValidateMFAPasscode(passcode, auth.User.MFASecretKey, t)
if err != nil {
return ErrValidation.Wrap(ErrMFAPasscode.Wrap(err))
}
if !valid {
return ErrValidation.Wrap(ErrMFAPasscode.New(mfaPasscodeInvalidErrMsg))
}
} else {
return ErrMFAMissing.New(mfaRequiredErrMsg)
}
auth.User.MFAEnabled = false

6
satellite/console/service.go
View File

@ -846,6 +846,10 @@ func (s *Service) Token(ctx context.Context, request AuthUser) (token string, er
}
if user.MFAEnabled {
if request.MFARecoveryCode != "" && request.MFAPasscode != "" {
return "", ErrMFAConflict.New(mfaConflictErrMsg)
}
if request.MFARecoveryCode != "" {
found := false
codeIndex := -1
@ -875,7 +879,7 @@ func (s *Service) Token(ctx context.Context, request AuthUser) (token string, er
return "", ErrUnauthorized.New(mfaPasscodeInvalidErrMsg)
}
} else {
return "", ErrMFAPasscodeRequired.New(mfaPasscodeRequiredErrMsg)
return "", ErrMFALogin.Wrap(ErrMFAMissing.New(mfaRequiredErrMsg))
}
}

53
satellite/console/service_test.go
View File

@ -384,7 +384,7 @@ func TestMFA(t *testing.T) {
// Expect no token due to lack of MFA passcode.
token, err := service.Token(ctx, request)
require.True(t, console.ErrMFAPasscodeRequired.Has(err))
require.True(t, console.ErrMFAMissing.Has(err))
require.Empty(t, token)
// Expect no token due to bad MFA passcode.
@ -441,7 +441,7 @@ func TestMFA(t *testing.T) {
require.NoError(t, err)
updateAuth()
err = service.DisableUserMFA(authCtx, badCode, time.Time{})
err = service.DisableUserMFA(authCtx, badCode, time.Time{}, "")
require.True(t, console.ErrValidation.Has(err))
updateAuth()
@ -450,13 +450,60 @@ func TestMFA(t *testing.T) {
require.NotEmpty(t, auth.User.MFARecoveryCodes)
})
t.Run("TestDisableUserMFAConflict", func(t *testing.T) {
// Expect MFA-disabling attempt to fail when providing both recovery code and passcode.
goodCode, err := console.NewMFAPasscode(key, time.Time{})
require.NoError(t, err)
updateAuth()
err = service.DisableUserMFA(authCtx, goodCode, time.Time{}, auth.User.MFARecoveryCodes[0])
require.True(t, console.ErrMFAConflict.Has(err))
updateAuth()
require.True(t, auth.User.MFAEnabled)
require.NotEmpty(t, auth.User.MFASecretKey)
require.NotEmpty(t, auth.User.MFARecoveryCodes)
})
t.Run("TestDisableUserMFAGoodPasscode", func(t *testing.T) {
// Expect MFA-disabling attempt to succeed when providing valid passcode.
goodCode, err := console.NewMFAPasscode(key, time.Time{})
require.NoError(t, err)
updateAuth()
err = service.DisableUserMFA(authCtx, goodCode, time.Time{})
err = service.DisableUserMFA(authCtx, goodCode, time.Time{}, "")
require.NoError(t, err)
updateAuth()
require.False(t, auth.User.MFAEnabled)
require.Empty(t, auth.User.MFASecretKey)
require.Empty(t, auth.User.MFARecoveryCodes)
})
t.Run("TestDisableUserMFAGoodRecoveryCode", func(t *testing.T) {
// Expect MFA-disabling attempt to succeed when providing valid recovery code.
// Enable MFA
key, err = service.ResetMFASecretKey(authCtx)
require.NoError(t, err)
goodCode, err := console.NewMFAPasscode(key, time.Time{})
require.NoError(t, err)
updateAuth()
err = service.EnableUserMFA(authCtx, goodCode, time.Time{})
require.NoError(t, err)
updateAuth()
_, err = service.ResetMFARecoveryCodes(authCtx)
require.NoError(t, err)
updateAuth()
require.True(t, auth.User.MFAEnabled)
require.NotEmpty(t, auth.User.MFASecretKey)
require.NotEmpty(t, auth.User.MFARecoveryCodes)
// Disable MFA
err = service.DisableUserMFA(authCtx, "", time.Time{}, auth.User.MFARecoveryCodes[0])
require.NoError(t, err)
updateAuth()

24
web/satellite/src/api/auth.ts
View File

@ -6,14 +6,14 @@ import { ErrorEmailUsed } from '@/api/errors/ErrorEmailUsed';
import { ErrorMFARequired } from '@/api/errors/ErrorMFARequired';
import { ErrorTooManyRequests } from '@/api/errors/ErrorTooManyRequests';
import { ErrorUnauthorized } from '@/api/errors/ErrorUnauthorized';
import { UpdatedUser, User } from '@/types/users';
import { UpdatedUser, User, UsersApi } from '@/types/users';
import { HttpClient } from '@/utils/httpClient';
/**
* AuthHttpApi is a console Auth API.
* Exposes all auth-related functionality
*/
export class AuthHttpApi {
export class AuthHttpApi implements UsersApi {
private readonly http: HttpClient = new HttpClient();
private readonly ROOT_PATH: string = '/api/v0/auth';
@ -313,10 +313,11 @@ export class AuthHttpApi {
*
* @throws Error
*/
public async disableUserMFA(passcode: string): Promise<void> {
public async disableUserMFA(passcode: string, recoveryCode: string): Promise<void> {
const path = `${this.ROOT_PATH}/mfa/disable`;
const body = {
passcode: passcode,
passcode: passcode || null,
recoveryCode: recoveryCode || null,
};
const response = await this.http.post(path, JSON.stringify(body));
@ -324,12 +325,17 @@ export class AuthHttpApi {
if (response.ok) {
return;
}
if (response.status === 401) {
throw new ErrorUnauthorized();
const result = await response.json();
if (!response.ok) {
const errMsg = result.error || 'Cannot disable MFA. Please try again later';
switch (response.status) {
case 401:
throw new ErrorUnauthorized(errMsg);
default:
throw new Error(errMsg);
}
}
throw new Error('Can not disable MFA. Please try again later');
}
/**

43
web/satellite/src/components/account/mfa/DisableMFAPopup.vue
View File

@ -10,7 +10,10 @@
</p>
<div class="disable-mfa__container__confirm">
<h2 class="disable-mfa__container__confirm__title">Confirm Authentication Code</h2>
<ConfirmMFAInput :on-input="onConfirmInput" :is-error="isError" />
<ConfirmMFAInput ref="mfaInput" :on-input="onConfirmInput" :is-error="isError" :is-recovery="isRecoveryCodeState" />
<span class="disable-mfa__container__confirm__toggle" @click="toggleRecoveryCodeState">
Or use {{ isRecoveryCodeState ? '2FA code' : 'recovery code' }}
</span>
</div>
<p class="disable-mfa__container__info">
After disabling 2FA, remove the authentication code from your TOTP app.
@ -29,7 +32,7 @@
width="50%"
height="44px"
:on-press="disable"
:is-disabled="!confirmPasscode || isLoading"
:is-disabled="!(request.recoveryCode || request.passcode) || isLoading"
/>
</div>
<div class="disable-mfa__container__close-container" @click="toggleModal">
@ -48,6 +51,11 @@ import VButton from '@/components/common/VButton.vue';
import CloseCrossIcon from '@/../static/images/common/closeCross.svg';
import { USER_ACTIONS } from '@/store/modules/users';
import { DisableMFARequest } from '@/types/users';
interface ClearInput {
clearInput(): void;
}
@Component({
components: {
@ -62,26 +70,41 @@ export default class DisableMFAPopup extends Vue {
public isError = false;
public isLoading = false;
public confirmPasscode = '';
public request = new DisableMFARequest();
public isRecoveryCodeState = false;
public $refs!: {
mfaInput: ConfirmMFAInput & ClearInput;
}
/**
* Sets confirmation passcode value from input.
*/
public onConfirmInput(value: string): void {
this.isError = false;
this.confirmPasscode = value;
this.isRecoveryCodeState ? this.request.recoveryCode = value : this.request.passcode = value;
}
/**
* Toggles whether the MFA recovery code page is shown.
*/
public toggleRecoveryCodeState(): void {
this.isError = false;
this.request.recoveryCode = this.request.passcode = '';
this.$refs.mfaInput.clearInput();
this.isRecoveryCodeState = !this.isRecoveryCodeState;
}
/**
* Disables user MFA.
*/
public async disable(): Promise<void> {
if (!this.confirmPasscode || this.isLoading || this.isError) return;
if (!(this.request.recoveryCode || this.request.passcode) || this.isLoading || this.isError) return;
this.isLoading = true;
try {
await this.$store.dispatch(USER_ACTIONS.DISABLE_USER_MFA, this.confirmPasscode);
await this.$store.dispatch(USER_ACTIONS.DISABLE_USER_MFA, this.request);
await this.$store.dispatch(USER_ACTIONS.GET);
await this.$notify.success('MFA was disabled successfully');
@ -154,6 +177,14 @@ export default class DisableMFAPopup extends Vue {
color: #000;
margin-bottom: 20px;
}
&__toggle {
font-size: 16px;
color: #0068dc;
cursor: pointer;
margin-top: 20px;
text-align: center;
}
}
&__info {

6
web/satellite/src/store/modules/users.ts
View File

@ -2,7 +2,7 @@
// See LICENSE for copying information.
import { StoreModule } from '@/store';
import { UpdatedUser, User, UsersApi } from '@/types/users';
import { DisableMFARequest, UpdatedUser, User, UsersApi } from '@/types/users';
import { MetaUtils } from '@/utils/meta';
export const USER_ACTIONS = {
@ -104,8 +104,8 @@ export function makeUsersModule(api: UsersApi): StoreModule<UsersState> {
return user;
},
[DISABLE_USER_MFA]: async function (_, passcode: string): Promise<void> {
await api.disableUserMFA(passcode);
[DISABLE_USER_MFA]: async function (_, request: DisableMFARequest): Promise<void> {
await api.disableUserMFA(request.passcode, request.recoveryCode);
},
[ENABLE_USER_MFA]: async function (_, passcode: string): Promise<void> {
await api.enableUserMFA(passcode);

12
web/satellite/src/types/users.ts
View File

@ -30,7 +30,7 @@ export interface UsersApi {
*
* @throws Error
*/
disableUserMFA(code: string): Promise<void>;
disableUserMFA(passcode: string, recoveryCode: string): Promise<void>;
/**
* Generate user's MFA secret.
*
@ -94,3 +94,13 @@ export class UpdatedUser {
return !!this.fullName;
}
}
/**
* DisableMFARequest represents a request to disable multi-factor authentication.
*/
export class DisableMFARequest {
public constructor(
public passcode: string = '',
public recoveryCode: string = '',
) {}
}

2
web/satellite/tests/unit/mock/api/users.ts
View File

@ -25,7 +25,7 @@ export class UsersApiMock implements UsersApi {
return Promise.resolve();
}
public disableUserMFA(_: string): Promise<void> {
public disableUserMFA(_passcode: string, _recoveryCode: string): Promise<void> {
return Promise.resolve();
}