From 9d13c649a2aff61f51ea1e0cf37b62729d68e4f8 Mon Sep 17 00:00:00 2001 From: Jeremy Wharton Date: Thu, 18 Nov 2021 13:55:37 -0500 Subject: [PATCH] satellite/{console,satellitedb}: Forbid creating users with used email This change disallows creation of users possessing the same email. If a user attempts to create an account with an email address that's already used - whether it belongs to an active account or not - he will be notified of unsuccessful account creation. If he attempts to log in using an email address belonging to an inactive account, he will be presented with a link allowing him to re-send the verification email. Attempting to register with an email address belonging to an existing account triggers a password reset email. Change-Id: Iefd8c3bef00ecb1dd9e8504594607aa0dca7d82e --- .../console/consoleweb/consoleapi/auth.go | 269 +++++++++----- .../consoleweb/consoleapi/auth_test.go | 131 ++++++- .../console/consoleweb/consoleapi/common.go | 51 +++ .../consoleweb/consoleql/mutation_test.go | 6 +- .../consoleweb/consoleql/query_test.go | 6 +- satellite/console/consoleweb/server.go | 2 +- satellite/console/mfa.go | 7 +- satellite/console/service.go | 88 ++--- satellite/console/service_test.go | 10 +- satellite/mailservice/service.go | 10 +- scripts/testdata/satellite-config.yaml.lock | 6 + web/satellite/src/api/auth.ts | 45 ++- .../src/api/errors/ErrorEmailUsed.ts | 11 - .../components/common/RegistrationSuccess.vue | 64 ++-- web/satellite/src/router/index.ts | 8 + web/satellite/src/views/ActivateAccount.vue | 210 +++++++++++ web/satellite/src/views/LoginArea.vue | 328 ++++++++---------- web/satellite/src/views/RegisterArea.vue | 14 +- 18 files changed, 865 insertions(+), 401 deletions(-) delete mode 100644 web/satellite/src/api/errors/ErrorEmailUsed.ts create mode 100644 web/satellite/src/views/ActivateAccount.vue diff --git a/satellite/console/consoleweb/consoleapi/auth.go b/satellite/console/consoleweb/consoleapi/auth.go index 00dd9be31..5aaae7878 100644 --- a/satellite/console/consoleweb/consoleapi/auth.go +++ b/satellite/console/consoleweb/consoleapi/auth.go @@ -41,31 +41,37 @@ var ( // Auth is an api controller that exposes all auth functionality. type Auth struct { - log *zap.Logger - ExternalAddress string - LetUsKnowURL string - TermsAndConditionsURL string - ContactInfoURL string - service *console.Service - analytics *analytics.Service - mailService *mailservice.Service - cookieAuth *consolewebauth.CookieAuth - partners *rewards.PartnersService + log *zap.Logger + ExternalAddress string + LetUsKnowURL string + TermsAndConditionsURL string + ContactInfoURL string + PasswordRecoveryURL string + CancelPasswordRecoveryURL string + ActivateAccountURL string + service *console.Service + analytics *analytics.Service + mailService *mailservice.Service + cookieAuth *consolewebauth.CookieAuth + partners *rewards.PartnersService } // NewAuth is a constructor for api auth controller. func NewAuth(log *zap.Logger, service *console.Service, mailService *mailservice.Service, cookieAuth *consolewebauth.CookieAuth, partners *rewards.PartnersService, analytics *analytics.Service, externalAddress string, letUsKnowURL string, termsAndConditionsURL string, contactInfoURL string) *Auth { return &Auth{ - log: log, - ExternalAddress: externalAddress, - LetUsKnowURL: letUsKnowURL, - TermsAndConditionsURL: termsAndConditionsURL, - ContactInfoURL: contactInfoURL, - service: service, - mailService: mailService, - cookieAuth: cookieAuth, - partners: partners, - analytics: analytics, + log: log, + ExternalAddress: externalAddress, + LetUsKnowURL: letUsKnowURL, + TermsAndConditionsURL: termsAndConditionsURL, + ContactInfoURL: contactInfoURL, + PasswordRecoveryURL: externalAddress + "password-recovery/", + CancelPasswordRecoveryURL: externalAddress + "cancel-password-recovery/", + ActivateAccountURL: externalAddress + "activation/", + service: service, + mailService: mailService, + cookieAuth: cookieAuth, + partners: partners, + analytics: analytics, } } @@ -84,10 +90,12 @@ func (a *Auth) Token(w http.ResponseWriter, r *http.Request) { token, err := a.service.Token(ctx, tokenRequest) if err != nil { - if !console.ErrMFAMissing.Has(err) { + if console.ErrMFAMissing.Has(err) { + serveCustomJSONError(a.log, w, 200, err, a.getUserErrorMessage(err)) + } else { a.log.Info("Error authenticating token request", zap.String("email", tokenRequest.Email), zap.Error(ErrAuthAPI.Wrap(err))) + a.serveJSONError(w, err) } - a.serveJSONError(w, err) return } @@ -112,6 +120,7 @@ func (a *Auth) Logout(w http.ResponseWriter, r *http.Request) { } // Register creates new user, sends activation e-mail. +// If a user with the given e-mail address already exists, a password reset e-mail is sent instead. func (a *Auth) Register(w http.ResponseWriter, r *http.Request) { ctx := r.Context() var err error @@ -130,6 +139,17 @@ func (a *Auth) Register(w http.ResponseWriter, r *http.Request) { return } + var userID uuid.UUID + defer func() { + if err == nil { + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode(userID) + if err != nil { + a.log.Error("registration handler could not encode userID", zap.Error(ErrAuthAPI.Wrap(err))) + } + } + }() + var registerData struct { FullName string `json:"fullName"` ShortName string `json:"shortName"` @@ -154,60 +174,107 @@ func (a *Auth) Register(w http.ResponseWriter, r *http.Request) { return } - secret, err := console.RegistrationSecretFromBase64(registerData.SecretInput) - if err != nil { + verified, unverified, err := a.service.GetUserByEmailWithUnverified(ctx, registerData.Email) + if err != nil && !console.ErrEmailNotFound.Has(err) { a.serveJSONError(w, err) return } - if registerData.Partner != "" { - registerData.UserAgent = []byte(registerData.Partner) - } + if verified != nil { + recoveryToken, err := a.service.GeneratePasswordRecoveryToken(ctx, verified.ID) + if err != nil { + a.serveJSONError(w, err) + return + } - ip, err := web.GetRequestIP(r) - if err != nil { - a.serveJSONError(w, err) + userName := verified.ShortName + if verified.ShortName == "" { + userName = verified.FullName + } + + a.mailService.SendRenderedAsync( + ctx, + []post.Address{{Address: verified.Email, Name: userName}}, + &consoleql.ForgotPasswordEmail{ + Origin: a.ExternalAddress, + UserName: userName, + ResetLink: a.PasswordRecoveryURL + "?token=" + recoveryToken, + CancelPasswordRecoveryLink: a.CancelPasswordRecoveryURL + "?token=" + recoveryToken, + LetUsKnowURL: a.LetUsKnowURL, + ContactInfoURL: a.ContactInfoURL, + TermsAndConditionsURL: a.TermsAndConditionsURL, + }, + ) + userID = verified.ID return } - user, err := a.service.CreateUser(ctx, - console.CreateUser{ - FullName: registerData.FullName, - ShortName: registerData.ShortName, - Email: registerData.Email, - PartnerID: registerData.PartnerID, - UserAgent: registerData.UserAgent, - Password: registerData.Password, - IsProfessional: registerData.IsProfessional, - Position: registerData.Position, - CompanyName: registerData.CompanyName, - EmployeeCount: registerData.EmployeeCount, - HaveSalesContact: registerData.HaveSalesContact, - RecaptchaResponse: registerData.RecaptchaResponse, - IP: ip, - }, - secret, - ) - if err != nil { - a.serveJSONError(w, err) - return - } + var user *console.User + if len(unverified) > 0 { + user = &unverified[0] + } else { + secret, err := console.RegistrationSecretFromBase64(registerData.SecretInput) + if err != nil { + a.serveJSONError(w, err) + return + } - trackCreateUserFields := analytics.TrackCreateUserFields{ - ID: user.ID, - AnonymousID: loadSession(r), - FullName: user.FullName, - Email: user.Email, - Type: analytics.Personal, + if registerData.Partner != "" { + registerData.UserAgent = []byte(registerData.Partner) + info, err := a.partners.ByName(ctx, registerData.Partner) + if err != nil { + a.log.Warn("Invalid partner name", zap.String("Partner name", registerData.Partner), zap.String("User email", registerData.Email), zap.Error(err)) + } else { + registerData.PartnerID = info.ID + } + } + + ip, err := web.GetRequestIP(r) + if err != nil { + a.serveJSONError(w, err) + return + } + + user, err = a.service.CreateUser(ctx, + console.CreateUser{ + FullName: registerData.FullName, + ShortName: registerData.ShortName, + Email: registerData.Email, + PartnerID: registerData.PartnerID, + UserAgent: registerData.UserAgent, + Password: registerData.Password, + IsProfessional: registerData.IsProfessional, + Position: registerData.Position, + CompanyName: registerData.CompanyName, + EmployeeCount: registerData.EmployeeCount, + HaveSalesContact: registerData.HaveSalesContact, + RecaptchaResponse: registerData.RecaptchaResponse, + IP: ip, + }, + secret, + ) + if err != nil { + a.serveJSONError(w, err) + return + } + + trackCreateUserFields := analytics.TrackCreateUserFields{ + ID: user.ID, + AnonymousID: loadSession(r), + FullName: user.FullName, + Email: user.Email, + Type: analytics.Personal, + } + if user.IsProfessional { + trackCreateUserFields.Type = analytics.Professional + trackCreateUserFields.EmployeeCount = user.EmployeeCount + trackCreateUserFields.CompanyName = user.CompanyName + trackCreateUserFields.JobTitle = user.Position + trackCreateUserFields.HaveSalesContact = user.HaveSalesContact + } + a.analytics.TrackCreateUser(trackCreateUserFields) } - if user.IsProfessional { - trackCreateUserFields.Type = analytics.Professional - trackCreateUserFields.EmployeeCount = user.EmployeeCount - trackCreateUserFields.CompanyName = user.CompanyName - trackCreateUserFields.JobTitle = user.Position - trackCreateUserFields.HaveSalesContact = user.HaveSalesContact - } - a.analytics.TrackCreateUser(trackCreateUserFields) + userID = user.ID token, err := a.service.GenerateActivationToken(ctx, user.ID, user.Email) if err != nil { @@ -215,7 +282,7 @@ func (a *Auth) Register(w http.ResponseWriter, r *http.Request) { return } - link := a.ExternalAddress + "activation/?token=" + token + link := a.ActivateAccountURL + "?token=" + token userName := user.ShortName if user.ShortName == "" { userName = user.FullName @@ -230,13 +297,6 @@ func (a *Auth) Register(w http.ResponseWriter, r *http.Request) { UserName: userName, }, ) - - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(user.ID) - if err != nil { - a.log.Error("registration handler could not encode userID", zap.Error(ErrAuthAPI.Wrap(err))) - return - } } // loadSession looks for a cookie for the session id. @@ -395,9 +455,8 @@ func (a *Auth) ForgotPassword(w http.ResponseWriter, r *http.Request) { return } - user, err := a.service.GetUserByEmail(ctx, email) - if err != nil { - a.serveJSONError(w, err) + user, _, err := a.service.GetUserByEmailWithUnverified(ctx, email) + if err != nil || user == nil { return } @@ -407,8 +466,8 @@ func (a *Auth) ForgotPassword(w http.ResponseWriter, r *http.Request) { return } - passwordRecoveryLink := a.ExternalAddress + "password-recovery/?token=" + recoveryToken - cancelPasswordRecoveryLink := a.ExternalAddress + "cancel-password-recovery/?token=" + recoveryToken + passwordRecoveryLink := a.PasswordRecoveryURL + "?token=" + recoveryToken + cancelPasswordRecoveryLink := a.CancelPasswordRecoveryURL + "?token=" + recoveryToken userName := user.ShortName if user.ShortName == "" { userName = user.FullName @@ -433,43 +492,66 @@ func (a *Auth) ForgotPassword(w http.ResponseWriter, r *http.Request) { ) } -// ResendEmail generates activation token by userID and sends email account activation email to user. +// ResendEmail generates activation token by e-mail address and sends email account activation email to user. +// If the account is already activated, a password reset e-mail is sent instead. func (a *Auth) ResendEmail(w http.ResponseWriter, r *http.Request) { ctx := r.Context() var err error defer mon.Task()(&ctx)(&err) params := mux.Vars(r) - id, ok := params["id"] + email, ok := params["email"] if !ok { - a.serveJSONError(w, err) return } - userID, err := uuid.FromString(id) + verified, unverified, err := a.service.GetUserByEmailWithUnverified(ctx, email) if err != nil { - a.serveJSONError(w, err) return } - user, err := a.service.GetUser(ctx, userID) - if err != nil { - a.serveJSONError(w, err) + if verified != nil { + recoveryToken, err := a.service.GeneratePasswordRecoveryToken(ctx, verified.ID) + if err != nil { + a.serveJSONError(w, err) + return + } + + userName := verified.ShortName + if verified.ShortName == "" { + userName = verified.FullName + } + + a.mailService.SendRenderedAsync( + ctx, + []post.Address{{Address: verified.Email, Name: userName}}, + &consoleql.ForgotPasswordEmail{ + Origin: a.ExternalAddress, + UserName: userName, + ResetLink: a.PasswordRecoveryURL + "?token=" + recoveryToken, + CancelPasswordRecoveryLink: a.CancelPasswordRecoveryURL + "?token=" + recoveryToken, + LetUsKnowURL: a.LetUsKnowURL, + ContactInfoURL: a.ContactInfoURL, + TermsAndConditionsURL: a.TermsAndConditionsURL, + }, + ) return } + user := unverified[0] + token, err := a.service.GenerateActivationToken(ctx, user.ID, user.Email) if err != nil { a.serveJSONError(w, err) return } - link := a.ExternalAddress + "activation/?token=" + token userName := user.ShortName if user.ShortName == "" { userName = user.FullName } + link := a.ActivateAccountURL + "?token=" + token contactInfoURL := a.ContactInfoURL termsAndConditionsURL := a.TermsAndConditionsURL @@ -602,18 +684,15 @@ func (a *Auth) serveJSONError(w http.ResponseWriter, err error) { // getStatusCode returns http.StatusCode depends on console error class. func (a *Auth) getStatusCode(err error) int { switch { - case console.ErrValidation.Has(err), console.ErrRecaptcha.Has(err): + case console.ErrValidation.Has(err), console.ErrRecaptcha.Has(err), console.ErrMFAMissing.Has(err): return http.StatusBadRequest - case console.ErrUnauthorized.Has(err), console.ErrRecoveryToken.Has(err): + case console.ErrUnauthorized.Has(err), console.ErrRecoveryToken.Has(err), console.ErrLoginCredentials.Has(err): return http.StatusUnauthorized case console.ErrEmailUsed.Has(err), console.ErrMFAConflict.Has(err): return http.StatusConflict case errors.Is(err, errNotImplemented): return http.StatusNotImplemented - case console.ErrMFAMissing.Has(err), console.ErrMFAPasscode.Has(err), console.ErrMFARecoveryCode.Has(err): - if console.ErrMFALogin.Has(err) { - return http.StatusOK - } + case console.ErrMFAPasscode.Has(err), console.ErrMFARecoveryCode.Has(err): return http.StatusBadRequest default: return http.StatusInternalServerError @@ -642,6 +721,8 @@ func (a *Auth) getUserErrorMessage(err error) string { 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 console.ErrLoginCredentials.Has(err): + return "Your login credentials are incorrect, please try again" case errors.Is(err, errNotImplemented): return "The server is incapable of fulfilling the request" default: diff --git a/satellite/console/consoleweb/consoleapi/auth_test.go b/satellite/console/consoleweb/consoleapi/auth_test.go index 816632a48..02e878319 100644 --- a/satellite/console/consoleweb/consoleapi/auth_test.go +++ b/satellite/console/consoleweb/consoleapi/auth_test.go @@ -5,6 +5,7 @@ package consoleapi_test import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -26,6 +27,7 @@ import ( "storj.io/common/testcontext" "storj.io/common/uuid" + "storj.io/storj/private/post" "storj.io/storj/private/testplanet" "storj.io/storj/satellite" "storj.io/storj/satellite/console" @@ -453,7 +455,7 @@ func TestMFAEndpoints(t *testing.T) { Password: user.FullName, MFARecoveryCode: "BADCODE", }) - require.True(t, console.ErrUnauthorized.Has(err)) + require.True(t, console.ErrMFARecoveryCode.Has(err)) require.Empty(t, newToken) for _, code := range codes { @@ -470,7 +472,7 @@ func TestMFAEndpoints(t *testing.T) { // Expect error when providing expired recovery code. newToken, err = sat.API.Console.Service.Token(ctx, opts) - require.True(t, console.ErrUnauthorized.Has(err)) + require.True(t, console.ErrMFARecoveryCode.Has(err)) require.Empty(t, newToken) } @@ -564,3 +566,128 @@ func TestResetPasswordEndpoint(t *testing.T) { require.Equal(t, http.StatusOK, tryReset(tokenStr, newPass)) }) } + +type EmailVerifier struct { + Data consoleapi.ContextChannel + Context context.Context +} + +func (v *EmailVerifier) SendEmail(ctx context.Context, msg *post.Message) error { + body := "" + for _, part := range msg.Parts { + body += part.Content + } + return v.Data.Send(v.Context, body) +} + +func (v *EmailVerifier) FromAddress() post.Address { + return post.Address{} +} + +func TestRegistrationEmail(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 0, UplinkCount: 0, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + sat := planet.Satellites[0] + + jsonBody, err := json.Marshal(map[string]interface{}{ + "fullName": "Test User", + "shortName": "Test", + "email": "test@mail.test", + "password": "123a123", + }) + require.NoError(t, err) + + register := func() string { + url := planet.Satellites[0].ConsoleURL() + "/api/v0/auth/register" + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(jsonBody)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + + result, err := http.DefaultClient.Do(req) + require.NoError(t, err) + require.Equal(t, http.StatusOK, result.StatusCode) + + var userID string + require.NoError(t, json.NewDecoder(result.Body).Decode(&userID)) + require.NoError(t, result.Body.Close()) + + return userID + } + + sender := &EmailVerifier{Context: ctx} + sat.API.Mail.Service.Sender = sender + + // Registration attempts using new e-mail address should send activation e-mail. + userID := register() + body, err := sender.Data.Get(ctx) + require.NoError(t, err) + require.Contains(t, body, "/activation") + + // Registration attempts using existing but unverified e-mail address should send activation e-mail. + newUserID := register() + require.Equal(t, userID, newUserID) + body, err = sender.Data.Get(ctx) + require.NoError(t, err) + require.Contains(t, body, "/activation") + + // Registration attempts using existing and verified e-mail address should send password reset e-mail. + userUUID, err := uuid.FromString(userID) + require.NoError(t, err) + user, err := sat.DB.Console().Users().Get(ctx, userUUID) + require.NoError(t, err) + + user.Status = console.Active + require.NoError(t, sat.DB.Console().Users().Update(ctx, user)) + + newUserID = register() + require.Equal(t, userID, newUserID) + body, err = sender.Data.Get(ctx) + require.NoError(t, err) + require.Contains(t, body, "/password-recovery") + }) +} + +func TestResendActivationEmail(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 0, UplinkCount: 0, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + sat := planet.Satellites[0] + usersRepo := sat.DB.Console().Users() + + user, err := sat.AddUser(ctx, console.CreateUser{ + FullName: "Test User", + Email: "test@mail.test", + }, 1) + require.NoError(t, err) + + resendEmail := func() { + url := planet.Satellites[0].ConsoleURL() + "/api/v0/auth/resend-email/" + user.Email + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBufferString(user.Email)) + require.NoError(t, err) + + result, err := http.DefaultClient.Do(req) + require.NoError(t, err) + require.NoError(t, result.Body.Close()) + require.Equal(t, http.StatusOK, result.StatusCode) + } + + sender := &EmailVerifier{Context: ctx} + sat.API.Mail.Service.Sender = sender + + // Expect password reset e-mail to be sent when using verified e-mail address. + resendEmail() + body, err := sender.Data.Get(ctx) + require.NoError(t, err) + require.Contains(t, body, "/password-recovery") + + // Expect activation e-mail to be sent when using unverified e-mail address. + user.Status = console.Inactive + require.NoError(t, usersRepo.Update(ctx, user)) + + resendEmail() + body, err = sender.Data.Get(ctx) + require.NoError(t, err) + require.Contains(t, body, "/activation") + }) +} diff --git a/satellite/console/consoleweb/consoleapi/common.go b/satellite/console/consoleweb/consoleapi/common.go index 28559d134..ca3313a8c 100644 --- a/satellite/console/consoleweb/consoleapi/common.go +++ b/satellite/console/consoleweb/consoleapi/common.go @@ -4,8 +4,10 @@ package consoleapi import ( + "context" "encoding/json" "net/http" + "sync" "github.com/zeebo/errs" "go.uber.org/zap" @@ -48,3 +50,52 @@ func serveCustomJSONError(log *zap.Logger, w http.ResponseWriter, status int, er log.Error("failed to write json error response", zap.Error(ErrUtils.Wrap(err))) } } + +// ContextChannel is a generic, context-aware channel. +type ContextChannel struct { + mu sync.Mutex + channel chan interface{} + initialized bool +} + +// Get waits until a value is sent and returns it, or returns an error if the context has closed. +func (c *ContextChannel) Get(ctx context.Context) (interface{}, error) { + c.initialize() + select { + case val := <-c.channel: + return val, nil + default: + select { + case <-ctx.Done(): + return nil, ErrUtils.New("context closed") + case val := <-c.channel: + return val, nil + } + } +} + +// Send waits until a value can be sent and sends it, or returns an error if the context has closed. +func (c *ContextChannel) Send(ctx context.Context, val interface{}) error { + c.initialize() + select { + case c.channel <- val: + return nil + default: + select { + case <-ctx.Done(): + return ErrUtils.New("context closed") + case c.channel <- val: + return nil + } + } +} + +func (c *ContextChannel) initialize() { + c.mu.Lock() + defer c.mu.Unlock() + if c.initialized { + return + } + c.channel = make(chan interface{}) + c.initialized = true +} diff --git a/satellite/console/consoleweb/consoleql/mutation_test.go b/satellite/console/consoleweb/consoleql/mutation_test.go index 5a4d7e96e..5e77b0556 100644 --- a/satellite/console/consoleweb/consoleql/mutation_test.go +++ b/satellite/console/consoleweb/consoleql/mutation_test.go @@ -103,7 +103,11 @@ func TestGraphqlMutation(t *testing.T) { partnersService, paymentsService.Accounts(), analyticsService, - console.Config{PasswordCost: console.TestPasswordCost, DefaultProjectLimit: 5}, + console.Config{ + PasswordCost: console.TestPasswordCost, + DefaultProjectLimit: 5, + TokenExpirationTime: 24 * time.Hour, + }, ) require.NoError(t, err) diff --git a/satellite/console/consoleweb/consoleql/query_test.go b/satellite/console/consoleweb/consoleql/query_test.go index 3112ea51e..a5b68535d 100644 --- a/satellite/console/consoleweb/consoleql/query_test.go +++ b/satellite/console/consoleweb/consoleql/query_test.go @@ -87,7 +87,11 @@ func TestGraphqlQuery(t *testing.T) { partnersService, paymentsService.Accounts(), analyticsService, - console.Config{PasswordCost: console.TestPasswordCost, DefaultProjectLimit: 5}, + console.Config{ + PasswordCost: console.TestPasswordCost, + DefaultProjectLimit: 5, + TokenExpirationTime: 24 * time.Hour, + }, ) require.NoError(t, err) diff --git a/satellite/console/consoleweb/server.go b/satellite/console/consoleweb/server.go index 7cb8a6196..e6f4d86bb 100644 --- a/satellite/console/consoleweb/server.go +++ b/satellite/console/consoleweb/server.go @@ -240,7 +240,7 @@ func NewServer(logger *zap.Logger, config Config, service *console.Service, mail authRouter.Handle("/token", server.ipRateLimiter.Limit(http.HandlerFunc(authController.Token))).Methods(http.MethodPost) authRouter.Handle("/register", server.ipRateLimiter.Limit(http.HandlerFunc(authController.Register))).Methods(http.MethodPost, http.MethodOptions) authRouter.Handle("/forgot-password/{email}", server.ipRateLimiter.Limit(http.HandlerFunc(authController.ForgotPassword))).Methods(http.MethodPost) - authRouter.Handle("/resend-email/{id}", server.ipRateLimiter.Limit(http.HandlerFunc(authController.ResendEmail))).Methods(http.MethodPost) + authRouter.Handle("/resend-email/{email}", server.ipRateLimiter.Limit(http.HandlerFunc(authController.ResendEmail))).Methods(http.MethodPost) authRouter.Handle("/reset-password", server.ipRateLimiter.Limit(http.HandlerFunc(authController.ResetPassword))).Methods(http.MethodPost) paymentController := consoleapi.NewPayments(logger, service) diff --git a/satellite/console/mfa.go b/satellite/console/mfa.go index 4334038cb..18b9ac778 100644 --- a/satellite/console/mfa.go +++ b/satellite/console/mfa.go @@ -30,15 +30,12 @@ const ( var ( // ErrMFAMissing is error type that occurs when a request is incomplete - // due to missing MFA passcode and recovery code. - ErrMFAMissing = errs.Class("MFA code required") + // due to missing MFA passcode or recovery code. + ErrMFAMissing = errs.Class("MFA credentials missing") // 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") diff --git a/satellite/console/service.go b/satellite/console/service.go index afbc7c4c5..5ac468f72 100644 --- a/satellite/console/service.go +++ b/satellite/console/service.go @@ -6,8 +6,6 @@ package console import ( "context" "crypto/subtle" - "database/sql" - "errors" "fmt" "net/mail" "sort" @@ -38,10 +36,6 @@ const ( // maxLimit specifies the limit for all paged queries. maxLimit = 50 - // TokenExpirationTime specifies the expiration time for - // auth tokens, account recovery tokens, and activation tokens. - TokenExpirationTime = 24 * time.Hour - // TestPasswordCost is the hashing complexity to use for testing. TestPasswordCost = bcrypt.MinCost ) @@ -50,14 +44,16 @@ const ( const ( unauthorizedErrMsg = "You are not authorized to perform this action" emailUsedErrMsg = "This email is already in use, try another" + emailNotFoundErrMsg = "There are no users with the specified email" passwordRecoveryTokenIsExpiredErrMsg = "Your password recovery link has expired, please request another one" - credentialsErrMsg = "Your email or password was incorrect, please try again" + credentialsErrMsg = "Your login credentials are incorrect, please try again" passwordIncorrectErrMsg = "Your password needs at least %d characters long" projectOwnerDeletionForbiddenErrMsg = "%s is a project owner and can not be deleted" apiKeyWithNameExistsErrMsg = "An API Key with this name already exists in this project, please use a different name" apiKeyWithNameDoesntExistErrMsg = "An API Key with this name doesn't exist in this project." teamMemberDoesNotExistErrMsg = `There is no account on this Satellite for the user(s) you have entered. Please add team members with active accounts` + activationTokenExpiredErrMsg = "This activation token has expired, please request another one" usedRegTokenErrMsg = "This registration token has already been used" projLimitErrMsg = "Sorry, project creation is limited for your account. Please contact support!" @@ -79,9 +75,15 @@ var ( // ErrUsage is error type of project usage. ErrUsage = errs.Class("project usage") + // ErrLoginCredentials occurs when provided invalid login credentials. + ErrLoginCredentials = errs.Class("login credentials") + // ErrEmailUsed is error type that occurs on repeating auth attempts with email. ErrEmailUsed = errs.Class("email used") + // ErrEmailNotFound occurs when no users have the specified email. + ErrEmailNotFound = errs.Class("email not found") + // ErrNoAPIKey is error type that occurs when there is no api key found. ErrNoAPIKey = errs.Class("no api key found") @@ -128,9 +130,10 @@ func init() { // Config keeps track of core console service configuration parameters. type Config struct { - PasswordCost int `help:"password hashing cost (0=automatic)" testDefault:"4" default:"0"` - OpenRegistrationEnabled bool `help:"enable open registration" default:"false" testDefault:"true"` - DefaultProjectLimit int `help:"default project limits for users" default:"1" testDefault:"5"` + PasswordCost int `help:"password hashing cost (0=automatic)" testDefault:"4" default:"0"` + OpenRegistrationEnabled bool `help:"enable open registration" default:"false" testDefault:"true"` + DefaultProjectLimit int `help:"default project limits for users" default:"1" testDefault:"5"` + TokenExpirationTime time.Duration `help:"expiration time for auth tokens, account recovery tokens, and activation tokens" default:"24h"` UsageLimits UsageLimitsConfig Recaptcha RecaptchaConfig } @@ -590,13 +593,13 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R return nil, ErrRegToken.Wrap(err) } - u, err = s.store.Users().GetByEmail(ctx, user.Email) - if err == nil { - return nil, ErrEmailUsed.New(emailUsedErrMsg) - } - if !errors.Is(err, sql.ErrNoRows) { + verified, unverified, err := s.store.Users().GetByEmailWithUnverified(ctx, user.Email) + if err != nil { return nil, Error.Wrap(err) } + if verified != nil || len(unverified) != 0 { + return nil, ErrEmailUsed.New(emailUsedErrMsg) + } hash, err := bcrypt.GenerateFromPassword([]byte(user.Password), s.config.PasswordCost) if err != nil { @@ -678,7 +681,7 @@ func (s *Service) GenerateActivationToken(ctx context.Context, id uuid.UUID, ema claims := &consoleauth.Claims{ ID: id, Email: email, - Expiration: time.Now().Add(time.Hour * 24), + Expiration: time.Now().Add(s.config.TokenExpirationTime), } return s.createToken(ctx, claims) @@ -720,6 +723,10 @@ func (s *Service) ActivateAccount(ctx context.Context, activationToken string) ( return "", err } + if time.Now().After(claims.Expiration) { + return "", ErrTokenExpiration.New(activationTokenExpiredErrMsg) + } + _, err = s.store.Users().GetByEmail(ctx, claims.Email) if err == nil { return "", ErrEmailUsed.New(emailUsedErrMsg) @@ -730,12 +737,6 @@ func (s *Service) ActivateAccount(ctx context.Context, activationToken string) ( return "", Error.Wrap(err) } - now := time.Now() - - if now.After(user.CreatedAt.Add(TokenExpirationTime)) { - return "", ErrTokenExpiration.Wrap(err) - } - user.Status = Active err = s.store.Users().Update(ctx, user) if err != nil { @@ -748,7 +749,7 @@ func (s *Service) ActivateAccount(ctx context.Context, activationToken string) ( // now that the account is activated, create a token to be stored in a cookie to log the user in. claims = &consoleauth.Claims{ ID: user.ID, - Expiration: time.Now().Add(TokenExpirationTime), + Expiration: time.Now().Add(s.config.TokenExpirationTime), } token, err = s.createToken(ctx, claims) @@ -784,7 +785,7 @@ func (s *Service) ResetPassword(ctx context.Context, resetPasswordToken, passwor return Error.Wrap(err) } - if t.Sub(token.CreatedAt) > TokenExpirationTime { + if t.Sub(token.CreatedAt) > s.config.TokenExpirationTime { return ErrRecoveryToken.Wrap(ErrTokenExpiration.New(passwordRecoveryTokenIsExpiredErrMsg)) } @@ -824,14 +825,14 @@ func (s *Service) RevokeResetPasswordToken(ctx context.Context, resetPasswordTok func (s *Service) Token(ctx context.Context, request AuthUser) (token string, err error) { defer mon.Task()(&ctx)(&err) - user, err := s.store.Users().GetByEmail(ctx, request.Email) - if err != nil { - return "", ErrUnauthorized.New(credentialsErrMsg) + user, _, err := s.store.Users().GetByEmailWithUnverified(ctx, request.Email) + if user == nil { + return "", ErrLoginCredentials.New(credentialsErrMsg) } err = bcrypt.CompareHashAndPassword(user.PasswordHash, []byte(request.Password)) if err != nil { - return "", ErrUnauthorized.New(credentialsErrMsg) + return "", ErrLoginCredentials.New(credentialsErrMsg) } if user.MFAEnabled { @@ -850,7 +851,7 @@ func (s *Service) Token(ctx context.Context, request AuthUser) (token string, er } } if !found { - return "", ErrUnauthorized.New(mfaRecoveryInvalidErrMsg) + return "", ErrMFARecoveryCode.New(mfaRecoveryInvalidErrMsg) } user.MFARecoveryCodes = append(user.MFARecoveryCodes[:codeIndex], user.MFARecoveryCodes[codeIndex+1:]...) @@ -862,19 +863,19 @@ func (s *Service) Token(ctx context.Context, request AuthUser) (token string, er } else if request.MFAPasscode != "" { valid, err := ValidateMFAPasscode(request.MFAPasscode, user.MFASecretKey, time.Now()) if err != nil { - return "", ErrUnauthorized.Wrap(err) + return "", ErrMFAPasscode.Wrap(err) } if !valid { - return "", ErrUnauthorized.New(mfaPasscodeInvalidErrMsg) + return "", ErrMFAPasscode.New(mfaPasscodeInvalidErrMsg) } } else { - return "", ErrMFALogin.Wrap(ErrMFAMissing.New(mfaRequiredErrMsg)) + return "", ErrMFAMissing.New(mfaRequiredErrMsg) } } claims := consoleauth.Claims{ ID: user.ID, - Expiration: time.Now().Add(TokenExpirationTime), + Expiration: time.Now().Add(s.config.TokenExpirationTime), } token, err = s.createToken(ctx, &claims) @@ -911,16 +912,20 @@ func (s *Service) GetUserID(ctx context.Context) (id uuid.UUID, err error) { return auth.User.ID, nil } -// GetUserByEmail returns User by email. -func (s *Service) GetUserByEmail(ctx context.Context, email string) (u *User, err error) { +// GetUserByEmailWithUnverified returns Users by email. +func (s *Service) GetUserByEmailWithUnverified(ctx context.Context, email string) (verified *User, unverified []User, err error) { defer mon.Task()(&ctx)(&err) - result, err := s.store.Users().GetByEmail(ctx, email) + verified, unverified, err = s.store.Users().GetByEmailWithUnverified(ctx, email) if err != nil { - return nil, Error.Wrap(err) + return verified, unverified, err } - return result, nil + if verified == nil && len(unverified) == 0 { + err = ErrEmailNotFound.New(emailNotFoundErrMsg) + } + + return verified, unverified, err } // UpdateAccount updates User. @@ -964,8 +969,11 @@ func (s *Service) ChangeEmail(ctx context.Context, newEmail string) (err error) return ErrValidation.Wrap(err) } - _, err = s.store.Users().GetByEmail(ctx, newEmail) - if err == nil { + verified, unverified, err := s.store.Users().GetByEmailWithUnverified(ctx, newEmail) + if err != nil { + return Error.Wrap(err) + } + if verified != nil || len(unverified) != 0 { return ErrEmailUsed.New(emailUsedErrMsg) } diff --git a/satellite/console/service_test.go b/satellite/console/service_test.go index 04d889b6c..cddf9d32a 100644 --- a/satellite/console/service_test.go +++ b/satellite/console/service_test.go @@ -225,9 +225,9 @@ func TestService(t *testing.T) { err = service.ChangeEmail(authCtx2, newEmail) require.NoError(t, err) - userWithUpdatedEmail, err := service.GetUserByEmail(authCtx2, newEmail) + user, _, err := service.GetUserByEmailWithUnverified(authCtx2, newEmail) require.NoError(t, err) - require.Equal(t, newEmail, userWithUpdatedEmail.Email) + require.Equal(t, newEmail, user.Email) err = service.ChangeEmail(authCtx2, newEmail) require.Error(t, err) @@ -437,7 +437,7 @@ func TestMFA(t *testing.T) { request.MFAPasscode = wrongCode token, err = service.Token(ctx, request) - require.True(t, console.ErrUnauthorized.Has(err)) + require.True(t, console.ErrMFAPasscode.Has(err)) require.Empty(t, token) // Expect token when providing valid passcode. @@ -469,7 +469,7 @@ func TestMFA(t *testing.T) { // Expect no token due to providing previously-used recovery code. token, err = service.Token(ctx, request) - require.True(t, console.ErrUnauthorized.Has(err)) + require.True(t, console.ErrMFARecoveryCode.Has(err)) require.Empty(t, token) updateAuth() @@ -582,7 +582,7 @@ func TestResetPassword(t *testing.T) { require.True(t, console.ErrRecoveryToken.Has(err)) // Expect error when providing good but expired token. - err = service.ResetPassword(ctx, tokenStr, newPass, token.CreatedAt.Add(console.TokenExpirationTime).Add(time.Second)) + err = service.ResetPassword(ctx, tokenStr, newPass, token.CreatedAt.Add(sat.Config.Console.TokenExpirationTime).Add(time.Second)) require.True(t, console.ErrTokenExpiration.Has(err)) // Expect error when providing good token with bad (too short) password. diff --git a/satellite/mailservice/service.go b/satellite/mailservice/service.go index 1a7f93c4d..652c51df5 100644 --- a/satellite/mailservice/service.go +++ b/satellite/mailservice/service.go @@ -55,7 +55,7 @@ type Message interface { // architecture: Service type Service struct { log *zap.Logger - sender Sender + Sender Sender html *htmltemplate.Template // TODO(yar): prepare plain text version @@ -67,7 +67,7 @@ type Service struct { // New creates new service. func New(log *zap.Logger, sender Sender, templatePath string) (*Service, error) { var err error - service := &Service{log: log, sender: sender} + service := &Service{log: log, Sender: sender} // TODO(yar): prepare plain text version // service.text, err = texttemplate.ParseGlob(filepath.Join(templatePath, "*.txt")) @@ -92,7 +92,7 @@ func (service *Service) Close() error { // Send is generalized method for sending custom email message. func (service *Service) Send(ctx context.Context, msg *post.Message) (err error) { defer mon.Task()(&ctx)(&err) - return service.sender.SendEmail(ctx, msg) + return service.Sender.SendEmail(ctx, msg) } // SendRenderedAsync renders content from htmltemplate and texttemplate templates then sends it asynchronously. @@ -140,7 +140,7 @@ func (service *Service) SendRendered(ctx context.Context, to []post.Address, msg } m := &post.Message{ - From: service.sender.FromAddress(), + From: service.Sender.FromAddress(), To: to, Subject: msg.Subject(), PlainText: textBuffer.String(), @@ -152,5 +152,5 @@ func (service *Service) SendRendered(ctx context.Context, to []post.Address, msg }, } - return service.sender.SendEmail(ctx, m) + return service.Sender.SendEmail(ctx, m) } diff --git a/scripts/testdata/satellite-config.yaml.lock b/scripts/testdata/satellite-config.yaml.lock index 0177a0a2b..98fb616cb 100755 --- a/scripts/testdata/satellite-config.yaml.lock +++ b/scripts/testdata/satellite-config.yaml.lock @@ -19,6 +19,9 @@ # reCAPTCHA site key # admin.console-config.recaptcha.site-key: "" +# expiration time for auth tokens, account recovery tokens, and activation tokens +# admin.console-config.token-expiration-time: 24h0m0s + # the default free-tier bandwidth usage limit # admin.console-config.usage-limits.bandwidth.free: 150.00 GB @@ -235,6 +238,9 @@ compensation.withheld-percents: 75,75,75,50,50,50,25,25,25,0,0,0,0,0,0 # url link to terms and conditions page # console.terms-and-conditions-url: https://storj.io/storage-sla/ +# expiration time for auth tokens, account recovery tokens, and activation tokens +# console.token-expiration-time: 24h0m0s + # the default free-tier bandwidth usage limit # console.usage-limits.bandwidth.free: 150.00 GB diff --git a/web/satellite/src/api/auth.ts b/web/satellite/src/api/auth.ts index 7f2a3dac2..4c207681c 100644 --- a/web/satellite/src/api/auth.ts +++ b/web/satellite/src/api/auth.ts @@ -2,7 +2,6 @@ // See LICENSE for copying information. import { ErrorBadRequest } from '@/api/errors/ErrorBadRequest'; -import { ErrorEmailUsed } from '@/api/errors/ErrorEmailUsed'; import { ErrorMFARequired } from '@/api/errors/ErrorMFARequired'; import { ErrorTooManyRequests } from '@/api/errors/ErrorTooManyRequests'; import { ErrorUnauthorized } from '@/api/errors/ErrorUnauthorized'; @@ -16,21 +15,26 @@ import { HttpClient } from '@/utils/httpClient'; export class AuthHttpApi implements UsersApi { private readonly http: HttpClient = new HttpClient(); private readonly ROOT_PATH: string = '/api/v0/auth'; + private readonly rateLimitErrMsg = 'You\'ve exceeded limit of attempts, try again in 5 minutes'; /** * Used to resend an registration confirmation email. * - * @param userId - id of newly created user + * @param email - email of newly created user * @throws Error */ - public async resendEmail(userId: string): Promise { - const path = `${this.ROOT_PATH}/resend-email/${userId}`; - const response = await this.http.post(path, userId); + public async resendEmail(email: string): Promise { + const path = `${this.ROOT_PATH}/resend-email/${email}`; + const response = await this.http.post(path, email); if (response.ok) { return; } - throw new Error('can not resend Email'); + if (response.status == 429) { + throw new ErrorTooManyRequests(this.rateLimitErrMsg); + } + + throw new Error('Failed to send email'); } /** @@ -61,13 +65,15 @@ export class AuthHttpApi implements UsersApi { return result; } + const result = await response.json(); + const errMsg = result.error || 'Failed to receive authentication token'; switch (response.status) { case 401: - throw new ErrorUnauthorized('Your email or password was incorrect, please try again'); + throw new ErrorUnauthorized(errMsg); case 429: - throw new ErrorTooManyRequests('You\'ve exceeded limit of attempts, try again in 5 minutes'); + throw new ErrorTooManyRequests(this.rateLimitErrMsg); default: - throw new Error('Can not receive authentication token'); + throw new Error(errMsg); } } @@ -104,7 +110,16 @@ export class AuthHttpApi implements UsersApi { return; } - throw new Error('There is no such email'); + const result = await response.json(); + const errMsg = result.error || 'Failed to send password reset link'; + switch (response.status) { + case 404: + throw new ErrorUnauthorized(errMsg); + case 429: + throw new ErrorTooManyRequests(this.rateLimitErrMsg); + default: + throw new Error(errMsg); + } } /** @@ -226,7 +241,7 @@ export class AuthHttpApi implements UsersApi { * @returns id of created user * @throws Error */ - public async register(user: {fullName: string; shortName: string; email: string; partner: string; partnerId: string; password: string; isProfessional: boolean; position: string; companyName: string; employeeCount: string; haveSalesContact: boolean }, secret: string, recaptchaResponse: string): Promise { + public async register(user: {fullName: string; shortName: string; email: string; partner: string; partnerId: string; password: string; isProfessional: boolean; position: string; companyName: string; employeeCount: string; haveSalesContact: boolean }, secret: string, recaptchaResponse: string): Promise { const path = `${this.ROOT_PATH}/register`; const body = { secret: secret, @@ -244,24 +259,20 @@ export class AuthHttpApi implements UsersApi { recaptchaResponse: recaptchaResponse, }; const response = await this.http.post(path, JSON.stringify(body)); - const result = await response.json(); if (!response.ok) { + const result = await response.json(); const errMsg = result.error || 'Cannot register user'; switch (response.status) { case 400: throw new ErrorBadRequest(errMsg); case 401: throw new ErrorUnauthorized(errMsg); - case 409: - throw new ErrorEmailUsed(errMsg); case 429: - throw new ErrorTooManyRequests('You\'ve exceeded limit of attempts, try again in 5 minutes'); + throw new ErrorTooManyRequests(this.rateLimitErrMsg); default: throw new Error(errMsg); } } - - return result; } /** diff --git a/web/satellite/src/api/errors/ErrorEmailUsed.ts b/web/satellite/src/api/errors/ErrorEmailUsed.ts deleted file mode 100644 index 79004bbdf..000000000 --- a/web/satellite/src/api/errors/ErrorEmailUsed.ts +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright (C) 2020 Storj Labs, Inc. -// See LICENSE for copying information. - -/** - * ErrorEmailUsed is a custom error type for performing 'email is already in use' operations. - */ -export class ErrorEmailUsed extends Error { - public constructor(message = 'email used') { - super(message); - } -} diff --git a/web/satellite/src/components/common/RegistrationSuccess.vue b/web/satellite/src/components/common/RegistrationSuccess.vue index 697d1cc3e..08dc6bf62 100644 --- a/web/satellite/src/components/common/RegistrationSuccess.vue +++ b/web/satellite/src/components/common/RegistrationSuccess.vue @@ -10,8 +10,13 @@

You're almost there!

+
+ If an account with the email address + + exists, a verification email has been sent. +

- Check your email to confirm your account and get started. + Check your inbox to activate your account and get started.

Didn't receive a verification email? @@ -25,7 +30,7 @@ width="450px" height="50px" :on-press="onResendEmailButtonClick" - :is-disabled="isResendEmailButtonDisabled" + :is-disabled="secondsToWait != 0" />

@@ -46,7 +51,7 @@ + + \ No newline at end of file diff --git a/web/satellite/src/views/LoginArea.vue b/web/satellite/src/views/LoginArea.vue index e072034b2..7b2012e9b 100644 --- a/web/satellite/src/views/LoginArea.vue +++ b/web/satellite/src/views/LoginArea.vue @@ -7,32 +7,41 @@