8000 chore: More UI friendly errors by Emyrk · Pull Request #1994 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

chore: More UI friendly errors #1994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Jun 3, 2022
Merged

chore: More UI friendly errors #1994

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
352b86a
chore: More UI friendly errors
Emyrk Jun 2, 2022
7f9b66a
chore: Update BE http errors to be ui friendly
Emyrk Jun 3, 2022
46b52d5
chore: More friendly errors as responses
Emyrk Jun 3, 2022
1592d08
Fix typo
Emyrk Jun 3, 2022
6b08a08
Add internal error to the cli
Emyrk Jun 3, 2022
1b3947f
Merge remote-tracking branch 'origin/main' into stevenmasley/ui_errors
Emyrk Jun 3, 2022
deb5cf1
Remove empty switch case
Emyrk Jun 3, 2022
5e34b5f
Update coderd/gitsshkey.go
Emyrk Jun 3, 2022
f355dff
Update coderd/gitsshkey.go
Emyrk Jun 3, 2022
6566e83
Update coderd/gitsshkey.go
Emyrk Jun 3, 2022
fed7b7f
Update coderd/gitsshkey.go
Emyrk Jun 3, 2022
5c4ec92
Update coderd/gitsshkey.go
Emyrk Jun 3, 2022
e373a06
Update coderd/httpapi/httpapi.go
Emyrk Jun 3, 2022
73ffa22
Rename "Internal" -> "Detail"
Emyrk Jun 3, 2022
5d59312
Rename "Errors" -> "Validations"
Emyrk Jun 3, 2022
a8e78bb
Fix proper captalization
Emyrk Jun 3, 2022
ee2f547
Fix typo
Emyrk Jun 3, 2022
4a5d03a
Fix accidental 'detail'
Emyrk Jun 3, 2022
8259e51
Merge remote-tracking branch 'origin/main' into stevenmasley/ui_errors
Emyrk Jun 3, 2022
819f2e1
PR comments addressed
Emyrk Jun 3, 2022
1116f94
Linting changes
Emyrk Jun 3, 2022
102a01f
Fix test error asserts
Emyrk Jun 3, 2022
af169f0
Fix test error asserts
Emyrk Jun 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
chore: More UI friendly errors 
Mainly capitlization + messages prefix error
  • Loading branch information
@Emyrk
Emyrk committed Jun 2, 2022
commit 352b86a9212dc7b20e26e7c072f7511fe1e0f06f
4 changes: 1 addition & 3 deletions coderd/authorize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ func (api *API) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.A
roles := httpmw.AuthorizationUserRoles(r)
err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject())
if err != nil {
httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
Message: err.Error(),
})
httpapi.Forbidden(rw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: what allowed us to make this change? Does it mean that we no longer send those random Message errors on forbidden routes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vapurrmaid for 10000 security reasons all forbidden messages should be identical. If they were different, then the different errors allow the end user to gain information.

There is a line of "security" vs "usability" that we will need to decide on these endpoints. As it's unhelpful by design.


// Log the errors for debugging
internalError := new(rbac.UnauthorizedError)
Expand Down
3 changes: 2 additions & 1 deletion coderd/csp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package coderd

import (
"encoding/json"
"fmt"
"net/http"

"github.com/coder/coder/coderd/httpapi"
Expand All @@ -23,7 +24,7 @@ func (api *API) logReportCSPViolations(rw http.ResponseWriter, r *http.Request)
if err != nil {
api.Logger.Warn(ctx, "csp violation", slog.Error(err))
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "failed to read body",
Message: fmt.Sprintf("Failed to read body, invalid json: %s", err.Error()),
})
return
}
Expand Down
10 changes: 5 additions & 5 deletions coderd/files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
case "application/x-tar":
default:
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("unsupported content type: %s", contentType),
Message: fmt.Sprintf("Unsupported content type: %s", contentType),
})
return
}
Expand All @@ -41,7 +41,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
data, err := io.ReadAll(r.Body)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("read file: %s", err),
Message: fmt.Sprintf("Read file from request: %s", err),
})
return
}
Expand All @@ -64,7 +64,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("insert file: %s", err),
Message: fmt.Sprintf("Saving file: %s", err),
})
return
}
Expand All @@ -78,7 +78,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
hash := chi.URLParam(r, "hash")
if hash == "" {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "hash must be provided",
Message: "File hash must be provided in url",
})
return
}
Expand All @@ -89,7 +89,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("get file: %s", err),
Message: fmt.Sprintf("Fetch file: %s", err),
})
return
}
Expand Down
8 changes: 4 additions & 4 deletions coderd/gitsshkey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
privateKey, publicKey, err := gitsshkey.Generate(api.SSHKeygenAlgorithm)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("regenerate key pair: %s", err),
Message: fmt.Sprintf("Regenerate key pair: %s", err),
})
return
}
Expand All @@ -35,15 +35,15 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("update git SSH key: %s", err),
Message: fmt.Sprintf("Update git SSH key: %s", err),
})
return
}

newKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("get git SSH key: %s", err),
Message: fmt.Sprintf("Get Git SSH key: %s", err),
})
return
}
Expand All @@ -67,7 +67,7 @@ func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("update git SSH key: %s", err),
Message: fmt.Sprintf("Update git SSH key: %s", err),
})
return
}
Expand Down
6 changes: 3 additions & 3 deletions coderd/members.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ func (api *API) updateOrganizationMemberRoles(ctx context.Context, args database

roleOrg, err := uuid.Parse(orgID)
if err != nil {
return database.OrganizationMember{}, xerrors.Errorf("role must have proper uuids for organization, %q does not", r)
return database.OrganizationMember{}, xerrors.Errorf("Role must have proper uuids for organization, %q does not", r)
}

if roleOrg != args.OrgID {
return database.OrganizationMember{}, xerrors.Errorf("must only pass roles for org %q", args.OrgID.String())
return database.OrganizationMember{}, xerrors.Errorf("Must only pass roles for org %q", args.OrgID.String())
}

if _, err := rbac.RoleByName(r); err != nil {
Expand All @@ -90,7 +90,7 @@ func (api *API) updateOrganizationMemberRoles(ctx context.Context, args database

updatedUser, err := api.Database.UpdateMemberRoles(ctx, args)
if err != nil {
return database.OrganizationMember{}, xerrors.Errorf("update site roles: %w", err)
return database.OrganizationMember{}, xerrors.Errorf("Update site roles: %w", err)
}
return updatedUser, nil
}
Expand Down
6 changes: 3 additions & 3 deletions coderd/organizations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
_, err := api.Database.GetOrganizationByName(r.Context(), req.Name)
if err == nil {
httpapi.Write(rw, http.StatusConflict, httpapi.Response{
Message: "organization already exists with that name",
Message: "Organization already exists with that name",
})
return
}
if !errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("get organization: %s", err.Error()),
Message: fmt.Sprintf("Get organization: %s", err.Error()),
})
return
}
Expand Down Expand Up @@ -83,7 +83,7 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: err.Error(),
Message: fmt.Sprintf("Insert organization member: %s", err.Error()),
})
return
}
Expand Down
0