Skip to content

Commit adeccfa

Browse files
committed
validations
1 parent 4ed7847 commit adeccfa

File tree

3 files changed

+134
-32
lines changed

3 files changed

+134
-32
lines changed

api/restHandler/UserAttributesRestHandler.go

Lines changed: 110 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@ package restHandler
1818

1919
import (
2020
"encoding/json"
21-
"errors"
22-
"github.com/devtron-labs/devtron/pkg/attributes/bean"
2321
"net/http"
2422

23+
"github.com/devtron-labs/devtron/pkg/attributes/bean"
24+
2525
"github.com/devtron-labs/devtron/api/restHandler/common"
2626
"github.com/devtron-labs/devtron/pkg/attributes"
2727
"github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin"
2828
"github.com/devtron-labs/devtron/pkg/auth/user"
29-
"github.com/gorilla/mux"
3029
"go.uber.org/zap"
3130
)
3231

@@ -56,18 +55,32 @@ func NewUserAttributesRestHandlerImpl(logger *zap.SugaredLogger, enforcer casbin
5655
}
5756

5857
func (handler *UserAttributesRestHandlerImpl) AddUserAttributes(w http.ResponseWriter, r *http.Request) {
59-
dto, success := handler.validateUserAttributesRequest(w, r, "PatchUserAttributes")
58+
dto, success := handler.validateUserAttributesRequest(w, r, "AddUserAttributes")
6059
if !success {
6160
return
6261
}
6362

64-
handler.logger.Infow("request payload, AddUserAttributes", "payload", dto)
63+
handler.logger.Infow("Adding user attributes",
64+
"operation", "add_user_attributes",
65+
"userId", dto.UserId,
66+
"key", dto.Key)
67+
6568
resp, err := handler.userAttributesService.AddUserAttributes(dto)
6669
if err != nil {
67-
handler.logger.Errorw("service err, AddUserAttributes", "err", err, "payload", dto)
68-
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
70+
handler.logger.Errorw("Failed to add user attributes",
71+
"operation", "add_user_attributes",
72+
"userId", dto.UserId,
73+
"key", dto.Key,
74+
"err", err)
75+
76+
// Use enhanced error response builder
77+
errBuilder := common.NewErrorResponseBuilder(w, r).
78+
WithOperation("user attributes creation").
79+
WithResource("user attribute", dto.Key)
80+
errBuilder.HandleError(err)
6981
return
7082
}
83+
7184
common.WriteJsonResp(w, nil, resp, http.StatusOK)
7285
}
7386

