From 29d1ecd67d04e3ebc72c0fd2b5e042122b4c1b50 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 04:25:32 -0400 Subject: [PATCH 01/26] remove basic response type --- backend/internal/v1/v1_common/response.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/backend/internal/v1/v1_common/response.go b/backend/internal/v1/v1_common/response.go index dddb381c..c5627680 100644 --- a/backend/internal/v1/v1_common/response.go +++ b/backend/internal/v1/v1_common/response.go @@ -1,8 +1 @@ package v1_common - -/* -Use this for any json response that just needs a simple message field. -*/ -type basicResponse struct { - Message string `json:"message"` -} From 10c8c19a945fae9eebb0833677cbeee24de73049 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 04:26:11 -0400 Subject: [PATCH 02/26] start implementing common error types --- backend/internal/v1/v1_common/errors.go | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/backend/internal/v1/v1_common/errors.go b/backend/internal/v1/v1_common/errors.go index c5627680..eec49294 100644 --- a/backend/internal/v1/v1_common/errors.go +++ b/backend/internal/v1/v1_common/errors.go @@ -1 +1,35 @@ package v1_common + +/* +Common error types for v1 +*/ +const ( + ErrorTypeValidation ErrorType = "VALIDATION_ERROR" + ErrorTypeAuth ErrorType = "AUTH_ERROR" + ErrorTypeNotFound ErrorType = "NOT_FOUND" + ErrorTypeBadRequest ErrorType = "BAD_REQUEST" + ErrorTypeInternal ErrorType = "INTERNAL_ERROR" + ErrorTypeUnavailable ErrorType = "SERVICE_UNAVAILABLE" + ErrorTypeForbidden ErrorType = "FORBIDDEN" +) + +/* +Use this for any json response that just needs a simple message field. +*/ +func (e *APIError) Error() string { + return e.Message +} + +/* +NewError creates a new APIError object. +*/ +func NewError(errorType ErrorType, code int, message string, details string) *APIError { + return &APIError{ + Type: errorType, + Message: message, + Details: details, + Code: code, + } +} + +// TODO: Implement the rest of the error types From efe5cfefbafdcc8d452d9ec7e5df491cdd9b3d87 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 04:26:26 -0400 Subject: [PATCH 03/26] add api error types + move basic response here + errortype --- backend/internal/v1/v1_common/types.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/backend/internal/v1/v1_common/types.go b/backend/internal/v1/v1_common/types.go index c5627680..e6d5b582 100644 --- a/backend/internal/v1/v1_common/types.go +++ b/backend/internal/v1/v1_common/types.go @@ -1 +1,21 @@ package v1_common + +/* +Use this for any json response that just needs a simple message field. +*/ +type basicResponse struct { + Message string `json:"message"` +} + +type ErrorType string + +/* +Use this for any API response that needs a message and a request_id. +*/ +type APIError struct { + Type ErrorType `json:"type"` + Message string `json:"message"` + Details string `json:"details,omitempty"` + RequestID string `json:"request_id,omitempty` + Code int `json:"-"` +} From 2877018567a3793f683d6000ed8a81681c28e118 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 16:25:28 -0400 Subject: [PATCH 04/26] create all error constructors --- backend/internal/v1/v1_common/errors.go | 40 ++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/backend/internal/v1/v1_common/errors.go b/backend/internal/v1/v1_common/errors.go index eec49294..f62e0712 100644 --- a/backend/internal/v1/v1_common/errors.go +++ b/backend/internal/v1/v1_common/errors.go @@ -1,5 +1,10 @@ package v1_common +import ( + "fmt" + "net/http" +) + /* Common error types for v1 */ @@ -32,4 +37,37 @@ func NewError(errorType ErrorType, code int, message string, details string) *AP } } -// TODO: Implement the rest of the error types +/* +NewValidationError creates a new APIError object with type VALIDATION_ERROR. +*/ +func NewValidationError(message string) *APIError { + return NewError(ErrorTypeValidation, http.StatusBadRequest, message, "") +} + +/* +NewNotFoundError creates a new APIError object with type NOT_FOUND. +*/ +func NewNotFoundError(resource string) *APIError { + return NewError(ErrorTypeNotFound, http.StatusNotFound, fmt.Sprintf("%s not found", resource), "") +} + +/* +NewAuthError creates a new APIError object with type AUTH_ERROR. +*/ +func NewAuthError(message string) *APIError { + return NewError(ErrorTypeAuth, http.StatusUnauthorized, message, "") +} + +/* +NewForbiddenError creates a new APIError object with type FORBIDDEN. +*/ +func NewForbiddenError(message string) *APIError { + return NewError(ErrorTypeForbidden, http.StatusForbidden, message, "") +} + +/* +NewInternalError creates a new APIError object with type INTERNAL_ERROR. +*/ +func NewInternalError(err error) *APIError { + return NewError(ErrorTypeInternal, http.StatusInternalServerError, "Internal server error", err.Error()) +} From b95120858577266cc4af25ea1fc881296cbd5316 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 18:30:19 -0400 Subject: [PATCH 05/26] add getuserid + getuserole helpers --- backend/internal/v1/v1_common/helpers.go | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/backend/internal/v1/v1_common/helpers.go b/backend/internal/v1/v1_common/helpers.go index 7fe018b6..5f1c94e5 100644 --- a/backend/internal/v1/v1_common/helpers.go +++ b/backend/internal/v1/v1_common/helpers.go @@ -1,8 +1,10 @@ package v1_common import ( + "KonferCA/SPUR/db" "net/http" + "github.com/google/uuid" "github.com/labstack/echo/v4" ) @@ -47,3 +49,31 @@ func Fail(c echo.Context, code int, publicErrMsg string, internalErr error) erro } return echo.NewHTTPError(code, publicErrMsg) } + +/* +Helper that gets the user id from the context. + +Returns an error if the user id is not found in the context. +*/ +func GetUserID(c echo.Context) (uuid.UUID, error) { + userID, ok := c.Get("used_id").(uuid.UUID) + if !ok { + return uuid.Nil, NewAuthError("user id not found in context") + } + + return userID, nil +} + +/* +Helper that gets the user role from the context. + +Returns an error if the user role is not found in the context. +*/ +func GetUserRole(c echo.Context) (db.UserRole, error) { + userRole, ok := c.Get("user_role").(db.UserRole) + if !ok { + return "", NewAuthError("user role not found in context") + } + + return userRole, nil +} From 6001b17508b98c8120bad4eabecf0cf6101f3071 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 19:45:46 -0400 Subject: [PATCH 06/26] move error types --- backend/internal/v1/v1_common/constants.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/backend/internal/v1/v1_common/constants.go b/backend/internal/v1/v1_common/constants.go index c5627680..9abbbe3b 100644 --- a/backend/internal/v1/v1_common/constants.go +++ b/backend/internal/v1/v1_common/constants.go @@ -1 +1,14 @@ package v1_common + +/* +Common error types for v1 +*/ +const ( + ErrorTypeValidation ErrorType = "VALIDATION_ERROR" + ErrorTypeAuth ErrorType = "AUTH_ERROR" + ErrorTypeNotFound ErrorType = "NOT_FOUND" + ErrorTypeBadRequest ErrorType = "BAD_REQUEST" + ErrorTypeInternal ErrorType = "INTERNAL_ERROR" + ErrorTypeUnavailable ErrorType = "SERVICE_UNAVAILABLE" + ErrorTypeForbidden ErrorType = "FORBIDDEN" +) From 63871acdc1eef3848a6af327217c6669f3114f31 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 19:46:06 -0400 Subject: [PATCH 07/26] move constants --- backend/internal/v1/v1_common/errors.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/backend/internal/v1/v1_common/errors.go b/backend/internal/v1/v1_common/errors.go index f62e0712..44fdd581 100644 --- a/backend/internal/v1/v1_common/errors.go +++ b/backend/internal/v1/v1_common/errors.go @@ -5,19 +5,6 @@ import ( "net/http" ) -/* -Common error types for v1 -*/ -const ( - ErrorTypeValidation ErrorType = "VALIDATION_ERROR" - ErrorTypeAuth ErrorType = "AUTH_ERROR" - ErrorTypeNotFound ErrorType = "NOT_FOUND" - ErrorTypeBadRequest ErrorType = "BAD_REQUEST" - ErrorTypeInternal ErrorType = "INTERNAL_ERROR" - ErrorTypeUnavailable ErrorType = "SERVICE_UNAVAILABLE" - ErrorTypeForbidden ErrorType = "FORBIDDEN" -) - /* Use this for any json response that just needs a simple message field. */ From 44a69b8a53edf33b532e2cc4de3a4b6acbb2fca2 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 19:46:42 -0400 Subject: [PATCH 08/26] add common helper functions for ctx --- backend/internal/v1/v1_common/helpers.go | 45 +++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/backend/internal/v1/v1_common/helpers.go b/backend/internal/v1/v1_common/helpers.go index 5f1c94e5..05cf5d1e 100644 --- a/backend/internal/v1/v1_common/helpers.go +++ b/backend/internal/v1/v1_common/helpers.go @@ -23,7 +23,7 @@ func Success(c echo.Context, code int, message string) error { if message == "" { message = http.StatusText(code) } - return c.JSON(code, basicResponse{Message: message}) + return c.JSON(code, BasicResponse{Message: message}) } /* @@ -50,6 +50,23 @@ func Fail(c echo.Context, code int, publicErrMsg string, internalErr error) erro return echo.NewHTTPError(code, publicErrMsg) } +/* +BindAndValidate binds the request body to a struct and validates it. + +Returns an error if either binding or validation fails. +*/ +func BindandValidate(c echo.Context, req interface{}) error { + if err := c.Bind(req); err != nil { + return NewValidationError("invalid request body") + } + + if err := c.Validate(req); err != nil { + return NewValidationError(err.Error()) + } + + return nil +} + /* Helper that gets the user id from the context. @@ -77,3 +94,29 @@ func GetUserRole(c echo.Context) (db.UserRole, error) { return userRole, nil } + +/* +Helper that checks if the user is an admin. + +Returns true if the user is an admin, false otherwise. +*/ +func IsAdmin(c echo.Context) bool { + role, err := GetUserRole(c) + if err != nil { + return false + } + + return role == db.UserRoleAdmin +} + +/* +Helper that checks if the route requires admin access. + +Returns an error if the user is not an admin. +*/ +func RequireAdmin(c echo.Context) error { + if !IsAdmin(c) { + return NewForbiddenError("Admin access required") + } + return nil +} From b126bc9adfce38c27a2f068dfbea95e43e90399b Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 19:47:45 -0400 Subject: [PATCH 09/26] move and update helpers test --- backend/internal/tests/helpers_test.go | 338 ++++++++++++++++++ backend/internal/v1/v1_common/helpers_test.go | 91 ----- 2 files changed, 338 insertions(+), 91 deletions(-) create mode 100644 backend/internal/tests/helpers_test.go delete mode 100644 backend/internal/v1/v1_common/helpers_test.go diff --git a/backend/internal/tests/helpers_test.go b/backend/internal/tests/helpers_test.go new file mode 100644 index 00000000..b6e297e9 --- /dev/null +++ b/backend/internal/tests/helpers_test.go @@ -0,0 +1,338 @@ +package tests + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "KonferCA/SPUR/db" + "KonferCA/SPUR/internal/middleware" + "KonferCA/SPUR/internal/v1/v1_common" + + "github.com/google/uuid" + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" +) + +// Test struct with validation tags +type testRequest struct { + Name string `json:"name" validate:"required"` + Email string `json:"email" validate:"required,email"` + Role db.UserRole `json:"role" validate:"valid_user_role"` + WalletAddress string `json:"wallet_address" validate:"wallet_address"` + LinkedInURL string `json:"linkedin_url" validate:"linkedin_url"` +} + +func TestSuccess(t *testing.T) { + tests := []struct { + name string + code int + message string + expectedCode int + expectedBody v1_common.BasicResponse + }{ + { + name: "with custom message", + code: http.StatusOK, + message: "test message", + expectedCode: http.StatusOK, + expectedBody: v1_common.BasicResponse{Message: "test message"}, + }, + { + name: "with empty message", + code: http.StatusCreated, + message: "", + expectedCode: http.StatusCreated, + expectedBody: v1_common.BasicResponse{Message: http.StatusText(http.StatusCreated)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + err := v1_common.Success(c, tt.code, tt.message) + assert.NoError(t, err) + + var gotBody v1_common.BasicResponse + err = json.NewDecoder(rec.Body).Decode(&gotBody) + assert.NoError(t, err) + + assert.Equal(t, tt.expectedCode, rec.Code) + assert.Equal(t, tt.expectedBody, gotBody) + }) + } +} + +func TestFail(t *testing.T) { + tests := []struct { + name string + code int + publicErrMsg string + internalErr error + expectedCode int + expectedMsg string + }{ + { + name: "with custom message and internal error", + code: http.StatusBadRequest, + publicErrMsg: "invalid input", + internalErr: errors.New("validation failed"), + expectedCode: http.StatusBadRequest, + expectedMsg: "invalid input", + }, + { + name: "with empty message", + code: http.StatusNotFound, + publicErrMsg: "", + internalErr: nil, + expectedCode: http.StatusNotFound, + expectedMsg: http.StatusText(http.StatusNotFound), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + err := v1_common.Fail(c, tt.code, tt.publicErrMsg, tt.internalErr) + httpErr, ok := err.(*echo.HTTPError) + assert.True(t, ok) + assert.Equal(t, tt.expectedCode, httpErr.Code) + assert.Equal(t, tt.expectedMsg, httpErr.Message) + + if tt.internalErr != nil { + contextErr, ok := c.Get("internal_error").(error) + assert.True(t, ok) + assert.Equal(t, tt.internalErr, contextErr) + } + }) + } +} + +func TestBindAndValidate(t *testing.T) { + e := echo.New() + validator := middleware.NewRequestValidator() + e.Validator = validator + + tests := []struct { + name string + body string + expectError bool + checkError func(*testing.T, error) + }{ + { + name: "valid request", + body: `{ + "name": "test", + "email": "test@example.com", + "role": "startup_owner", + "wallet_address": "0x1234567890123456789012345678901234567890123456789012345678901234", + "linkedin_url": "https://linkedin.com/in/test" + }`, + expectError: false, + }, + { + name: "invalid json", + body: `{"name": }`, + expectError: true, + checkError: func(t *testing.T, err error) { + assert.Error(t, err) + assert.Equal(t, "invalid request body", err.Error()) + }, + }, + { + name: "missing required fields", + body: `{}`, + expectError: true, + checkError: func(t *testing.T, err error) { + assert.Error(t, err) + validationErr := err.Error() + assert.Contains(t, validationErr, "Name") + assert.Contains(t, validationErr, "required") + }, + }, + { + name: "invalid email", + body: `{ + "name": "test", + "email": "invalid-email", + "role": "startup_owner" + }`, + expectError: true, + checkError: func(t *testing.T, err error) { + assert.Error(t, err) + assert.Contains(t, err.Error(), "Email") + assert.Contains(t, err.Error(), "valid email") + }, + }, + { + name: "invalid role", + body: `{ + "name": "test", + "email": "test@example.com", + "role": "invalid_role" + }`, + expectError: true, + checkError: func(t *testing.T, err error) { + assert.Error(t, err) + assert.Contains(t, err.Error(), "Role") + assert.Contains(t, err.Error(), "valid user role") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(tt.body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + var testReq testRequest + err := v1_common.BindandValidate(c, &testReq) + + if tt.expectError { + if tt.checkError != nil { + tt.checkError(t, err) + } else { + assert.Error(t, err) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestUserContext(t *testing.T) { + tests := []struct { + name string + setupContext func(echo.Context) + testFunc func(echo.Context) error + expectError bool + checkResult func(*testing.T, interface{}, error) + }{ + { + name: "get valid user id", + setupContext: func(c echo.Context) { + id := uuid.New() + c.Set("used_id", id) + }, + testFunc: func(c echo.Context) error { + _, err := v1_common.GetUserID(c) + return err + }, + expectError: false, + }, + { + name: "get invalid user id", + setupContext: func(c echo.Context) {}, + testFunc: func(c echo.Context) error { + _, err := v1_common.GetUserID(c) + return err + }, + expectError: true, + }, + { + name: "get valid user role", + setupContext: func(c echo.Context) { + c.Set("user_role", db.UserRoleAdmin) + }, + testFunc: func(c echo.Context) error { + _, err := v1_common.GetUserRole(c) + return err + }, + expectError: false, + }, + { + name: "get invalid user role", + setupContext: func(c echo.Context) {}, + testFunc: func(c echo.Context) error { + _, err := v1_common.GetUserRole(c) + return err + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + tt.setupContext(c) + err := tt.testFunc(c) + + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestAdminChecks(t *testing.T) { + tests := []struct { + name string + setupContext func(echo.Context) + expectAdmin bool + expectError bool + }{ + { + name: "is admin", + setupContext: func(c echo.Context) { + c.Set("user_role", db.UserRoleAdmin) + }, + expectAdmin: true, + expectError: false, + }, + { + name: "not admin", + setupContext: func(c echo.Context) { + c.Set("user_role", db.UserRoleStartupOwner) + }, + expectAdmin: false, + expectError: true, + }, + { + name: "no role", + setupContext: func(c echo.Context) {}, + expectAdmin: false, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + tt.setupContext(c) + + isAdmin := v1_common.IsAdmin(c) + assert.Equal(t, tt.expectAdmin, isAdmin) + + err := v1_common.RequireAdmin(c) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/backend/internal/v1/v1_common/helpers_test.go b/backend/internal/v1/v1_common/helpers_test.go deleted file mode 100644 index accbdf59..00000000 --- a/backend/internal/v1/v1_common/helpers_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package v1_common - -import ( - "encoding/json" - "errors" - "io" - "net/http" - "net/http/httptest" - "testing" - - "github.com/labstack/echo/v4" - "github.com/stretchr/testify/assert" -) - -func TestRequestResponders(t *testing.T) { - t.Run("Test success responder", func(t *testing.T) { - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) - err := Success(c, http.StatusOK, "test") - assert.NoError(t, err) - data, err := io.ReadAll(rec.Body) - assert.NoError(t, err) - var resBody basicResponse - err = json.Unmarshal(data, &resBody) - assert.NoError(t, err) - assert.Equal(t, rec.Code, http.StatusOK) - assert.Equal(t, resBody, basicResponse{Message: "test"}) - }) - - t.Run("Test success responder with empty message", func(t *testing.T) { - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) - err := Success(c, http.StatusOK, "") - assert.NoError(t, err) - data, err := io.ReadAll(rec.Body) - assert.NoError(t, err) - var resBody basicResponse - err = json.Unmarshal(data, &resBody) - assert.NoError(t, err) - assert.Equal(t, rec.Code, http.StatusOK) - assert.Equal(t, resBody, basicResponse{Message: http.StatusText(http.StatusOK)}) - }) - - t.Run("Test fail responder", func(t *testing.T) { - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) - testErr := errors.New("test-error") - err, ok := Fail(c, http.StatusBadRequest, "test", testErr).(*echo.HTTPError) - assert.True(t, ok) - assert.Equal(t, err.Code, http.StatusBadRequest) - assert.Equal(t, err.Message, "test") - contextErr, ok := c.Get("internal_error").(error) - assert.True(t, ok) - assert.Equal(t, contextErr, testErr) - }) - - t.Run("Test fail responder with empty message", func(t *testing.T) { - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) - testErr := errors.New("test-error") - err, ok := Fail(c, http.StatusBadRequest, "", testErr).(*echo.HTTPError) - assert.True(t, ok) - assert.Equal(t, err.Code, http.StatusBadRequest) - assert.Equal(t, err.Message, http.StatusText(http.StatusBadRequest)) - contextErr, ok := c.Get("internal_error").(error) - assert.True(t, ok) - assert.Equal(t, contextErr, testErr) - }) - - t.Run("Test fail responder with nil error", func(t *testing.T) { - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) - err, ok := Fail(c, http.StatusBadRequest, "", nil).(*echo.HTTPError) - assert.True(t, ok) - assert.Equal(t, err.Code, http.StatusBadRequest) - assert.Equal(t, err.Message, http.StatusText(http.StatusBadRequest)) - contextErr, ok := c.Get("internal_error").(error) - assert.False(t, ok) - assert.Equal(t, contextErr, nil) - }) -} From c2819bb150272d46d568d133436c1bf0161c23e3 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 19:48:47 -0400 Subject: [PATCH 10/26] update basic response to be global --- backend/internal/v1/v1_common/types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/internal/v1/v1_common/types.go b/backend/internal/v1/v1_common/types.go index e6d5b582..8b33f3f7 100644 --- a/backend/internal/v1/v1_common/types.go +++ b/backend/internal/v1/v1_common/types.go @@ -3,7 +3,7 @@ package v1_common /* Use this for any json response that just needs a simple message field. */ -type basicResponse struct { +type BasicResponse struct { Message string `json:"message"` } @@ -16,6 +16,6 @@ type APIError struct { Type ErrorType `json:"type"` Message string `json:"message"` Details string `json:"details,omitempty"` - RequestID string `json:"request_id,omitempty` + RequestID string `json:"request_id,omitempty"` Code int `json:"-"` } From 1245340d53fd98a51d7e42220225bd9fa1ad70f2 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 20:03:20 -0400 Subject: [PATCH 11/26] fix typo --- backend/internal/tests/helpers_test.go | 2 +- backend/internal/v1/v1_common/helpers.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/internal/tests/helpers_test.go b/backend/internal/tests/helpers_test.go index b6e297e9..475d83b5 100644 --- a/backend/internal/tests/helpers_test.go +++ b/backend/internal/tests/helpers_test.go @@ -226,7 +226,7 @@ func TestUserContext(t *testing.T) { name: "get valid user id", setupContext: func(c echo.Context) { id := uuid.New() - c.Set("used_id", id) + c.Set("user_id", id) }, testFunc: func(c echo.Context) error { _, err := v1_common.GetUserID(c) diff --git a/backend/internal/v1/v1_common/helpers.go b/backend/internal/v1/v1_common/helpers.go index 05cf5d1e..8356306c 100644 --- a/backend/internal/v1/v1_common/helpers.go +++ b/backend/internal/v1/v1_common/helpers.go @@ -73,7 +73,7 @@ Helper that gets the user id from the context. Returns an error if the user id is not found in the context. */ func GetUserID(c echo.Context) (uuid.UUID, error) { - userID, ok := c.Get("used_id").(uuid.UUID) + userID, ok := c.Get("user_id").(uuid.UUID) if !ok { return uuid.Nil, NewAuthError("user id not found in context") } From d2d005667a34b1d8da43c92c3baebe52d07c05a6 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 20:18:00 -0400 Subject: [PATCH 12/26] add success constant --- backend/internal/v1/v1_common/constants.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/backend/internal/v1/v1_common/constants.go b/backend/internal/v1/v1_common/constants.go index 9abbbe3b..3fc24dd7 100644 --- a/backend/internal/v1/v1_common/constants.go +++ b/backend/internal/v1/v1_common/constants.go @@ -1,5 +1,12 @@ package v1_common +/* +Common success types for v1 +*/ +const ( + SuccessTypeOK SuccessType = "SUCCESS" +) + /* Common error types for v1 */ From 365aa8bef94cc6179db8f1be65d40a8f54e1ea19 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 20:19:04 -0400 Subject: [PATCH 13/26] update success + fail to use correct response + error --- backend/internal/v1/v1_common/helpers.go | 52 +++++++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/backend/internal/v1/v1_common/helpers.go b/backend/internal/v1/v1_common/helpers.go index 8356306c..453f904f 100644 --- a/backend/internal/v1/v1_common/helpers.go +++ b/backend/internal/v1/v1_common/helpers.go @@ -23,7 +23,12 @@ func Success(c echo.Context, code int, message string) error { if message == "" { message = http.StatusText(code) } - return c.JSON(code, BasicResponse{Message: message}) + + return c.JSON(code, APISuccess{ + Type: SuccessTypeOK, + Message: message, + Code: code, + }) } /* @@ -47,7 +52,50 @@ func Fail(c echo.Context, code int, publicErrMsg string, internalErr error) erro if publicErrMsg == "" { publicErrMsg = http.StatusText(code) } - return echo.NewHTTPError(code, publicErrMsg) + + errorType := determineErrorType(code) + + return &APIError{ + Type: errorType, + Message: publicErrMsg, + Details: getErrorDetails(internalErr), + Code: code, + } +} + +/* +Helper function to determine the error type based on the http status code. + +Returns the error type. +*/ +func determineErrorType(code int) ErrorType { + switch code { + case http.StatusBadRequest: + return ErrorTypeBadRequest + case http.StatusUnauthorized: + return ErrorTypeAuth + case http.StatusForbidden: + return ErrorTypeForbidden + case http.StatusNotFound: + return ErrorTypeNotFound + case http.StatusServiceUnavailable: + return ErrorTypeUnavailable + default: + return ErrorTypeInternal + } +} + +/* +Helper function to get the error details. + +Returns the error details as a string. +*/ +func getErrorDetails(err error) string { + if err == nil { + return "" + } + + return err.Error() } /* From f3d5a4a6d59b328e4c7427d6e6a953276a01b0a4 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 20:19:09 -0400 Subject: [PATCH 14/26] add success type --- backend/internal/v1/v1_common/types.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/backend/internal/v1/v1_common/types.go b/backend/internal/v1/v1_common/types.go index 8b33f3f7..7eb5aa5e 100644 --- a/backend/internal/v1/v1_common/types.go +++ b/backend/internal/v1/v1_common/types.go @@ -7,8 +7,18 @@ type BasicResponse struct { Message string `json:"message"` } +type SuccessType string type ErrorType string +/* +Use this for any API response that needs a message. +*/ +type APISuccess struct { + Type SuccessType `json:"type"` + Message string `json:"message"` + Code int `json:"-"` +} + /* Use this for any API response that needs a message and a request_id. */ From 8b0eb42ce6ae09b73b793e152995ab40a776e5ca Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:39:40 -0400 Subject: [PATCH 15/26] update global error handler to use common types --- backend/internal/server/error_handler.go | 109 +++++++++++------------ 1 file changed, 51 insertions(+), 58 deletions(-) diff --git a/backend/internal/server/error_handler.go b/backend/internal/server/error_handler.go index b6dceb06..3662dde6 100644 --- a/backend/internal/server/error_handler.go +++ b/backend/internal/server/error_handler.go @@ -1,6 +1,8 @@ package server import ( + "KonferCA/SPUR/internal/v1/v1_common" + "fmt" "net/http" "github.com/go-playground/validator/v10" @@ -26,80 +28,71 @@ Example: }) */ func errorHandler(err error, c echo.Context) { - req := c.Request() - internalErr, ok := c.Get("internal_error").(error) - if !ok { - internalErr = nil - } - requestID := req.Header.Get(echo.HeaderXRequestID) + var ( + code = http.StatusInternalServerError + message = "internal server error" + errType = v1_common.ErrorTypeInternal + details = "" + ) - // default error response - status := http.StatusInternalServerError - message := "internal server error" - var validationErrors []string + internalErr, _ := c.Get("internal_error").(error) // handle different error types switch e := err.(type) { case *echo.HTTPError: - status = e.Code - // since the echo.HTTPError allows type any for the - // message field, we should make sure that it is an - // actual string that was passed before using it. - // problems can arise if an struct was passed but - // not meant to be exposed to the public or - // is just straight up unreadable. + code = e.Code + errType = v1_common.DetermineErrorType(code) if msg, ok := e.Message.(string); ok { message = msg } else { - message = http.StatusText(e.Code) + message = http.StatusText(code) } - case validator.ValidationErrors: - // handle validation errors specially - status = http.StatusBadRequest + code = http.StatusBadRequest + errType = v1_common.ErrorTypeValidation message = "validation failed" - validationErrors = make([]string, len(e)) - for i, err := range e { - validationErrors[i] = err.Error() - } - - case error: - // assign the returned error from handlers as the internal error. - // this is probably an internal error when trying to respond. - // this ensures that no internal error message gets leaks to the public. - if internalErr == nil { - internalErr = err + details = e.Error() + case *v1_common.APIError: + code = e.Code + errType = e.Type + message = e.Message + details = e.Details + default: + if internalErr != nil { + details = internalErr.Error() + } else { + details = err.Error() } } + requestID := c.Request().Header.Get(echo.HeaderXRequestID) + // log with more context - log. - Error(). - AnErr("internal_error", internalErr). - AnErr("request_error", err). - Str("request_id", requestID). - Str("method", req.Method). - Str("path", req.URL.Path). - Int("status", status). - Str("user_agent", req.UserAgent()). - Msg("request error") + logger := log.With(). + Str("request_error", fmt.Sprintf("code=%d, message=%s", code, message)). + Str("request_id", c.Response().Header().Get(echo.HeaderXRequestID)). + Str("method", c.Request().Method). + Str("path", c.Request().URL.Path). + Int("status", code). + Str("user_agent", c.Request().UserAgent()). + Logger() - // return json response - if !c.Response().Committed { - response := ErrorResponse{ - Status: status, - Message: message, - RequestID: requestID, - } - if len(validationErrors) > 0 { - response.Errors = validationErrors - } + if internalErr != nil { + logger = logger.With().Err(internalErr).Logger() + } - if err := c.JSON(status, response); err != nil { - log.Error(). - Err(err). - Str("request_id", requestID). - Msg("failed to send error response") - } + logger.Error().Msg("request error") + + apiError := &v1_common.APIError{ + Type: errType, + Message: message, + Details: details, + RequestID: requestID, + Code: code, + } + + if !c.Response().Committed { + c.Response().WriteHeader(code) + _ = c.JSON(code, apiError) } } From d2e0021a5023923fae0c1320b008b65b75b30640 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:39:54 -0400 Subject: [PATCH 16/26] update error handler tests --- backend/internal/server/error_handler_test.go | 146 ++++++++---------- 1 file changed, 64 insertions(+), 82 deletions(-) diff --git a/backend/internal/server/error_handler_test.go b/backend/internal/server/error_handler_test.go index 2d1819a1..79902adb 100644 --- a/backend/internal/server/error_handler_test.go +++ b/backend/internal/server/error_handler_test.go @@ -1,6 +1,7 @@ package server import ( + "KonferCA/SPUR/internal/v1/v1_common" "bytes" "encoding/json" "errors" @@ -25,7 +26,8 @@ func TestGlobalErrorHandler(t *testing.T) { name string handler echo.HandlerFunc expectedStatus int - expectedBody string + expectedType v1_common.ErrorType + expectedMsg string }{ { name: "http error", @@ -33,7 +35,8 @@ func TestGlobalErrorHandler(t *testing.T) { return echo.NewHTTPError(http.StatusBadRequest, "bad request") }, expectedStatus: http.StatusBadRequest, - expectedBody: `{"status":400,"message":"bad request"}`, + expectedType: v1_common.ErrorTypeBadRequest, + expectedMsg: "bad request", }, { name: "generic error", @@ -41,7 +44,8 @@ func TestGlobalErrorHandler(t *testing.T) { return echo.NewHTTPError(http.StatusInternalServerError, "something went wrong") }, expectedStatus: http.StatusInternalServerError, - expectedBody: `{"status":500,"message":"something went wrong"}`, + expectedType: v1_common.ErrorTypeInternal, + expectedMsg: "something went wrong", }, { name: "validation error", @@ -52,22 +56,14 @@ func TestGlobalErrorHandler(t *testing.T) { } v := validator.New() - err := v.Struct(TestStruct{ + return v.Struct(TestStruct{ Email: "invalid-email", Age: -1, }) - - return err }, expectedStatus: http.StatusBadRequest, - expectedBody: `{ - "status": 400, - "message": "validation failed", - "errors": [ - "Key: 'TestStruct.Email' Error:Field validation for 'Email' failed on the 'email' tag", - "Key: 'TestStruct.Age' Error:Field validation for 'Age' failed on the 'gt' tag" - ] - }`, + expectedType: v1_common.ErrorTypeValidation, + expectedMsg: "validation failed", }, { name: "with request id", @@ -76,11 +72,11 @@ func TestGlobalErrorHandler(t *testing.T) { return echo.NewHTTPError(http.StatusBadRequest, "bad request") }, expectedStatus: http.StatusBadRequest, - expectedBody: `{"status":400,"message":"bad request","request_id":"test-123"}`, + expectedType: v1_common.ErrorTypeBadRequest, + expectedMsg: "bad request", }, } - // run tests for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -92,34 +88,31 @@ func TestGlobalErrorHandler(t *testing.T) { e.HTTPErrorHandler(err, c) } + var response v1_common.APIError + err = json.Unmarshal(rec.Body.Bytes(), &response) + assert.NoError(t, err) assert.Equal(t, tt.expectedStatus, rec.Code) - assert.JSONEq(t, tt.expectedBody, rec.Body.String()) + assert.Equal(t, tt.expectedType, response.Type) + assert.Equal(t, tt.expectedMsg, response.Message) }) } } type logEntry struct { - Level string `json:"level"` - InternalError string `json:"internal_error"` - RequestError string `json:"request_error"` - RequestID string `json:"request_id"` - Method string `json:"method"` - Path string `json:"path"` - Status int `json:"status"` - UserAgent string `json:"user_agent"` - Message string `json:"message"` -} - -type customError struct { - msg string -} - -func (e *customError) Error() string { - return e.msg + Level string `json:"level"` + InternalError error `json:"internal_error,omitempty"` + RequestError string `json:"request_error"` + RequestID string `json:"request_id"` + Method string `json:"method"` + Path string `json:"path"` + Status int `json:"status"` + UserAgent string `json:"user_agent"` + Message string `json:"message"` + Code int `json:"code"` + Type v1_common.ErrorType `json:"type"` } func TestErrorHandler(t *testing.T) { - // setting a validation error to not mock anything type TestStruct struct { Email string `validate:"required,email"` Age int `validate:"required,gt=0"` @@ -136,30 +129,35 @@ func TestErrorHandler(t *testing.T) { err error internalErr error expectedStatus int + expectedType v1_common.ErrorType expectedMsg string }{ { name: "internal server error", err: errors.New("something went wrong"), expectedStatus: http.StatusInternalServerError, + expectedType: v1_common.ErrorTypeInternal, expectedMsg: "internal server error", }, { name: "http error with string message", err: echo.NewHTTPError(http.StatusBadRequest, "invalid input"), expectedStatus: http.StatusBadRequest, + expectedType: v1_common.ErrorTypeBadRequest, expectedMsg: "invalid input", }, { name: "http error with non-string message", err: echo.NewHTTPError(http.StatusBadRequest, struct{ foo string }{foo: "bar"}), expectedStatus: http.StatusBadRequest, + expectedType: v1_common.ErrorTypeBadRequest, expectedMsg: http.StatusText(http.StatusBadRequest), }, { name: "validation error", err: validationErr, expectedStatus: http.StatusBadRequest, + expectedType: v1_common.ErrorTypeValidation, expectedMsg: "validation failed", }, { @@ -167,17 +165,23 @@ func TestErrorHandler(t *testing.T) { err: errors.New("handler error"), internalErr: errors.New("internal error"), expectedStatus: http.StatusInternalServerError, + expectedType: v1_common.ErrorTypeInternal, expectedMsg: "internal server error", }, + { + name: "api error", + err: &v1_common.APIError{Type: v1_common.ErrorTypeAuth, Message: "unauthorized", Code: http.StatusUnauthorized}, + expectedStatus: http.StatusUnauthorized, + expectedType: v1_common.ErrorTypeAuth, + expectedMsg: "unauthorized", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create a buffer to capture log output var buf bytes.Buffer log.Logger = zerolog.New(&buf) - // Setup Echo context e := echo.New() req := httptest.NewRequest(http.MethodPost, "/test", nil) req.Header.Set(echo.HeaderXRequestID, "test-request-id") @@ -185,63 +189,41 @@ func TestErrorHandler(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec) - // Set internal error if provided if tt.internalErr != nil { c.Set("internal_error", tt.internalErr) } - // Call error handler + // call error handler and verify it returns no error errorHandler(tt.err, c) - // Parse log output - var entry logEntry - err := json.Unmarshal(buf.Bytes(), &entry) - assert.NoError(t, err) + // verify status was set correctly + assert.Equal(t, tt.expectedStatus, rec.Result().StatusCode, "Status code mismatch") - // Verify log fields - assert.Equal(t, "error", entry.Level) - assert.Equal(t, "test-request-id", entry.RequestID) - assert.Equal(t, http.MethodPost, entry.Method) - assert.Equal(t, "/test", entry.Path) - assert.Equal(t, tt.expectedStatus, entry.Status) - assert.Equal(t, "test-agent", entry.UserAgent) - assert.Equal(t, "request error", entry.Message) - - // Verify error logging - assert.Contains(t, entry.RequestError, tt.err.Error()) - if tt.internalErr != nil { - assert.Contains(t, entry.InternalError, tt.internalErr.Error()) - } else if tt.err != nil && !isHTTPError(tt.err) && !isValidationError(tt.err) { - // If no internal error was set and the error is not HTTP or validation, - // the handler error should be set as internal error - assert.Contains(t, entry.InternalError, tt.err.Error()) + // check response + var response v1_common.APIError + err := json.Unmarshal(rec.Body.Bytes(), &response) + assert.NoError(t, err) + assert.Equal(t, tt.expectedStatus, response.Code, "Response code mismatch") + assert.Equal(t, tt.expectedType, response.Type, "Error type mismatch") + assert.Equal(t, tt.expectedMsg, response.Message, "Error message mismatch") + assert.Equal(t, "test-request-id", response.RequestID, "Request ID mismatch") + + // verify log entry + var logEntry struct { + Level string `json:"level"` + ReqErr string `json:"request_error"` + Status int `json:"status"` + Message string `json:"message"` } - - // Verify response - var response ErrorResponse - err = json.Unmarshal(rec.Body.Bytes(), &response) + err = json.Unmarshal(buf.Bytes(), &logEntry) assert.NoError(t, err) - assert.Equal(t, tt.expectedStatus, response.Status) - assert.Equal(t, tt.expectedMsg, response.Message) - assert.Equal(t, "test-request-id", response.RequestID) + assert.Equal(t, "error", logEntry.Level) + assert.Equal(t, tt.expectedStatus, logEntry.Status) - // Verify validation errors if applicable + // check validation errors if _, ok := tt.err.(validator.ValidationErrors); ok { - assert.NotEmpty(t, response.Errors) - } else { - assert.Empty(t, response.Errors) + assert.NotEmpty(t, response.Details) } }) } } - -// Helper functions to check error types -func isHTTPError(err error) bool { - _, ok := err.(*echo.HTTPError) - return ok -} - -func isValidationError(err error) bool { - _, ok := err.(validator.ValidationErrors) - return ok -} From 4f7b168382733cee82e5966e6213272178dfa89d Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:40:07 -0400 Subject: [PATCH 17/26] update auth test to use common types --- backend/internal/tests/auth_test.go | 79 ++++++++++++++++++++++------- 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/backend/internal/tests/auth_test.go b/backend/internal/tests/auth_test.go index 79568698..b12f2305 100644 --- a/backend/internal/tests/auth_test.go +++ b/backend/internal/tests/auth_test.go @@ -6,11 +6,11 @@ import ( "KonferCA/SPUR/internal/middleware" "KonferCA/SPUR/internal/server" "KonferCA/SPUR/internal/v1/v1_auth" + "KonferCA/SPUR/internal/v1/v1_common" "bytes" "context" "encoding/json" "fmt" - "io" "net/http" "net/http/httptest" "os" @@ -25,7 +25,7 @@ import ( func TestAuthEndpoints(t *testing.T) { // Setup test environment setupEnv() - + // Additional required env vars os.Setenv("JWT_SECRET", "test-secret-key") @@ -45,7 +45,7 @@ func TestAuthEndpoints(t *testing.T) { ID: userID.String(), Email: "test@example.com", Password: string(hashedPassword), - Role: db.UserRoleStartupOwner, + Role: db.UserRoleStartupOwner, EmailVerified: true, CreatedAt: time.Now().Unix(), UpdatedAt: time.Now().Unix(), @@ -54,9 +54,9 @@ func TestAuthEndpoints(t *testing.T) { // Insert test user _, err = s.GetDB().Exec(context.Background(), ` - INSERT INTO users (id, email, password, role, email_verified, created_at, updated_at, token_salt) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - `, testUser.ID, testUser.Email, testUser.Password, testUser.Role, testUser.EmailVerified, + INSERT INTO users (id, email, password, role, email_verified, created_at, updated_at, token_salt) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + `, testUser.ID, testUser.Email, testUser.Password, testUser.Role, testUser.EmailVerified, testUser.CreatedAt, testUser.UpdatedAt, testUser.TokenSalt) if err != nil { t.Fatalf("Failed to create test user: %v", err) @@ -68,6 +68,10 @@ func TestAuthEndpoints(t *testing.T) { payload v1_auth.LoginRequest expectedStatus int checkResponse bool + expectedError *struct { + errorType v1_common.ErrorType + errorMessage string + } }{ { name: "Valid Login", @@ -85,7 +89,13 @@ func TestAuthEndpoints(t *testing.T) { Password: "wrongpassword", }, expectedStatus: http.StatusUnauthorized, - checkResponse: false, + expectedError: &struct { + errorType v1_common.ErrorType + errorMessage string + }{ + errorType: v1_common.ErrorTypeAuth, + errorMessage: "Invalid email or password", + }, }, { name: "Invalid Email", @@ -94,7 +104,13 @@ func TestAuthEndpoints(t *testing.T) { Password: "testpassword123", }, expectedStatus: http.StatusUnauthorized, - checkResponse: false, + expectedError: &struct { + errorType v1_common.ErrorType + errorMessage string + }{ + errorType: v1_common.ErrorTypeAuth, + errorMessage: "Invalid email or password", + }, }, { name: "Invalid Email Format", @@ -103,19 +119,23 @@ func TestAuthEndpoints(t *testing.T) { Password: "testpassword123", }, expectedStatus: http.StatusBadRequest, - checkResponse: false, + expectedError: &struct { + errorType v1_common.ErrorType + errorMessage string + }{ + errorType: v1_common.ErrorTypeBadRequest, + errorMessage: "Validation failed", + }, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - // Create request jsonBody, _ := json.Marshal(tc.payload) req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", bytes.NewBuffer(jsonBody)) req.Header.Set("Content-Type", "application/json") rec := httptest.NewRecorder() - // Send request through the server s.GetEcho().ServeHTTP(rec, req) assert.Equal(t, tc.expectedStatus, rec.Code) @@ -142,6 +162,12 @@ func TestAuthEndpoints(t *testing.T) { } } assert.True(t, foundRefreshToken) + } else if tc.expectedError != nil { + var errResp v1_common.APIError + err := json.NewDecoder(rec.Body).Decode(&errResp) + assert.NoError(t, err) + assert.Equal(t, tc.expectedError.errorType, errResp.Type) + assert.Contains(t, errResp.Message, tc.expectedError.errorMessage) } }) } @@ -161,6 +187,10 @@ func TestAuthEndpoints(t *testing.T) { setupAuth func(req *http.Request) expectedStatus int expectedBody map[string]interface{} + expectedError *struct { + errorType v1_common.ErrorType + errorMessage string + } }{ { name: "Valid Token", @@ -178,7 +208,13 @@ func TestAuthEndpoints(t *testing.T) { // No auth header }, expectedStatus: http.StatusUnauthorized, - expectedBody: nil, + expectedError: &struct { + errorType v1_common.ErrorType + errorMessage string + }{ + errorType: v1_common.ErrorTypeAuth, + errorMessage: "missing authorization header", + }, }, { name: "Invalid Token", @@ -186,7 +222,13 @@ func TestAuthEndpoints(t *testing.T) { req.Header.Set("Authorization", "Bearer invalid-token") }, expectedStatus: http.StatusUnauthorized, - expectedBody: nil, + expectedError: &struct { + errorType v1_common.ErrorType + errorMessage string + }{ + errorType: v1_common.ErrorTypeAuth, + errorMessage: "invalid token", + }, }, } @@ -196,18 +238,21 @@ func TestAuthEndpoints(t *testing.T) { tc.setupAuth(req) rec := httptest.NewRecorder() - // Send request through the server s.GetEcho().ServeHTTP(rec, req) assert.Equal(t, tc.expectedStatus, rec.Code) if tc.expectedBody != nil { - resBodyBytes, err := io.ReadAll(rec.Body) - assert.NoError(t, err) var response map[string]interface{} - err = json.Unmarshal(resBodyBytes, &response) + err := json.NewDecoder(rec.Body).Decode(&response) assert.NoError(t, err) assert.Equal(t, tc.expectedBody, response) + } else if tc.expectedError != nil { + var errResp v1_common.APIError + err := json.NewDecoder(rec.Body).Decode(&errResp) + assert.NoError(t, err) + assert.Equal(t, tc.expectedError.errorType, errResp.Type) + assert.Equal(t, tc.expectedError.errorMessage, errResp.Message) } }) } From e57acaa4398d490b1af13747f02ff924e6645e96 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:40:25 -0400 Subject: [PATCH 18/26] update helpers test --- backend/internal/tests/helpers_test.go | 75 ++++++++++++++++++-------- 1 file changed, 53 insertions(+), 22 deletions(-) diff --git a/backend/internal/tests/helpers_test.go b/backend/internal/tests/helpers_test.go index 475d83b5..a73aad56 100644 --- a/backend/internal/tests/helpers_test.go +++ b/backend/internal/tests/helpers_test.go @@ -72,28 +72,54 @@ func TestSuccess(t *testing.T) { func TestFail(t *testing.T) { tests := []struct { - name string - code int - publicErrMsg string - internalErr error - expectedCode int - expectedMsg string + name string + code int + publicErrMsg string + internalErr error + expectedCode int + expectedType v1_common.ErrorType + expectedMsg string + expectedDetails string }{ { - name: "with custom message and internal error", - code: http.StatusBadRequest, - publicErrMsg: "invalid input", - internalErr: errors.New("validation failed"), - expectedCode: http.StatusBadRequest, - expectedMsg: "invalid input", + name: "with custom message and internal error", + code: http.StatusBadRequest, + publicErrMsg: "invalid input", + internalErr: errors.New("validation failed"), + expectedCode: http.StatusBadRequest, + expectedType: v1_common.ErrorTypeBadRequest, + expectedMsg: "invalid input", + expectedDetails: "validation failed", }, { - name: "with empty message", - code: http.StatusNotFound, - publicErrMsg: "", - internalErr: nil, - expectedCode: http.StatusNotFound, - expectedMsg: http.StatusText(http.StatusNotFound), + name: "with empty message", + code: http.StatusNotFound, + publicErrMsg: "", + internalErr: nil, + expectedCode: http.StatusNotFound, + expectedType: v1_common.ErrorTypeNotFound, + expectedMsg: http.StatusText(http.StatusNotFound), + expectedDetails: "", + }, + { + name: "unauthorized error", + code: http.StatusUnauthorized, + publicErrMsg: "not authorized", + internalErr: errors.New("token expired"), + expectedCode: http.StatusUnauthorized, + expectedType: v1_common.ErrorTypeAuth, + expectedMsg: "not authorized", + expectedDetails: "token expired", + }, + { + name: "forbidden error", + code: http.StatusForbidden, + publicErrMsg: "access denied", + internalErr: nil, + expectedCode: http.StatusForbidden, + expectedType: v1_common.ErrorTypeForbidden, + expectedMsg: "access denied", + expectedDetails: "", }, } @@ -105,10 +131,15 @@ func TestFail(t *testing.T) { c := e.NewContext(req, rec) err := v1_common.Fail(c, tt.code, tt.publicErrMsg, tt.internalErr) - httpErr, ok := err.(*echo.HTTPError) - assert.True(t, ok) - assert.Equal(t, tt.expectedCode, httpErr.Code) - assert.Equal(t, tt.expectedMsg, httpErr.Message) + + apiErr, ok := err.(*v1_common.APIError) + assert.True(t, ok, "Expected APIError type") + if ok { + assert.Equal(t, tt.expectedCode, apiErr.Code) + assert.Equal(t, tt.expectedType, apiErr.Type) + assert.Equal(t, tt.expectedMsg, apiErr.Message) + assert.Equal(t, tt.expectedDetails, apiErr.Details) + } if tt.internalErr != nil { contextErr, ok := c.Get("internal_error").(error) From e29b6ff0c74924726034fd3d290aa173dfc1c289 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:40:41 -0400 Subject: [PATCH 19/26] update server test to use common types --- backend/internal/tests/server_test.go | 30 ++++++++++++--------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/backend/internal/tests/server_test.go b/backend/internal/tests/server_test.go index 84281648..1c6061c5 100644 --- a/backend/internal/tests/server_test.go +++ b/backend/internal/tests/server_test.go @@ -5,6 +5,7 @@ import ( "KonferCA/SPUR/internal/jwt" "KonferCA/SPUR/internal/server" "KonferCA/SPUR/internal/v1/v1_auth" + "KonferCA/SPUR/internal/v1/v1_common" "context" "encoding/json" "fmt" @@ -131,15 +132,13 @@ func TestServer(t *testing.T) { rec := httptest.NewRecorder() s.Echo.ServeHTTP(rec, req) - assert.Equal(t, http.StatusBadRequest, rec.Code) - - resBodyBytes, err := io.ReadAll(rec.Body) - assert.Nil(t, err) - var resBody map[string]any - err = json.Unmarshal(resBodyBytes, &resBody) - assert.Nil(t, err) - assert.Equal(t, "Missing required query parameter: 'token'", resBody["message"]) + var apiErr v1_common.APIError + err := json.NewDecoder(rec.Body).Decode(&apiErr) + assert.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Equal(t, v1_common.ErrorTypeBadRequest, apiErr.Type) + assert.Equal(t, "Missing required query parameter: 'token'", apiErr.Message) }) t.Run("/auth/verify-email - 400 Bad Request - deny expired email token", func(t *testing.T) { @@ -149,7 +148,6 @@ func TestServer(t *testing.T) { assert.Nil(t, err) defer removeTestUser(ctx, email, s) - // generate a test email token that is expired exp := time.Now().Add(-(time.Minute * 30)).UTC() tokenID, err := createTestEmailToken(ctx, userID, exp, s) assert.Nil(t, err) @@ -160,15 +158,13 @@ func TestServer(t *testing.T) { rec := httptest.NewRecorder() s.Echo.ServeHTTP(rec, req) - assert.Equal(t, http.StatusBadRequest, rec.Code) - - resBodyBytes, err := io.ReadAll(rec.Body) - assert.Nil(t, err) - var resBody map[string]any - err = json.Unmarshal(resBodyBytes, &resBody) - assert.Nil(t, err) - assert.Equal(t, "Failed to verify email. Invalid or expired token.", resBody["message"]) + var apiErr v1_common.APIError + err = json.NewDecoder(rec.Body).Decode(&apiErr) + assert.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Equal(t, v1_common.ErrorTypeBadRequest, apiErr.Type) + assert.Equal(t, "Failed to verify email. Invalid or expired token.", apiErr.Message) }) }) } From 16e83d366c5eb9e7e3563de0847aa96482a65949 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:41:12 -0400 Subject: [PATCH 20/26] make helper functions public --- backend/internal/v1/v1_common/helpers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/internal/v1/v1_common/helpers.go b/backend/internal/v1/v1_common/helpers.go index 453f904f..81f2dd46 100644 --- a/backend/internal/v1/v1_common/helpers.go +++ b/backend/internal/v1/v1_common/helpers.go @@ -53,12 +53,12 @@ func Fail(c echo.Context, code int, publicErrMsg string, internalErr error) erro publicErrMsg = http.StatusText(code) } - errorType := determineErrorType(code) + errorType := DetermineErrorType(code) return &APIError{ Type: errorType, Message: publicErrMsg, - Details: getErrorDetails(internalErr), + Details: GetErrorDetails(internalErr), Code: code, } } @@ -68,7 +68,7 @@ Helper function to determine the error type based on the http status code. Returns the error type. */ -func determineErrorType(code int) ErrorType { +func DetermineErrorType(code int) ErrorType { switch code { case http.StatusBadRequest: return ErrorTypeBadRequest @@ -90,7 +90,7 @@ Helper function to get the error details. Returns the error details as a string. */ -func getErrorDetails(err error) string { +func GetErrorDetails(err error) string { if err == nil { return "" } From f0a1b25d13d58ea72566bf18b47303912d05e06d Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Sat, 21 Dec 2024 23:41:59 -0400 Subject: [PATCH 21/26] update code prop to allow serialization --- backend/internal/v1/v1_common/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/internal/v1/v1_common/types.go b/backend/internal/v1/v1_common/types.go index 7eb5aa5e..834a2e8e 100644 --- a/backend/internal/v1/v1_common/types.go +++ b/backend/internal/v1/v1_common/types.go @@ -27,5 +27,5 @@ type APIError struct { Message string `json:"message"` Details string `json:"details,omitempty"` RequestID string `json:"request_id,omitempty"` - Code int `json:"-"` + Code int `json:"code"` } From bf45aabc723f6dec412a634512d47e83e8402517 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Mon, 23 Dec 2024 01:39:05 -0400 Subject: [PATCH 22/26] update test to save & restore original logger --- backend/internal/server/error_handler_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/internal/server/error_handler_test.go b/backend/internal/server/error_handler_test.go index 79902adb..f32fb57f 100644 --- a/backend/internal/server/error_handler_test.go +++ b/backend/internal/server/error_handler_test.go @@ -113,6 +113,12 @@ type logEntry struct { } func TestErrorHandler(t *testing.T) { + originalLogger := log.Logger + + defer func() { + log.Logger = originalLogger + }() + type TestStruct struct { Email string `validate:"required,email"` Age int `validate:"required,gt=0"` From 4f6d0bdb83b4bfa106f9cdb46f7cb5c1a0f968b1 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Mon, 23 Dec 2024 03:34:00 -0400 Subject: [PATCH 23/26] store original logger + handle multiple validation errors + hide raw error messages from client + log error correctly --- backend/internal/server/error_handler.go | 37 +++++++++++++++++------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/backend/internal/server/error_handler.go b/backend/internal/server/error_handler.go index 3662dde6..8226d13a 100644 --- a/backend/internal/server/error_handler.go +++ b/backend/internal/server/error_handler.go @@ -4,6 +4,7 @@ import ( "KonferCA/SPUR/internal/v1/v1_common" "fmt" "net/http" + "strings" "github.com/go-playground/validator/v10" "github.com/labstack/echo/v4" @@ -32,7 +33,7 @@ func errorHandler(err error, c echo.Context) { code = http.StatusInternalServerError message = "internal server error" errType = v1_common.ErrorTypeInternal - details = "" + details string ) internalErr, _ := c.Get("internal_error").(error) @@ -51,7 +52,17 @@ func errorHandler(err error, c echo.Context) { code = http.StatusBadRequest errType = v1_common.ErrorTypeValidation message = "validation failed" - details = e.Error() + var errMsgs []string + for _, err := range e { + errMsgs = append(errMsgs, fmt.Sprintf( + "field '%s' failed on '%s' validation. got: %v, condition: %s", + err.Field(), + err.Tag(), + err.Value(), + err.Param(), + )) + } + details = strings.Join(errMsgs, "; ") case *v1_common.APIError: code = e.Code errType = e.Type @@ -59,28 +70,31 @@ func errorHandler(err error, c echo.Context) { details = e.Details default: if internalErr != nil { - details = internalErr.Error() + log.Error().Err(internalErr).Msg("internal error occurred") } else { - details = err.Error() + log.Error().Err(err).Msg("unexpected error occurred") } + details = "an unexpected error occurred. please try again later" } requestID := c.Request().Header.Get(echo.HeaderXRequestID) // log with more context - logger := log.With(). + logContext := log.With(). Str("request_error", fmt.Sprintf("code=%d, message=%s", code, message)). - Str("request_id", c.Response().Header().Get(echo.HeaderXRequestID)). + Str("request_id", requestID). Str("method", c.Request().Method). Str("path", c.Request().URL.Path). Int("status", code). - Str("user_agent", c.Request().UserAgent()). - Logger() + Str("user_agent", c.Request().UserAgent()) if internalErr != nil { - logger = logger.With().Err(internalErr).Logger() + logContext = logContext.Str("error", internalErr.Error()) + } else if err != nil && code == http.StatusInternalServerError { + logContext = logContext.Str("error", err.Error()) } + logger := logContext.Logger() logger.Error().Msg("request error") apiError := &v1_common.APIError{ @@ -92,7 +106,8 @@ func errorHandler(err error, c echo.Context) { } if !c.Response().Committed { - c.Response().WriteHeader(code) - _ = c.JSON(code, apiError) + if err := c.JSON(code, apiError); err != nil { + log.Error().Err(err).Msg("failed to send error response") + } } } From 5df238f331ef376df876e6083ab52a8b2e946c05 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Mon, 23 Dec 2024 03:34:05 -0400 Subject: [PATCH 24/26] update test --- backend/internal/server/error_handler_test.go | 101 +++++++++++------- 1 file changed, 62 insertions(+), 39 deletions(-) diff --git a/backend/internal/server/error_handler_test.go b/backend/internal/server/error_handler_test.go index f32fb57f..797e4dd3 100644 --- a/backend/internal/server/error_handler_test.go +++ b/backend/internal/server/error_handler_test.go @@ -7,6 +7,7 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "testing" "github.com/go-playground/validator/v10" @@ -131,33 +132,41 @@ func TestErrorHandler(t *testing.T) { }) tests := []struct { - name string - err error - internalErr error - expectedStatus int - expectedType v1_common.ErrorType - expectedMsg string + name string + err error + internalErr error + expectedStatus int + expectedType v1_common.ErrorType + expectedMsg string + expectedDetails string + checkLoggedError bool }{ { - name: "internal server error", - err: errors.New("something went wrong"), - expectedStatus: http.StatusInternalServerError, - expectedType: v1_common.ErrorTypeInternal, - expectedMsg: "internal server error", + name: "internal server error", + err: errors.New("something went wrong"), + expectedStatus: http.StatusInternalServerError, + expectedType: v1_common.ErrorTypeInternal, + expectedMsg: "internal server error", + expectedDetails: "an unexpected error occurred. please try again later", + checkLoggedError: true, }, { - name: "http error with string message", - err: echo.NewHTTPError(http.StatusBadRequest, "invalid input"), - expectedStatus: http.StatusBadRequest, - expectedType: v1_common.ErrorTypeBadRequest, - expectedMsg: "invalid input", + name: "http error with string message", + err: echo.NewHTTPError(http.StatusBadRequest, "invalid input"), + expectedStatus: http.StatusBadRequest, + expectedType: v1_common.ErrorTypeBadRequest, + expectedMsg: "invalid input", + expectedDetails: "", + checkLoggedError: false, }, { - name: "http error with non-string message", - err: echo.NewHTTPError(http.StatusBadRequest, struct{ foo string }{foo: "bar"}), - expectedStatus: http.StatusBadRequest, - expectedType: v1_common.ErrorTypeBadRequest, - expectedMsg: http.StatusText(http.StatusBadRequest), + name: "http error with non-string message", + err: echo.NewHTTPError(http.StatusBadRequest, struct{ foo string }{foo: "bar"}), + expectedStatus: http.StatusBadRequest, + expectedType: v1_common.ErrorTypeBadRequest, + expectedMsg: http.StatusText(http.StatusBadRequest), + expectedDetails: "", + checkLoggedError: false, }, { name: "validation error", @@ -165,21 +174,28 @@ func TestErrorHandler(t *testing.T) { expectedStatus: http.StatusBadRequest, expectedType: v1_common.ErrorTypeValidation, expectedMsg: "validation failed", + expectedDetails: "field 'Email' failed on 'email' validation. got: invalid-email, condition: ; " + + "field 'Age' failed on 'gt' validation. got: -1, condition: 0", + checkLoggedError: false, }, { - name: "with internal error set", - err: errors.New("handler error"), - internalErr: errors.New("internal error"), - expectedStatus: http.StatusInternalServerError, - expectedType: v1_common.ErrorTypeInternal, - expectedMsg: "internal server error", + name: "with internal error set", + err: errors.New("handler error"), + internalErr: errors.New("internal error"), + expectedStatus: http.StatusInternalServerError, + expectedType: v1_common.ErrorTypeInternal, + expectedMsg: "internal server error", + expectedDetails: "an unexpected error occurred. please try again later", + checkLoggedError: true, }, { - name: "api error", - err: &v1_common.APIError{Type: v1_common.ErrorTypeAuth, Message: "unauthorized", Code: http.StatusUnauthorized}, - expectedStatus: http.StatusUnauthorized, - expectedType: v1_common.ErrorTypeAuth, - expectedMsg: "unauthorized", + name: "api error", + err: &v1_common.APIError{Type: v1_common.ErrorTypeAuth, Message: "unauthorized", Code: http.StatusUnauthorized}, + expectedStatus: http.StatusUnauthorized, + expectedType: v1_common.ErrorTypeAuth, + expectedMsg: "unauthorized", + expectedDetails: "", + checkLoggedError: false, }, } @@ -213,22 +229,29 @@ func TestErrorHandler(t *testing.T) { assert.Equal(t, tt.expectedType, response.Type, "Error type mismatch") assert.Equal(t, tt.expectedMsg, response.Message, "Error message mismatch") assert.Equal(t, "test-request-id", response.RequestID, "Request ID mismatch") + assert.Equal(t, tt.expectedDetails, response.Details, "Details mismatch") // verify log entry + logLines := strings.Split(strings.TrimSpace(buf.String()), "\n") var logEntry struct { Level string `json:"level"` ReqErr string `json:"request_error"` Status int `json:"status"` Message string `json:"message"` + Error string `json:"error,omitempty"` } - err = json.Unmarshal(buf.Bytes(), &logEntry) - assert.NoError(t, err) - assert.Equal(t, "error", logEntry.Level) - assert.Equal(t, tt.expectedStatus, logEntry.Status) - // check validation errors - if _, ok := tt.err.(validator.ValidationErrors); ok { - assert.NotEmpty(t, response.Details) + if len(logLines) > 0 { + lastLine := logLines[len(logLines)-1] + err = json.Unmarshal([]byte(lastLine), &logEntry) + assert.NoError(t, err, "Failed to parse log entry") + + assert.Equal(t, "error", logEntry.Level, "Log level mismatch") + assert.Equal(t, tt.expectedStatus, logEntry.Status, "Log status mismatch") + + if tt.checkLoggedError { + assert.NotEmpty(t, logEntry.Error, "Expected error to be logged") + } } }) } From e4f8f58bfb64170ffcf045f7995f2dcb40eb3843 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Mon, 23 Dec 2024 16:08:54 -0400 Subject: [PATCH 25/26] format validation errors to json --- backend/internal/server/error_handler.go | 26 ++++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/backend/internal/server/error_handler.go b/backend/internal/server/error_handler.go index 8226d13a..398d1b17 100644 --- a/backend/internal/server/error_handler.go +++ b/backend/internal/server/error_handler.go @@ -2,6 +2,7 @@ package server import ( "KonferCA/SPUR/internal/v1/v1_common" + "encoding/json" "fmt" "net/http" "strings" @@ -52,17 +53,24 @@ func errorHandler(err error, c echo.Context) { code = http.StatusBadRequest errType = v1_common.ErrorTypeValidation message = "validation failed" - var errMsgs []string + fieldErrors := make(map[string]map[string]interface{}) + for _, err := range e { - errMsgs = append(errMsgs, fmt.Sprintf( - "field '%s' failed on '%s' validation. got: %v, condition: %s", - err.Field(), - err.Tag(), - err.Value(), - err.Param(), - )) + fieldName := strings.ToLower(err.Field()) + fieldErrors[fieldName] = map[string]interface{}{ + "tag": err.Tag(), + "value": err.Value(), + "condition": err.Param(), + } + } + + detailsBytes, err := json.Marshal(fieldErrors) + if err != nil { + details = "error formatting validation details" + log.Error().Err(err).Msg("failed to format validation details") + } else { + details = string(detailsBytes) } - details = strings.Join(errMsgs, "; ") case *v1_common.APIError: code = e.Code errType = e.Type From 44ff8c99bcca4bccf0aae287a94843e2d66f7f71 Mon Sep 17 00:00:00 2001 From: Aidan Traboulay Date: Mon, 23 Dec 2024 16:09:05 -0400 Subject: [PATCH 26/26] update test for details in validation error type --- backend/internal/server/error_handler_test.go | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/backend/internal/server/error_handler_test.go b/backend/internal/server/error_handler_test.go index 797e4dd3..65b449a5 100644 --- a/backend/internal/server/error_handler_test.go +++ b/backend/internal/server/error_handler_test.go @@ -138,7 +138,7 @@ func TestErrorHandler(t *testing.T) { expectedStatus int expectedType v1_common.ErrorType expectedMsg string - expectedDetails string + expectedDetails interface{} checkLoggedError bool }{ { @@ -174,8 +174,18 @@ func TestErrorHandler(t *testing.T) { expectedStatus: http.StatusBadRequest, expectedType: v1_common.ErrorTypeValidation, expectedMsg: "validation failed", - expectedDetails: "field 'Email' failed on 'email' validation. got: invalid-email, condition: ; " + - "field 'Age' failed on 'gt' validation. got: -1, condition: 0", + expectedDetails: map[string]map[string]interface{}{ + "email": { + "tag": "email", + "value": "invalid-email", + "condition": "", + }, + "age": { + "tag": "gt", + "value": float64(-1), + "condition": "0", + }, + }, checkLoggedError: false, }, { @@ -229,7 +239,14 @@ func TestErrorHandler(t *testing.T) { assert.Equal(t, tt.expectedType, response.Type, "Error type mismatch") assert.Equal(t, tt.expectedMsg, response.Message, "Error message mismatch") assert.Equal(t, "test-request-id", response.RequestID, "Request ID mismatch") - assert.Equal(t, tt.expectedDetails, response.Details, "Details mismatch") + if tt.expectedType == v1_common.ErrorTypeValidation { + var detailsMap map[string]map[string]interface{} + err = json.Unmarshal([]byte(response.Details), &detailsMap) + assert.NoError(t, err, "Failed to parse validation details as JSON") + assert.Equal(t, tt.expectedDetails, detailsMap, "Validation details mismatch") + } else { + assert.Equal(t, tt.expectedDetails, response.Details, "Details mismatch") + } // verify log entry logLines := strings.Split(strings.TrimSpace(buf.String()), "\n")