Skip to content

Commit

Permalink
handle role http errors better and add http tests
Browse files Browse the repository at this point in the history
This handles role http endpoint errors better by better handling the
errors returned and setting the response status codes.

Additionally this adds testing to the http api so we may test these http
status codes to ensure they are working as we expect.

Signed-off-by: Mike Mason <[email protected]>
  • Loading branch information
mikemrm committed Mar 11, 2024
1 parent 7f9726c commit f6533cf
Show file tree
Hide file tree
Showing 10 changed files with 1,149 additions and 40 deletions.
2 changes: 1 addition & 1 deletion .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# [Choice] Go version (use -bullseye variants on local arm64/Apple Silicon): 1, 1.18, 1.17, 1-bullseye, 1.18-bullseye, 1.17-bullseye, 1-buster, 1.18-buster, 1.17-buster
FROM mcr.microsoft.com/vscode/devcontainers/go:1-1.20-bullseye
FROM mcr.microsoft.com/vscode/devcontainers/go:1-1.22-bullseye

# [Choice] Node.js version: none, lts/*, 16, 14, 12, 10
ARG NODE_VERSION="none"
Expand Down
13 changes: 7 additions & 6 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
module go.infratographer.com/permissions-api

go 1.20
go 1.22

require (
github.com/authzed/authzed-go v0.10.1
github.com/authzed/grpcutil v0.0.0-20240123194739-2ea1e3d2d98b
github.com/cockroachdb/cockroach-go/v2 v2.3.6
github.com/go-jose/go-jose/v4 v4.0.1
github.com/golang-jwt/jwt v3.2.2+incompatible
github.com/labstack/echo-jwt/v4 v4.2.0
github.com/labstack/echo/v4 v4.11.4
github.com/lib/pq v1.10.9
github.com/nats-io/nats.go v1.31.0
Expand All @@ -22,6 +25,7 @@ require (
go.opentelemetry.io/otel/trace v1.16.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.26.0
golang.org/x/exp v0.0.0-20230905200255-921286631fa9
google.golang.org/grpc v1.60.1
gopkg.in/yaml.v3 v3.0.1
)
Expand All @@ -40,7 +44,6 @@ require (
github.com/go-logr/logr v1.2.4 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/gofrs/flock v0.8.1 // indirect
github.com/golang-jwt/jwt v3.2.2+incompatible // indirect
github.com/golang-jwt/jwt/v5 v5.0.0 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 // indirect
Expand All @@ -60,7 +63,6 @@ require (
github.com/jzelinskie/stringz v0.0.2 // indirect
github.com/klauspost/compress v1.17.2 // indirect
github.com/labstack/echo-contrib v0.15.0 // indirect
github.com/labstack/echo-jwt/v4 v4.2.0 // indirect
github.com/labstack/gommon v0.4.2 // indirect
github.com/magiconair/properties v1.8.7 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
Expand Down Expand Up @@ -99,10 +101,9 @@ require (
go.opentelemetry.io/otel/metric v1.16.0 // indirect
go.opentelemetry.io/otel/sdk v1.16.0 // indirect
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
golang.org/x/crypto v0.17.0 // indirect
golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect
golang.org/x/crypto v0.19.0 // indirect
golang.org/x/net v0.19.0 // indirect
golang.org/x/sys v0.15.0 // indirect
golang.org/x/sys v0.17.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.5.0 // indirect
google.golang.org/genproto v0.0.0-20231106174013-bbf56f31fb17 // indirect
Expand Down
44 changes: 40 additions & 4 deletions go.sum

Large diffs are not rendered by default.

46 changes: 36 additions & 10 deletions internal/api/roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,13 @@ func (r *Router) roleGet(c echo.Context) error {
// Roles belong to resources by way of the actions they can perform; do the permissions
// check on the role resource.
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)

switch {
case err == nil:
case errors.Is(err, query.ErrRoleNotFound):
return echo.NewHTTPError(http.StatusNotFound, "resource not found").SetInternal(err)
default:
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}

// TODO: This shows an error for the role's resource, not the role. Determine if that
Expand All @@ -185,8 +190,13 @@ func (r *Router) roleGet(c echo.Context) error {
}

role, err := r.engine.GetRole(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)

switch {
case err == nil:
case errors.Is(err, query.ErrRoleNotFound):
return echo.NewHTTPError(http.StatusNotFound, "role not found").SetInternal(err)
default:
return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err)
}

resp := roleResponse{
Expand Down Expand Up @@ -278,16 +288,27 @@ func (r *Router) roleDelete(c echo.Context) error {
// Roles belong to resources by way of the actions they can perform; do the permissions
// check on the role resource.
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)

switch {
case err == nil:
case errors.Is(err, query.ErrRoleNotFound):
return echo.NewHTTPError(http.StatusNotFound, "resource not found").SetInternal(err)
default:
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}

if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleDelete, resource); err != nil {
return err
}

if err = r.engine.DeleteRole(ctx, roleResource); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error deleting resource").SetInternal(err)
err = r.engine.DeleteRole(ctx, roleResource)

switch {
case err == nil:
case errors.Is(err, query.ErrRoleNotFound):
return echo.NewHTTPError(http.StatusNotFound, "role not found").SetInternal(err)
default:
return echo.NewHTTPError(http.StatusInternalServerError, "error deleting role").SetInternal(err)
}

resp := deleteRoleResponse{
Expand Down Expand Up @@ -321,8 +342,13 @@ func (r *Router) roleGetResource(c echo.Context) error {
// There's a little irony here in that getting a role's resource here is required to actually
// do the permissions check.
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)

switch {
case err == nil:
case errors.Is(err, query.ErrRoleNotFound):
return echo.NewHTTPError(http.StatusNotFound, "role not found").SetInternal(err)
default:
return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err)
}

if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleGet, resource); err != nil {
Expand Down
Loading

0 comments on commit f6533cf

Please sign in to comment.