@@ -78,18 +91,32 @@ func (handler *UserAttributesRestHandlerImpl) AddUserAttributes(w http.ResponseW
7891
// @Success 200 {object} attributes.UserAttributesDto
7992
// @Router /orchestrator/attributes/user/update [POST]
8093
func (handler *UserAttributesRestHandlerImpl) UpdateUserAttributes(w http.ResponseWriter, r *http.Request) {
81-
dto, success := handler.validateUserAttributesRequest(w, r, "PatchUserAttributes")
94+
dto, success := handler.validateUserAttributesRequest(w, r, "UpdateUserAttributes")
8295
if !success {
8396
return
8497
}
8598

86-
handler.logger.Infow("request payload, UpdateUserAttributes", "payload", dto)
99+
handler.logger.Infow("Updating user attributes",
100+
"operation", "update_user_attributes",
101+
"userId", dto.UserId,
102+
"key", dto.Key)
103+
87104
resp, err := handler.userAttributesService.UpdateUserAttributes(dto)
88105
if err != nil {
89-
handler.logger.Errorw("service err, UpdateUserAttributes", "err", err, "payload", dto)
90-
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
106+
handler.logger.Errorw("Failed to update user attributes",
107+
"operation", "update_user_attributes",
108+
"userId", dto.UserId,
109+
"key", dto.Key,
110+
"err", err)
111+
112+
// Use enhanced error response builder
113+
errBuilder := common.NewErrorResponseBuilder(w, r).
114+
WithOperation("user attributes update").
115+
WithResource("user attribute", dto.Key)
116+
errBuilder.HandleError(err)
91117
return
92118
}
119+
93120
common.WriteJsonResp(w, nil, resp, http.StatusOK)
94121
}
95122

@@ -99,38 +126,68 @@ func (handler *UserAttributesRestHandlerImpl) PatchUserAttributes(w http.Respons
99126
return
100127
}
101128

102-
handler.logger.Infow("request payload, PatchUserAttributes", "payload", dto)
129+
handler.logger.Infow("Patching user attributes",
130+
"operation", "patch_user_attributes",
131+
"userId", dto.UserId,
132+
"key", dto.Key)
133+
103134
resp, err := handler.userAttributesService.PatchUserAttributes(dto)
104135
if err != nil {
105-
handler.logger.Errorw("service err, PatchUserAttributes", "err", err, "payload", dto)
106-
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
136+
handler.logger.Errorw("Failed to patch user attributes",
137+
"operation", "patch_user_attributes",
138+
"userId", dto.UserId,
139+
"key", dto.Key,
140+
"err", err)
141+
142+
// Use enhanced error response builder
143+
errBuilder := common.NewErrorResponseBuilder(w, r).
144+
WithOperation("user attributes patch").
145+
WithResource("user attribute", dto.Key)
146+
errBuilder.HandleError(err)
107147
return
108148
}
149+
109150
common.WriteJsonResp(w, nil, resp, http.StatusOK)
110151
}
111152

