From 80202d43a91d0f1b783c5899f02a54e46bfd9917 Mon Sep 17 00:00:00 2001 From: Michael-u21546551 Date: Fri, 23 Aug 2024 18:05:10 +0200 Subject: [PATCH 1/2] chore: Add LoginKey function to cache_keys_gen.go and CanMakeLogin function to cache.go --- occupi-backend/pkg/cache/cache.go | 54 +++++++- occupi-backend/pkg/cache/cache_keys_gen.go | 4 + occupi-backend/pkg/constants/constants.go | 1 + occupi-backend/pkg/handlers/auth_handlers.go | 60 ++++++++ .../tests/cache_methods_unit_test.go | 130 ++++++++++++++++++ 5 files changed, 247 insertions(+), 2 deletions(-) diff --git a/occupi-backend/pkg/cache/cache.go b/occupi-backend/pkg/cache/cache.go index 32ea3f11..028d4820 100644 --- a/occupi-backend/pkg/cache/cache.go +++ b/occupi-backend/pkg/cache/cache.go @@ -11,8 +11,6 @@ import ( "go.mongodb.org/mongo-driver/bson" ) -// TODO: Add methods to prevent users from requesting more than 5 logins per day, 5 otps per day, etc. but waiting for go-redis to be integrated first - func GetUser(appsession *models.AppSession, email string) (models.User, error) { if appsession.Cache == nil { return models.User{}, errors.New("cache not found") @@ -317,3 +315,55 @@ func DeleteSession(appsession *models.AppSession, uuid string) { return } } + +func CanMakeLogin(appsession *models.AppSession, email string) (bool, error) { + if appsession.Cache == nil { + return false, errors.New("cache not found") + } + + var eviction time.Duration + if configs.GetGinRunMode() == "test" { + eviction = 2 * time.Second + } else { + eviction = 24 * time.Hour + } + + // check if the user can make a login request + res := appsession.Cache.Get(context.Background(), LoginKey(email)) + + if res.Err() != nil { + // add the user to the cache with a value of 1 and evict after one day + // set the image in the cache + res1 := appsession.Cache.Set(context.Background(), LoginKey(email), 1, eviction) + + if res1.Err() != nil { + logrus.Error("failed to set user in cache", res1.Err()) + return false, res1.Err() + } + + return true, nil + } + + // check if the user has made more than 5 login requests + loginCount, err := res.Int() + + if err != nil { + return false, err + } + + // Check if the value is less than 5 + if loginCount < 5 { + // Increment the value + loginCount += 1 + + // Set the new value with the same expiration time (24 hours) + err = appsession.Cache.Set(context.Background(), LoginKey(email), loginCount, eviction).Err() + if err != nil { + return false, err + } + + return true, nil + } + + return false, nil +} diff --git a/occupi-backend/pkg/cache/cache_keys_gen.go b/occupi-backend/pkg/cache/cache_keys_gen.go index af06d3de..ebc236db 100644 --- a/occupi-backend/pkg/cache/cache_keys_gen.go +++ b/occupi-backend/pkg/cache/cache_keys_gen.go @@ -19,3 +19,7 @@ func ImageKey(imageID string) string { func SessionKey(email string) string { return "Sessions:" + email } + +func LoginKey(email string) string { + return "Login:" + email +} diff --git a/occupi-backend/pkg/constants/constants.go b/occupi-backend/pkg/constants/constants.go index e30547f2..5068186d 100644 --- a/occupi-backend/pkg/constants/constants.go +++ b/occupi-backend/pkg/constants/constants.go @@ -9,6 +9,7 @@ const ( UnAuthorizedCode = "UNAUTHORIZED" RequestEntityTooLargeCode = "REQUEST_ENTITY_TOO_LARGE" ForbiddenCode = "FORBIDDEN" + TooManyRequestsCode = "TOO_MANY_REQUESTS" Admin = "admin" Basic = "basic" AdminDBAccessOption = "authSource=admin" diff --git a/occupi-backend/pkg/handlers/auth_handlers.go b/occupi-backend/pkg/handlers/auth_handlers.go index 17aaa136..affeb3da 100644 --- a/occupi-backend/pkg/handlers/auth_handlers.go +++ b/occupi-backend/pkg/handlers/auth_handlers.go @@ -31,6 +31,21 @@ func Login(ctx *gin.Context, appsession *models.AppSession, role string, cookies return } + if canLogin, err := cache.CanMakeLogin(appsession, requestUser.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if err != nil { + captureError(ctx, err) + logrus.WithError(err).Error("Error checking if user can login") + } + + ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( + http.StatusTooManyRequests, + "Too many login attempts", + constants.TooManyRequestsCode, + "Too many login attempts, please try again later", + nil)) + return + } + // sanitize user password and email requestUser.EmployeeID = utils.SanitizeInput(requestUser.EmployeeID) @@ -105,6 +120,21 @@ func BeginLoginAdmin(ctx *gin.Context, appsession *models.AppSession) { return } + if canLogin, err := cache.CanMakeLogin(appsession, requestEmail.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if err != nil { + captureError(ctx, err) + logrus.WithError(err).Error("Error checking if user can login") + } + + ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( + http.StatusTooManyRequests, + "Too many login attempts", + constants.TooManyRequestsCode, + "Too many login attempts, please try again later", + nil)) + return + } + // validate email exists if valid, err := ValidateEmailExists(ctx, appsession, requestEmail.Email); !valid { if err != nil { @@ -239,6 +269,21 @@ func BeginRegistrationAdmin(ctx *gin.Context, appsession *models.AppSession) { return } + if canLogin, err := cache.CanMakeLogin(appsession, requestEmail.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if err != nil { + captureError(ctx, err) + logrus.WithError(err).Error("Error checking if user can login") + } + + ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( + http.StatusTooManyRequests, + "Too many registration attempts", + constants.TooManyRequestsCode, + "Too many registration attempts, please try again later", + nil)) + return + } + // validate email exists if valid, err := ValidateEmailExists(ctx, appsession, requestEmail.Email); !valid { if err != nil { @@ -363,6 +408,21 @@ func Register(ctx *gin.Context, appsession *models.AppSession) { return } + if canLogin, err := cache.CanMakeLogin(appsession, requestUser.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if err != nil { + captureError(ctx, err) + logrus.WithError(err).Error("Error checking if user can login") + } + + ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( + http.StatusTooManyRequests, + "Too many registration attempts", + constants.TooManyRequestsCode, + "Too many registration attempts, please try again later", + nil)) + return + } + // sanitize user password and email requestUser.EmployeeID = utils.SanitizeInput(requestUser.EmployeeID) diff --git a/occupi-backend/tests/cache_methods_unit_test.go b/occupi-backend/tests/cache_methods_unit_test.go index 2f5e76e0..0b2b08dd 100644 --- a/occupi-backend/tests/cache_methods_unit_test.go +++ b/occupi-backend/tests/cache_methods_unit_test.go @@ -52,6 +52,15 @@ func TestImageKey(t *testing.T) { } } +func TestLoginKey(t *testing.T) { + email := "test@example.com" + expected := "Login:test@example.com" + result := cache.LoginKey(email) + if result != expected { + t.Errorf("LoginKey(%s) = %s; want %s", email, result, expected) + } +} + func TestGetUser(t *testing.T) { // Test case: cache is nil t.Run("cache is nil", func(t *testing.T) { @@ -1080,3 +1089,124 @@ func TestDeleteSession(t *testing.T) { } }) } + +func TestCanMakeLogin(t *testing.T) { + email := "test@example.com" + key := cache.LoginKey(email) + + t.Run("cache not found", func(t *testing.T) { + // Set up a mock Redis client + db, _ := redismock.NewClientMock() + + appsession := &models.AppSession{ + Cache: db, + } + appsession.Cache = nil + canLogin, err := cache.CanMakeLogin(appsession, email) + assert.False(t, canLogin) + assert.EqualError(t, err, "cache not found") + }) + + t.Run("new user - set value", func(t *testing.T) { + // Set up a mock Redis client + db, mock := redismock.NewClientMock() + + appsession := &models.AppSession{ + Cache: db, + } + // Simulate Get returning a nil value (key not found) + mock.ExpectGet(key).RedisNil() + // Simulate successful Set operation + mock.ExpectSet(key, 1, 2*time.Second).SetVal("OK") + + canLogin, err := cache.CanMakeLogin(appsession, email) + assert.True(t, canLogin) + assert.NoError(t, err) + + // Ensure all expectations were met + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + }) + + t.Run("existing user with login count less than 5", func(t *testing.T) { + // Set up a mock Redis client + db, mock := redismock.NewClientMock() + + appsession := &models.AppSession{ + Cache: db, + } + // Simulate Get returning a value of 3 + mock.ExpectGet(key).SetVal("3") + // Simulate successful Set operation to update the value to 4 + mock.ExpectSet(key, 4, 2*time.Second).SetVal("OK") + + canLogin, err := cache.CanMakeLogin(appsession, email) + assert.True(t, canLogin) + assert.NoError(t, err) + + // Ensure all expectations were met + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + }) + + t.Run("existing user with login count 5 or more", func(t *testing.T) { + // Set up a mock Redis client + db, mock := redismock.NewClientMock() + + appsession := &models.AppSession{ + Cache: db, + } + // Simulate Get returning a value of 5 + mock.ExpectGet(key).SetVal("5") + + canLogin, err := cache.CanMakeLogin(appsession, email) + assert.False(t, canLogin) + assert.NoError(t, err) + + // Ensure all expectations were met + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + }) + + t.Run("error on Get", func(t *testing.T) { + // Set up a mock Redis client + db, mock := redismock.NewClientMock() + + appsession := &models.AppSession{ + Cache: db, + } + // Simulate Get operation error + mock.ExpectGet(key).SetErr(errors.New("redis get error")) + // Simulate successful Set operation + mock.ExpectSet(key, 1, 2*time.Second).SetVal("OK") + + canLogin, err := cache.CanMakeLogin(appsession, email) + assert.True(t, canLogin) + assert.NoError(t, err) + + // Ensure all expectations were met + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + }) + + t.Run("error on Set", func(t *testing.T) { + // Set up a mock Redis client + db, mock := redismock.NewClientMock() + + appsession := &models.AppSession{ + Cache: db, + } + // Simulate Get returning a value of 3 + mock.ExpectGet(key).SetVal("3") + // Simulate Set operation error + mock.ExpectSet(key, 4, 2*time.Second).SetErr(errors.New("redis set error")) + + canLogin, err := cache.CanMakeLogin(appsession, email) + assert.False(t, canLogin) + assert.EqualError(t, err, "redis set error") + + // Ensure all expectations were met + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + }) +} From 568995b581dd0ea640d32551cbb839cea2217b24 Mon Sep 17 00:00:00 2001 From: Michael-u21546551 Date: Fri, 23 Aug 2024 18:19:43 +0200 Subject: [PATCH 2/2] chore: Refactor login handlers to use CanLogin helper function --- occupi-backend/pkg/handlers/auth_handlers.go | 40 +++----------------- occupi-backend/pkg/handlers/auth_helpers.go | 14 +++++++ 2 files changed, 20 insertions(+), 34 deletions(-) diff --git a/occupi-backend/pkg/handlers/auth_handlers.go b/occupi-backend/pkg/handlers/auth_handlers.go index affeb3da..391f4dbf 100644 --- a/occupi-backend/pkg/handlers/auth_handlers.go +++ b/occupi-backend/pkg/handlers/auth_handlers.go @@ -31,18 +31,11 @@ func Login(ctx *gin.Context, appsession *models.AppSession, role string, cookies return } - if canLogin, err := cache.CanMakeLogin(appsession, requestUser.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if canLogin, err := CanLogin(ctx, appsession, requestUser.Email); !canLogin { if err != nil { captureError(ctx, err) logrus.WithError(err).Error("Error checking if user can login") } - - ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( - http.StatusTooManyRequests, - "Too many login attempts", - constants.TooManyRequestsCode, - "Too many login attempts, please try again later", - nil)) return } @@ -120,18 +113,11 @@ func BeginLoginAdmin(ctx *gin.Context, appsession *models.AppSession) { return } - if canLogin, err := cache.CanMakeLogin(appsession, requestEmail.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if canLogin, err := CanLogin(ctx, appsession, requestEmail.Email); !canLogin { if err != nil { captureError(ctx, err) logrus.WithError(err).Error("Error checking if user can login") } - - ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( - http.StatusTooManyRequests, - "Too many login attempts", - constants.TooManyRequestsCode, - "Too many login attempts, please try again later", - nil)) return } @@ -185,7 +171,7 @@ func BeginLoginAdmin(ctx *gin.Context, appsession *models.AppSession) { } // Save the session data - cache will expire in x defined minutes according to the config - if err := cache.SetSession(appsession, session, uuid); err != nil && err.Error() != "cache not found" { + if err := cache.SetSession(appsession, session, uuid); err != nil { captureError(ctx, err) ctx.JSON(http.StatusInternalServerError, utils.InternalServerError()) fmt.Printf("error saving WebAuthn session data: %v", err) @@ -269,18 +255,11 @@ func BeginRegistrationAdmin(ctx *gin.Context, appsession *models.AppSession) { return } - if canLogin, err := cache.CanMakeLogin(appsession, requestEmail.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if canLogin, err := CanLogin(ctx, appsession, requestEmail.Email); !canLogin { if err != nil { captureError(ctx, err) logrus.WithError(err).Error("Error checking if user can login") } - - ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( - http.StatusTooManyRequests, - "Too many registration attempts", - constants.TooManyRequestsCode, - "Too many registration attempts, please try again later", - nil)) return } @@ -323,7 +302,7 @@ func BeginRegistrationAdmin(ctx *gin.Context, appsession *models.AppSession) { } // Save the session data - cache will expire in x defined minutes according to the config - if err := cache.SetSession(appsession, session, uuid); err != nil && err.Error() != "cache not found" { + if err := cache.SetSession(appsession, session, uuid); err != nil { captureError(ctx, err) logrus.WithError(err).Error("Error saving session data in cache") ctx.JSON(http.StatusInternalServerError, utils.InternalServerError()) @@ -408,18 +387,11 @@ func Register(ctx *gin.Context, appsession *models.AppSession) { return } - if canLogin, err := cache.CanMakeLogin(appsession, requestUser.Email); !canLogin && (err == nil || err.Error() != "cache not found") { + if canLogin, err := CanLogin(ctx, appsession, requestUser.Email); !canLogin { if err != nil { captureError(ctx, err) logrus.WithError(err).Error("Error checking if user can login") } - - ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( - http.StatusTooManyRequests, - "Too many registration attempts", - constants.TooManyRequestsCode, - "Too many registration attempts, please try again later", - nil)) return } diff --git a/occupi-backend/pkg/handlers/auth_helpers.go b/occupi-backend/pkg/handlers/auth_helpers.go index 620105e4..cc9bd385 100644 --- a/occupi-backend/pkg/handlers/auth_helpers.go +++ b/occupi-backend/pkg/handlers/auth_helpers.go @@ -5,6 +5,7 @@ import ( "time" "github.com/COS301-SE-2024/occupi/occupi-backend/pkg/authenticator" + "github.com/COS301-SE-2024/occupi/occupi-backend/pkg/cache" "github.com/COS301-SE-2024/occupi/occupi-backend/pkg/constants" "github.com/COS301-SE-2024/occupi/occupi-backend/pkg/database" "github.com/COS301-SE-2024/occupi/occupi-backend/pkg/mail" @@ -554,3 +555,16 @@ func AttemptToSignNewEmail(ctx *gin.Context, appsession *models.AppSession, emai } return nil } + +func CanLogin(ctx *gin.Context, appsession *models.AppSession, email string) (bool, error) { + if canLogin, err := cache.CanMakeLogin(appsession, email); !canLogin && (err == nil || err.Error() != "cache not found") { + ctx.JSON(http.StatusTooManyRequests, utils.ErrorResponse( + http.StatusTooManyRequests, + "Too many login attempts", + constants.TooManyRequestsCode, + "Too many login attempts, please try again later", + nil)) + return false, err + } + return true, nil +}