Use internal errors status codes during authentication flow #9540
Conversation
itaiad200
added
the
include-changelog
pkg/api/auth_middleware.go
Outdated
| "group": groupName, | ||
| "user": u.Username, | ||
| }).Error("Failed to add external user to group") | ||
| return nil, fmt.Errorf("add user to group: %w", err) |
There was a problem hiding this comment.
I know the existing flow is not perfect but this changes behavior (for both SAML / OIDC).
At this point in the code the user is already created. Re-running this after an error will not re-attach the user.
We allowed this since one of the groups may not always exist at first or be even deleted in the future.
If we with an error here, enhanceWithFriendlyName will not run and the call will result in an error.
IMO either leave it as-is or, change order of operations.
The correct behavior is to complete enhanceWithFriendlyName since this is a user model operation and they were created in this call.
Then, try attaching to groups and if we return an error due to that make sure we reflect that to the user.
| if err != nil { | ||
| a.logger.WithError(err).WithField("username", username).Error("failed to get user") | ||
| return nil, err | ||
| return nil, fmt.Errorf("%v: %w", err, ErrInternalServerError) |
There was a problem hiding this comment.
Commenting here but, below - also validate HTTP status code in resp?
3 participants
Closes #9539