112153
func (handler *UserAttributesRestHandlerImpl) validateUserAttributesRequest(w http.ResponseWriter, r *http.Request, operation string) (*bean.UserAttributesDto, bool) {
154+
// 1. Authentication check using enhanced error handling
113155
userId, err := handler.userService.GetLoggedInUser(r)
114156
if userId == 0 || err != nil {
115157
common.HandleUnauthorized(w, r)
116158
return nil, false
117159
}
118160

161+
// 2. Request body parsing with enhanced error handling
119162
decoder := json.NewDecoder(r.Body)
120163
var dto bean.UserAttributesDto
121164
err = decoder.Decode(&dto)
122165
if err != nil {
123-
handler.logger.Errorw("request err, "+operation, "err", err, "payload", dto)
124-
common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
166+
handler.logger.Errorw("Request parsing error",
167+
"operation", operation,
168+
"err", err,
169+
"userId", userId)
170+
171+
// Use enhanced error response builder for request parsing errors
172+
errBuilder := common.NewErrorResponseBuilder(w, r).
173+
WithOperation(operation).
174+
WithResource("user attributes", "")
175+
errBuilder.HandleError(err)
125176
return nil, false
126177
}
127178

128179
dto.UserId = userId
129180

181+
// 3. Get user email with enhanced error handling
130182
emailId, err := handler.userService.GetActiveEmailById(userId)
131183
if err != nil {
132-
handler.logger.Errorw("request err, "+operation, "err", err, "payload", dto)
133-
common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden)
184+
handler.logger.Errorw("Failed to get user email",
185+
"operation", operation,
186+
"userId", userId,
187+
"err", err)
188+
189+
// Use enhanced error response for forbidden access
190+
common.WriteForbiddenError(w, "access user attributes", "user")
134191
return nil, false
135192
}
136193
dto.EmailId = emailId
@@ -145,36 +202,58 @@ func (handler *UserAttributesRestHandlerImpl) validateUserAttributesRequest(w ht
145202
// @Success 200 {object} attributes.UserAttributesDto
146203
// @Router /orchestrator/attributes/user/get [GET]
147204
func (handler *UserAttributesRestHandlerImpl) GetUserAttribute(w http.ResponseWriter, r *http.Request) {
205+
// 1. Authentication check using enhanced error handling
148206
userId, err := handler.userService.GetLoggedInUser(r)
149207
if userId == 0 || err != nil {
150208
common.HandleUnauthorized(w, r)
151209
return
152210
}
153211

154-
vars := mux.Vars(r)
155-
key := vars["key"]
156-
if key == "" {
157-
handler.logger.Errorw("request err, GetUserAttribute", "err", err, "key", key)
158-
common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
212+
// 2. Enhanced parameter extraction with automatic validation
213+
key, err := common.ExtractStringPathParamWithContext(w, r, "key")
214+
if err != nil {
215+
// Error already written by ExtractStringPathParamWithContext
159216
return
160217
}
161218

162-
dto := bean.UserAttributesDto{}
163-
219+
// 3. Get user email with enhanced error handling
164220
emailId, err := handler.userService.GetActiveEmailById(userId)
165221
if err != nil {
166-
handler.logger.Errorw("request err, UpdateUserAttributes", "err", err, "payload", dto)
167-
common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden)
222+
handler.logger.Errorw("Failed to get user email",
223+
"operation", "get_user_attribute",
224+
"userId", userId,
225+
"key", key,
226+
"err", err)
227+
228+
// Use enhanced error response for forbidden access
229+
common.WriteForbiddenError(w, "access user attributes", "user")
168230
return
169231
}
170-
dto.EmailId = emailId
171-
dto.Key = key
172232

233+
// 4. Prepare DTO
234+
dto := bean.UserAttributesDto{
235+
UserId: userId,
236+
EmailId: emailId,
237+
Key: key,
238+
}
239+
240+
// 5. Service call with enhanced error handling
173241
res, err := handler.userAttributesService.GetUserAttribute(&dto)
174242
if err != nil {
175-
handler.logger.Errorw("service err, GetAttributesById", "err", err)
176-
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
243+
handler.logger.Errorw("Failed to get user attribute",
244+
"operation", "get_user_attribute",
245+
"userId", userId,
246+
"key", key,
247+
"err", err)
248+
249+
// Use enhanced error response builder
250+
errBuilder := common.NewErrorResponseBuilder(w, r).
251+
WithOperation("user attribute retrieval").
252+
WithResource("user attribute", key)
253+
errBuilder.HandleError(err)
177254
return
178255
}
256+
257+
// 6. Success response
179258
common.WriteJsonResp(w, nil, res, http.StatusOK)
180259
}

api/restHandler/common/ApiResponseWriter.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ package common
1818

1919
import (
2020
"encoding/json"
21-
"github.com/devtron-labs/devtron/internal/util"
2221
"net/http"
22+
23+
"github.com/devtron-labs/devtron/internal/util"
2324
)
2425

2526
type ApiResponse struct {

api/restHandler/common/ParamParserUtils.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,28 @@ func ExtractIntPathParamWithContext(w http.ResponseWriter, r *http.Request, para
3838
return paramIntValue, nil
3939
}
4040

41+
// ExtractStringPathParamWithContext provides enhanced error messages for string path parameters
42+
func ExtractStringPathParamWithContext(w http.ResponseWriter, r *http.Request, paramName string) (string, error) {
43+
vars := mux.Vars(r)
44+
paramValue := vars[paramName]
45+
46+
if paramValue == "" {
47+
apiErr := util.NewMissingRequiredFieldError(paramName)
48+
WriteJsonResp(w, apiErr, nil, apiErr.HttpStatusCode)
49+
return "", apiErr
50+
}
51+
52+
// Trim whitespace and validate
53+
paramValue = strings.TrimSpace(paramValue)
54+
if paramValue == "" {
55+
apiErr := util.NewValidationErrorForField(paramName, "cannot be empty or contain only whitespace")
56+
WriteJsonResp(w, apiErr, nil, apiErr.HttpStatusCode)
57+
return "", apiErr
58+
}
59+
60+
return paramValue, nil
61+
}
62+
4163
func convertToInt(w http.ResponseWriter, paramValue string) (int, error) {
4264
paramIntValue, err := strconv.Atoi(paramValue)
4365
if err != nil {

0 commit comments

Comments
 (0)