From ca651fc3d1ad8374ac3ba61029a80335b15d1c0a Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Fri, 15 Mar 2024 09:30:48 -0500 Subject: [PATCH] Increase role http error and test quality (#222) * check action against policy on access check This updates access check requests to ensure the requested action exists in the policy for the provided resource beforce executing the request to spicedb and failing for this error. Knowing this we can return a relevant error (ErrInvalidAction) which we can now handle and return a 400 Bad Request status code instead of the previous 500 Internal Server Error status code we were returning due to the spicedb error we were receiving before. Signed-off-by: Mike Mason * handle context canceled request 500 errors Context Canceled errors from http requests should not produce 500 errors as those are client requests. This change adds an error middleware which can capture the canceled context error and return a 422 status instead. Signed-off-by: Mike Mason * validate create/update role actions requests This validates create/update role action requests before submitting the request to spicedb and handles the errors gracefully by returning 400 errors instead of generic 500 errors to the client. Signed-off-by: Mike Mason * handle role http errors better and add http tests 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 * fix and add tests for role assignment endpoints This adds http tests and fixes the http status codes for the role assignment and unassignment endpoints to resolve 500 errors occurring when provided role ids were didn't exist. Signed-off-by: Mike Mason --------- Signed-off-by: Mike Mason --- .devcontainer/Dockerfile | 2 +- go.mod | 13 +- go.sum | 44 +- internal/api/assignments.go | 31 +- internal/api/assignments_test.go | 488 ++++++++++++++++++++ internal/api/permissions.go | 29 +- internal/api/roles.go | 74 +++- internal/api/roles_test.go | 734 +++++++++++++++++++++++++++++++ internal/api/router.go | 25 ++ internal/api/router_test.go | 134 ++++++ internal/query/errors.go | 3 + internal/query/mock/mock.go | 48 +- internal/query/relations.go | 60 ++- internal/query/relations_test.go | 12 +- internal/testauth/claims.go | 34 ++ internal/testauth/testauth.go | 185 ++++++++ 16 files changed, 1847 insertions(+), 69 deletions(-) create mode 100644 internal/api/assignments_test.go create mode 100644 internal/api/roles_test.go create mode 100644 internal/api/router_test.go create mode 100644 internal/testauth/claims.go create mode 100644 internal/testauth/testauth.go diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 7920fac6..0d6421cb 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -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" diff --git a/go.mod b/go.mod index bb9cb67b..12657106 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 ) @@ -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 @@ -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 @@ -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 diff --git a/go.sum b/go.sum index 49c42e16..8c54537e 100644 --- a/go.sum +++ b/go.sum @@ -21,7 +21,9 @@ cloud.google.com/go/bigquery v1.5.0/go.mod h1:snEHRnqQbz117VIFhE8bmtwIDY80NLUZUM cloud.google.com/go/bigquery v1.7.0/go.mod h1://okPTzCYNXSlb24MZs83e2Do+h+VXtc4gLoIoXIAPc= cloud.google.com/go/bigquery v1.8.0/go.mod h1:J5hqkt3O0uAFnINi6JXValWIb1v0goeZM77hZzJN/fQ= cloud.google.com/go/compute v1.23.3 h1:6sVlXXBmbd7jNX0Ipq0trII3e4n1/MsADLK6a+aiVlk= +cloud.google.com/go/compute v1.23.3/go.mod h1:VCgBUoMnIVIR0CscqQiPJLAG25E3ZRZMzcFZeQ+h8CI= cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY= +cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA= cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7TKLgZqpHYE= cloud.google.com/go/datastore v1.1.0/go.mod h1:umbIZjpQpHh4hmRpGhH4tLFup+FVzqBi1b3c64qFpCk= cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I= @@ -51,6 +53,7 @@ github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZx github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/brianvoe/gofakeit/v6 v6.23.2 h1:lVde18uhad5wII/f5RMVFLtdQNE0HaGFuBUXmYKk8i8= +github.com/brianvoe/gofakeit/v6 v6.23.2/go.mod h1:Ow6qC71xtwm79anlwKRlWZW6zVq9D2XHE4QSSMP/rU8= github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM= github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -72,6 +75,7 @@ github.com/cncf/xds/go v0.0.0-20210805033703-aa0b78936158/go.mod h1:eXthEFrGJvWH github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4 h1:/inchEIKaYC1Akx+H+gqO04wryn5h75LSazbRlnya1k= +github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cockroachdb/apd v1.1.0 h1:3LFP3629v+1aKXU5Q37mxmRxX/pIu1nijXydLShEq5I= github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMeY4+DwBQ= github.com/cockroachdb/cockroach-go/v2 v2.3.6 h1:Wlv9TzkrG9V7i6u8dEtmXPrBzvfFp+CgJNs696rAajM= @@ -85,6 +89,7 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= +github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= @@ -97,12 +102,15 @@ github.com/envoyproxy/protoc-gen-validate v1.0.2/go.mod h1:GpiZQP3dDbg4JouG/NNS7 github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk= github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= +github.com/go-jose/go-jose/v4 v4.0.1 h1:QVEPDE3OluqXBQZDcnNvQrInro2h0e4eqNbnZSWqS6U= +github.com/go-jose/go-jose/v4 v4.0.1/go.mod h1:WVf9LFMHh/QVrmqrOfqun0C45tMe3RoiKJMPvgWwLfY= github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY= github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -123,6 +131,7 @@ github.com/golang-jwt/jwt/v5 v5.0.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVI github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/glog v1.0.0/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4= github.com/golang/glog v1.1.2 h1:DVjP2PbBOzHyzA+dn3WhHIq4NdVu3Q+pvivFICf/7fo= +github.com/golang/glog v1.1.2/go.mod h1:zR+okUeTbrL6EL3xHUDxZuEtGv04p5shwip1+mL/rLQ= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= @@ -163,6 +172,7 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= @@ -176,6 +186,7 @@ github.com/google/pprof v0.0.0-20200708004538-1a94d8640e99/go.mod h1:ZgVRPoUq/hf github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.4.0 h1:MtMxsa51/r9yyhkyLsVeVt0B+BGQZzpQiTQ4eHZ8bc4= +github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI= @@ -249,6 +260,7 @@ github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/X github.com/jzelinskie/stringz v0.0.2 h1:OSjMEYvz8tjhovgZ/6cGcPID736ubeukr35mu6RYAmg= github.com/jzelinskie/stringz v0.0.2/go.mod h1:hHYbgxJuNLRw91CmpuFsYEOyQqpDVFg8pvEh23vy4P0= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= +github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.2 h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4= @@ -257,10 +269,12 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxv github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/labstack/echo-contrib v0.15.0 h1:9K+oRU265y4Mu9zpRDv3X+DGTqUALY6oRHCSZZKCRVU= github.com/labstack/echo-contrib v0.15.0/go.mod h1:lei+qt5CLB4oa7VHTE0yEfQSEB9XTJI1LUqko9UWvo4= github.com/labstack/echo-jwt/v4 v4.2.0 h1:odSISV9JgcSCuhgQSV/6Io3i7nUmfM/QkBeR5GVJj5c= @@ -329,9 +343,11 @@ github.com/prometheus/common v0.40.0/go.mod h1:L65ZJPSmfn/UBWLQIHV7dBrKFidB/wPlF github.com/prometheus/procfs v0.11.0 h1:5EAgkfkMl659uZPbe9AS2N68a7Cc1TJbPEuGzFuRbyk= github.com/prometheus/procfs v0.11.0/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= +github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ= github.com/rs/zerolog v1.13.0/go.mod h1:YbFCdg8HfsridGWAh22vktObvhZbQsZXe4/zB0OKkWU= github.com/rs/zerolog v1.15.0/go.mod h1:xYTKnLHcpfU2225ny5qZjxnj9NvkumZYjJHlAThCjNc= @@ -344,6 +360,7 @@ github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdh github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24/go.mod h1:M+9NzErvs504Cn4c5DxATwIqPbtswREoFCre64PpcG4= github.com/shopspring/decimal v1.2.0/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= github.com/shopspring/decimal v1.3.1 h1:2Usl1nmF/WZucqkFZhnfFYxxxu8LG21F6nPQBE5gKV8= +github.com/shopspring/decimal v1.3.1/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= @@ -401,6 +418,7 @@ go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.4 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 h1:pginetY7+onl4qN1vl0xW/V/v6OBZ0vVdH+esuJgvmM= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0/go.mod h1:XiYsayHc36K3EByOO6nbAXnAWbrUxdjUROCEeeROOH8= go.opentelemetry.io/contrib/propagators/b3 v1.17.0 h1:ImOVvHnku8jijXqkwCSyYKRDt2YrnGXD4BbhcpfbfJo= +go.opentelemetry.io/contrib/propagators/b3 v1.17.0/go.mod h1:IkfUfMpKWmynvvE0264trz0sf32NRTZL4nuAN9AbWRc= go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s= go.opentelemetry.io/otel v1.16.0/go.mod h1:vl0h9NUa1D5s1nv3A5vZOYWn8av4K8Ml6JDeHrT/bx4= go.opentelemetry.io/otel/exporters/jaeger v1.16.0 h1:YhxxmXZ011C0aDZKoNw+juVWAmEfv/0W2XBOv9aHTaA= @@ -420,6 +438,7 @@ go.opentelemetry.io/otel/metric v1.16.0/go.mod h1:QE47cpOmkwipPiefDwo2wDzwJrlfxx go.opentelemetry.io/otel/sdk v1.16.0 h1:Z1Ok1YsijYL0CSJpHt4cS3wDDh7p572grzNrBMiMWgE= go.opentelemetry.io/otel/sdk v1.16.0/go.mod h1:tMsIuKXuuIWPBAOrH+eHtvhTL+SntFtXF9QD68aP6p4= go.opentelemetry.io/otel/sdk/metric v0.39.0 h1:Kun8i1eYf48kHH83RucG93ffz0zGV1sh46FAScOTuDI= +go.opentelemetry.io/otel/sdk/metric v0.39.0/go.mod h1:piDIRgjcK7u0HCL5pCA4e74qpK/jk3NiUoAHATVAmiI= go.opentelemetry.io/otel/trace v1.16.0 h1:8JRpaObFoW0pxuVPapkgH8UhHQj+bJW8jJsCZEu5MQs= go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLkqr2QVwea0ef0= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= @@ -432,6 +451,7 @@ go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= +go.uber.org/goleak v1.2.1/go.mod h1:qlT2yGI9QafXHhZZLxlSuNsMw3FFLxBr+tBRlmO1xH4= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU= @@ -457,8 +477,8 @@ golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= -golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= -golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= +golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -493,6 +513,7 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= +golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -534,6 +555,7 @@ golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6/go.mod h1:gOpvHmFTYa4Iltr golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.15.0 h1:s8pnnxNVzjWyrvYdFUQq5llS1PX2zhPXmccZv99h7uQ= +golang.org/x/oauth2 v0.15.0/go.mod h1:q48ptWNTY5XWf+JNten23lcvHpLJ0ZSxF5ttTHKVCAM= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -588,8 +610,8 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= +golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -661,6 +683,7 @@ golang.org/x/tools v0.0.0-20200825202427-b303f430e36d/go.mod h1:njjCfa9FT2d7l9Bc golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.13.0 h1:Iey4qkscZuv0VvIt8E0neZjtPVQFSc870HQ448QgEmQ= +golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -690,6 +713,7 @@ google.golang.org/appengine v1.6.1/go.mod h1:i06prIuMbXzDqacNJfV5OdTW448YApPu5ww google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/appengine v1.6.6/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/appengine v1.6.8 h1:IhEN5q69dyKagZPYMSdIjS2HqprW324FRQZJcGqPAsM= +google.golang.org/appengine v1.6.8/go.mod h1:1jJ3jBArFh5pcgW8gCtRJnepW8FzD1V44FJffLiz/Ds= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20190307195333-5fe7a883aa19/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= google.golang.org/genproto v0.0.0-20190418145605-e7d98fc518a7/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= @@ -764,11 +788,13 @@ google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqw gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:aPpfJ7XW+gOuirDoZ8gHhLh3kZ1B08FtV2bbmy7Jv3s= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/square/go-jose.v2 v2.6.0 h1:NGk74WTnPKBNUhNzQX7PYcTLUjoq7mzKk2OKbvwk2iI= +gopkg.in/square/go-jose.v2 v2.6.0/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= @@ -784,15 +810,25 @@ honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= lukechampine.com/uint128 v1.3.0 h1:cDdUVfRwDUDovz610ABgFD17nXD4/uDgVHl2sC3+sbo= +lukechampine.com/uint128 v1.3.0/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk= modernc.org/cc/v3 v3.41.0 h1:QoR1Sn3YWlmA1T4vLaKZfawdVtSiGx8H+cEojbC7v1Q= +modernc.org/cc/v3 v3.41.0/go.mod h1:Ni4zjJYJ04CDOhG7dn640WGfwBzfE0ecX8TyMB0Fv0Y= modernc.org/ccgo/v3 v3.16.14 h1:af6KNtFgsVmnDYrWk3PQCS9XT6BXe7o3ZFJKkIKvXNQ= +modernc.org/ccgo/v3 v3.16.14/go.mod h1:mPDSujUIaTNWQSG4eqKw+atqLOEbma6Ncsa94WbC9zo= modernc.org/libc v1.24.1 h1:uvJSeCKL/AgzBo2yYIPPTy82v21KgGnizcGYfBHaNuM= +modernc.org/libc v1.24.1/go.mod h1:FmfO1RLrU3MHJfyi9eYYmZBfi/R+tqZ6+hQ3yQQUkak= modernc.org/mathutil v1.6.0 h1:fRe9+AmYlaej+64JsEEhoWuAYBkOtQiMEU7n/XgfYi4= +modernc.org/mathutil v1.6.0/go.mod h1:Ui5Q9q1TR2gFm0AQRqQUaBWFLAhQpCwNcuhBOSedWPo= modernc.org/memory v1.6.0 h1:i6mzavxrE9a30whzMfwf7XWVODx2r5OYXvU46cirX7o= +modernc.org/memory v1.6.0/go.mod h1:PkUhL0Mugw21sHPeskwZW4D6VscE/GQJOnIpCnW6pSU= modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4= +modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0= modernc.org/sqlite v1.25.0 h1:AFweiwPNd/b3BoKnBOfFm+Y260guGMF+0UFk0savqeA= +modernc.org/sqlite v1.25.0/go.mod h1:FL3pVXie73rg3Rii6V/u5BoHlSoyeZeIgKZEgHARyCU= modernc.org/strutil v1.1.3 h1:fNMm+oJklMGYfU9Ylcywl0CO5O6nTfaowNsh2wpPjzY= +modernc.org/strutil v1.1.3/go.mod h1:MEHNA7PdEnEwLvspRMtWTNnp2nnyvMfkimT1NKNAGbw= modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= +modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/internal/api/assignments.go b/internal/api/assignments.go index 41fd0aa5..aec8fb82 100644 --- a/internal/api/assignments.go +++ b/internal/api/assignments.go @@ -1,10 +1,12 @@ package api import ( + "errors" "net/http" "go.infratographer.com/x/gidx" + "go.infratographer.com/permissions-api/internal/query" "go.infratographer.com/permissions-api/internal/types" "github.com/labstack/echo/v4" @@ -37,7 +39,7 @@ func (r *Router) assignmentCreate(c echo.Context) error { assigneeResource, err := r.engine.NewResourceFromID(assigneeID) if err != nil { - return echo.NewHTTPError(http.StatusBadRequest, "error creating resource").SetInternal(err) + return echo.NewHTTPError(http.StatusBadRequest, "error assigning subject").SetInternal(err) } subjectResource, err := r.currentSubject(c) @@ -51,8 +53,13 @@ func (r *Router) assignmentCreate(c echo.Context) error { } 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, actionRoleUpdate, resource); err != nil { @@ -96,8 +103,13 @@ func (r *Router) assignmentsList(c echo.Context) error { } 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 { @@ -169,8 +181,13 @@ func (r *Router) assignmentDelete(c echo.Context) error { } 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, actionRoleUpdate, resource); err != nil { diff --git a/internal/api/assignments_test.go b/internal/api/assignments_test.go new file mode 100644 index 00000000..9aec492e --- /dev/null +++ b/internal/api/assignments_test.go @@ -0,0 +1,488 @@ +package api + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.infratographer.com/x/echojwtx" + "go.infratographer.com/x/gidx" + + "go.infratographer.com/permissions-api/internal/query" + "go.infratographer.com/permissions-api/internal/query/mock" + "go.infratographer.com/permissions-api/internal/testauth" + "go.infratographer.com/permissions-api/internal/testingx" + "go.infratographer.com/permissions-api/internal/types" +) + +func TestAssignmentCreate(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + type testInput struct { + path string + json interface{} + } + + testCases := []testingx.TestCase[testInput, *httptest.ResponseRecorder]{ + { + Name: "BadSubjectID", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123/assignments", + json: map[string]interface{}{ + "subject_id": "bad-id", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusBadRequest, res.Success.Code) + }, + }, + { + Name: "InvalidRoleID", + Input: testInput{ + path: "/api/v1/roles/bad-id/assignments", + json: map[string]interface{}{ + "subject_id": "idntusr-abc123", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "RoleNotFound", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123/assignments", + json: map[string]interface{}{ + "subject_id": "idntusr-def456", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, query.ErrRoleNotFound) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "Assigned", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123/assignments", + json: map[string]interface{}{ + "subject_id": "idntusr-def456", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("AssignSubjectRole").Return(nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var retResp createAssignmentResponse + + err := json.NewDecoder(resp.Body).Decode(&retResp) + + require.NoError(t, err) + + assert.Equal(t, http.StatusCreated, resp.StatusCode) + assert.True(t, retResp.Success) + }, + }, + } + + testFn := func(ctx context.Context, input testInput) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + var body bytes.Buffer + + if input.json != nil { + if err = json.NewEncoder(&body).Encode(input.json); err != nil { + result.Err = err + + return result + } + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, input.path, &body) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} + +func TestAssignmentsList(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + testCases := []testingx.TestCase[string, *httptest.ResponseRecorder]{ + { + Name: "RoleResourceNotFound", + Input: "/api/v1/roles/permrol-abc123/assignments", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, query.ErrRoleNotFound) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "AssignmentsRetrieved", + Input: "/api/v1/roles/permrol-abc123/assignments", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("ListAssignments").Return([]types.Resource{{ + ID: gidx.MustNewID("idntusr"), + }}, nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var ret listAssignmentsResponse + + err := json.NewDecoder(resp.Body).Decode(&ret) + + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + require.NotEmpty(t, ret.Data) + assert.True(t, strings.HasPrefix(ret.Data[0].SubjectID, "idntusr-")) + }, + }, + } + + testFn := func(ctx context.Context, path string) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, path, nil) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} + +func TestAssignmentDelete(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + type testInput struct { + path string + json interface{} + } + + testCases := []testingx.TestCase[testInput, *httptest.ResponseRecorder]{ + { + Name: "BadSubjectID", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123/assignments", + json: map[string]interface{}{ + "subject_id": "bad-id", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusBadRequest, res.Success.Code) + }, + }, + { + Name: "InvalidRoleID", + Input: testInput{ + path: "/api/v1/roles/bad-id/assignments", + json: map[string]interface{}{ + "subject_id": "idntusr-abc123", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "RoleNotFound", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123/assignments", + json: map[string]interface{}{ + "subject_id": "idntusr-def456", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, query.ErrRoleNotFound) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "Unassigned", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123/assignments", + json: map[string]interface{}{ + "subject_id": "idntusr-def456", + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("UnassignSubjectRole").Return(nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var retResp deleteAssignmentResponse + + err := json.NewDecoder(resp.Body).Decode(&retResp) + + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.True(t, retResp.Success) + }, + }, + } + + testFn := func(ctx context.Context, input testInput) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + var body bytes.Buffer + + if input.json != nil { + if err = json.NewEncoder(&body).Encode(input.json); err != nil { + result.Err = err + + return result + } + } + + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, input.path, &body) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} diff --git a/internal/api/permissions.go b/internal/api/permissions.go index 1be231a9..fdb500d5 100644 --- a/internal/api/permissions.go +++ b/internal/api/permissions.go @@ -94,6 +94,14 @@ func (r *Router) checkActionWithResponse(ctx context.Context, subjectResource ty ) return echo.NewHTTPError(http.StatusForbidden, msg).SetInternal(err) + case errors.Is(err, query.ErrInvalidAction): + msg := fmt.Sprintf( + "invalid action '%s' for resource '%s'", + action, + resource.ID.String(), + ) + + return echo.NewHTTPError(http.StatusBadRequest, msg).SetInternal(err) case err != nil: return echo.NewHTTPError(http.StatusInternalServerError, "an error occurred checking permissions").SetInternal(err) default: @@ -218,6 +226,7 @@ func (r *Router) checkAllActions(c echo.Context) error { } var ( + badRequestErrors int unauthorizedErrors int internalErrors int allErrors []error @@ -227,7 +236,8 @@ func (r *Router) checkAllActions(c echo.Context) error { select { case result := <-resultsCh: if result.Error != nil { - if errors.Is(result.Error, query.ErrActionNotAssigned) { + switch { + case errors.Is(result.Error, query.ErrActionNotAssigned): err := fmt.Errorf( "%w: subject '%s' does not have permission to perform action '%s' on resource '%s'", ErrAccessDenied, @@ -239,7 +249,18 @@ func (r *Router) checkAllActions(c echo.Context) error { unauthorizedErrors++ allErrors = append(allErrors, err) - } else { + case errors.Is(result.Error, query.ErrInvalidAction): + err := fmt.Errorf( + "%w: invalid action '%s' for resource '%s'", + result.Error, + result.Request.Action, + result.Request.Resource.ID, + ) + + badRequestErrors++ + + allErrors = append(allErrors, err) + default: err := fmt.Errorf("check %d: %w", result.Request.Index, result.Error) internalErrors++ @@ -272,6 +293,10 @@ func (r *Router) checkAllActions(c echo.Context) error { return echo.NewHTTPError(http.StatusForbidden, msg).SetInternal(multierr.Combine(allErrors...)) } + if badRequestErrors != 0 { + return echo.NewHTTPError(http.StatusBadRequest, multierr.Combine(allErrors...)) + } + return nil } diff --git a/internal/api/roles.go b/internal/api/roles.go index 50166100..05b1f8f6 100644 --- a/internal/api/roles.go +++ b/internal/api/roles.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/otel/trace" "go.infratographer.com/permissions-api/internal/query" + "go.infratographer.com/permissions-api/internal/storage" ) const ( @@ -54,7 +55,14 @@ func (r *Router) roleCreate(c echo.Context) error { } role, err := r.engine.CreateRole(ctx, subjectResource, resource, reqBody.Name, reqBody.Actions) - if err != nil { + + switch { + case err == nil: + case errors.Is(err, query.ErrInvalidAction): + return echo.NewHTTPError(http.StatusBadRequest, "error creating resource: "+err.Error()) + case errors.Is(err, storage.ErrRoleAlreadyExists), errors.Is(err, storage.ErrRoleNameTaken): + return echo.NewHTTPError(http.StatusConflict, "error creating resource: "+err.Error()) + default: return echo.NewHTTPError(http.StatusInternalServerError, "error creating resource").SetInternal(err) } @@ -103,11 +111,12 @@ func (r *Router) roleUpdate(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 { - if errors.Is(err, query.ErrRoleNotFound) { - return echo.NewHTTPError(http.StatusNotFound, "resource not found").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) } @@ -116,7 +125,14 @@ func (r *Router) roleUpdate(c echo.Context) error { } role, err := r.engine.UpdateRole(ctx, subjectResource, roleResource, reqBody.Name, reqBody.Actions) - if err != nil { + + switch { + case err == nil: + case errors.Is(err, query.ErrInvalidAction): + return echo.NewHTTPError(http.StatusBadRequest, "error updating resource: "+err.Error()) + case errors.Is(err, storage.ErrRoleNameTaken): + return echo.NewHTTPError(http.StatusConflict, "error updating resource: "+err.Error()) + default: return echo.NewHTTPError(http.StatusInternalServerError, "error updating resource").SetInternal(err) } @@ -158,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 @@ -169,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{ @@ -262,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{ @@ -305,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 { diff --git a/internal/api/roles_test.go b/internal/api/roles_test.go new file mode 100644 index 00000000..27fec82f --- /dev/null +++ b/internal/api/roles_test.go @@ -0,0 +1,734 @@ +package api + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.infratographer.com/x/echojwtx" + "go.infratographer.com/x/gidx" + + "go.infratographer.com/permissions-api/internal/query" + "go.infratographer.com/permissions-api/internal/query/mock" + "go.infratographer.com/permissions-api/internal/storage" + "go.infratographer.com/permissions-api/internal/testauth" + "go.infratographer.com/permissions-api/internal/testingx" + "go.infratographer.com/permissions-api/internal/types" +) + +var contextKeyEngine = struct{}{} + +func TestRoleCreate(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + type testInput struct { + path string + json interface{} + } + + testCases := []testingx.TestCase[testInput, *httptest.ResponseRecorder]{ + { + Name: "ErrInvalidAction", + Input: testInput{ + path: "/api/v1/resources/tnntten-abc123/roles", + json: map[string]interface{}{ + "name": "my role", + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("SubjectHasPermission").Return(nil) + engine.On("CreateRole").Return(types.Role{}, query.ErrInvalidAction) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusBadRequest, res.Success.Code) + }, + }, + { + Name: "ErrRoleAlreadyExists", + Input: testInput{ + path: "/api/v1/resources/tnntten-abc123/roles", + json: map[string]interface{}{ + "name": "my role", + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("SubjectHasPermission").Return(nil) + engine.On("CreateRole").Return(types.Role{}, storage.ErrRoleAlreadyExists) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusConflict, res.Success.Code) + }, + }, + { + Name: "ErrRoleNameTaken", + Input: testInput{ + path: "/api/v1/resources/tnntten-abc123/roles", + json: map[string]interface{}{ + "name": "my role", + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("SubjectHasPermission").Return(nil) + engine.On("CreateRole").Return(types.Role{}, storage.ErrRoleNameTaken) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusConflict, res.Success.Code) + }, + }, + { + Name: "RoleCreated", + Input: testInput{ + path: "/api/v1/resources/tnntten-abc123/roles", + json: map[string]interface{}{ + "name": t.Name(), + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("SubjectHasPermission").Return(nil) + engine.On("CreateRole").Return(types.Role{ + ID: gidx.MustNewID(query.RolePrefix), + Name: t.Name(), + Actions: []string{ + "action1", + "action2", + }, + CreatedBy: "idntusr-abc123", + UpdatedBy: "idntusr-def456", + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var role roleResponse + + err := json.NewDecoder(resp.Body).Decode(&role) + + require.NoError(t, err) + + assert.Equal(t, http.StatusCreated, resp.StatusCode) + assert.Equal(t, query.RolePrefix, role.ID.Prefix()) + assert.Equal(t, t.Name(), role.Name) + assert.Equal(t, []string{"action1", "action2"}, role.Actions) + assert.Equal(t, "idntusr-abc123", role.CreatedBy.String()) + assert.Equal(t, "idntusr-def456", role.UpdatedBy.String()) + assert.NotEmpty(t, role.CreatedAt) + assert.NotEmpty(t, role.UpdatedAt) + }, + }, + } + + testFn := func(ctx context.Context, input testInput) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + var body bytes.Buffer + + if input.json != nil { + if err = json.NewEncoder(&body).Encode(input.json); err != nil { + result.Err = err + + return result + } + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://127.0.0.1"+input.path, &body) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} + +func TestRoleUpdate(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + type testInput struct { + path string + json interface{} + } + + testCases := []testingx.TestCase[testInput, *httptest.ResponseRecorder]{ + { + Name: "ErrInvalidAction", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123", + json: map[string]interface{}{ + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("UpdateRole").Return(types.Role{}, query.ErrInvalidAction) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusBadRequest, res.Success.Code) + }, + }, + { + Name: "ErrRoleNameTaken", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123", + json: map[string]interface{}{ + "name": "my role", + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("UpdateRole").Return(types.Role{}, storage.ErrRoleNameTaken) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusConflict, res.Success.Code) + }, + }, + { + Name: "RoleResourceNotFound", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123", + json: map[string]interface{}{ + "name": "my role", + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, query.ErrRoleNotFound) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "RoleUpdated", + Input: testInput{ + path: "/api/v1/roles/permrol-abc123", + json: map[string]interface{}{ + "name": t.Name(), + "actions": []string{ + "action1", + "action2", + }, + }, + }, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("UpdateRole").Return(types.Role{ + ID: gidx.MustNewID(query.RolePrefix), + Name: t.Name(), + Actions: []string{ + "action1", + "action2", + }, + CreatedBy: "idntusr-abc123", + UpdatedBy: "idntusr-def456", + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var role roleResponse + + err := json.NewDecoder(resp.Body).Decode(&role) + + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, query.RolePrefix, role.ID.Prefix()) + assert.Equal(t, t.Name(), role.Name) + assert.Equal(t, []string{"action1", "action2"}, role.Actions) + assert.Equal(t, "idntusr-abc123", role.CreatedBy.String()) + assert.Equal(t, "idntusr-def456", role.UpdatedBy.String()) + assert.NotEmpty(t, role.CreatedAt) + assert.NotEmpty(t, role.UpdatedAt) + }, + }, + } + + testFn := func(ctx context.Context, input testInput) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + var body bytes.Buffer + + if input.json != nil { + if err = json.NewEncoder(&body).Encode(input.json); err != nil { + result.Err = err + + return result + } + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPatch, "http://127.0.0.1"+input.path, &body) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} + +func TestRoleGet(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + testCases := []testingx.TestCase[string, *httptest.ResponseRecorder]{ + { + Name: "RoleResourceNotFound", + Input: "/api/v1/roles/permrol-abc123", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, query.ErrRoleNotFound) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "RoleRetrieved", + Input: "/api/v1/roles/permrol-abc123", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("GetRole").Return(types.Role{ + ID: gidx.MustNewID(query.RolePrefix), + Name: t.Name(), + Actions: []string{ + "action1", + "action2", + }, + CreatedBy: "idntusr-abc123", + UpdatedBy: "idntusr-def456", + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var role roleResponse + + err := json.NewDecoder(resp.Body).Decode(&role) + + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, query.RolePrefix, role.ID.Prefix()) + assert.Equal(t, t.Name(), role.Name) + assert.Equal(t, []string{"action1", "action2"}, role.Actions) + assert.Equal(t, "idntusr-abc123", role.CreatedBy.String()) + assert.Equal(t, "idntusr-def456", role.UpdatedBy.String()) + assert.NotEmpty(t, role.CreatedAt) + assert.NotEmpty(t, role.UpdatedAt) + }, + }, + } + + testFn := func(ctx context.Context, path string) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://127.0.0.1"+path, nil) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} + +func TestRoleDelete(t *testing.T) { + ctx := context.Background() + + authsrv := testauth.NewServer(t) + + testCases := []testingx.TestCase[string, *httptest.ResponseRecorder]{ + { + Name: "UnknownError", + Input: "/api/v1/roles/permrol-abc123", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("DeleteRole").Return(io.EOF) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusInternalServerError, res.Success.Code) + }, + }, + { + Name: "RoleResourceNotFound", + Input: "/api/v1/roles/permrol-abc123", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, query.ErrRoleNotFound) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusNotFound, res.Success.Code) + }, + }, + { + Name: "RoleDeleted", + Input: "/api/v1/roles/permrol-abc123", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + engine := mock.Engine{ + Namespace: "test", + } + + engine.On("GetRoleResource").Return(types.Resource{}, nil) + engine.On("SubjectHasPermission").Return(nil) + engine.On("DeleteRole").Return(nil) + + return context.WithValue(ctx, contextKeyEngine, &engine) + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + engine := ctx.Value(contextKeyEngine).(*mock.Engine) + engine.AssertExpectations(t) + + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + resp := res.Success.Result() + + defer resp.Body.Close() + + var roleResp deleteRoleResponse + + err := json.NewDecoder(resp.Body).Decode(&roleResp) + + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.True(t, roleResp.Success) + }, + }, + } + + testFn := func(ctx context.Context, path string) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + engine := ctx.Value(contextKeyEngine).(query.Engine) + + router, err := NewRouter(echojwtx.AuthConfig{Issuer: authsrv.Issuer}, engine) + if err != nil { + result.Err = err + + return result + } + + e := echo.New() + e.Use(echoTestLogger(t, e)) + + router.Routes(e.Group("")) + + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, "http://127.0.0.1"+path, nil) + if err != nil { + result.Err = err + + return result + } + + req.Header.Set("Authorization", "Bearer "+authsrv.TSignSubject(t, "idntusr-abc123")) + + resp := httptest.NewRecorder() + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} + +func echoTestLogger(t *testing.T, e *echo.Echo) echo.MiddlewareFunc { + t.Helper() + + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + err := next(c) + + req := c.Request() + + if err == nil { + t.Logf("%s %s", req.Method, req.URL.String()) + + return nil + } + + t.Logf("%s %s: %s", req.Method, req.URL.String(), err.Error()) + + return err + } + } +} diff --git a/internal/api/router.go b/internal/api/router.go index bf53a6ff..4ce585b9 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -2,6 +2,7 @@ package api import ( "context" + "errors" "net/http" "github.com/labstack/echo/v4" @@ -51,6 +52,8 @@ func NewRouter(authCfg echojwtx.AuthConfig, engine query.Engine, options ...Opti // Routes will add the routes for this API version to a router group func (r *Router) Routes(rg *echo.Group) { + rg.Use(errorMiddleware) + v1 := rg.Group("api/v1") { v1.Use(r.authMW) @@ -74,6 +77,28 @@ func (r *Router) Routes(rg *echo.Group) { } } +func errorMiddleware(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + err := next(c) + + if err == nil { + return nil + } + + // If error is an echo.HTTPError simply return it + if _, ok := err.(*echo.HTTPError); ok { + return err + } + + switch { + case errors.Is(err, context.Canceled): + return echo.ErrUnprocessableEntity.WithInternal(err) + default: + return err + } + } +} + // Option defines a router option function. type Option func(r *Router) error diff --git a/internal/api/router_test.go b/internal/api/router_test.go new file mode 100644 index 00000000..1d4ea5be --- /dev/null +++ b/internal/api/router_test.go @@ -0,0 +1,134 @@ +package api + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.infratographer.com/permissions-api/internal/testingx" +) + +func TestErrorMiddleware(t *testing.T) { + ctx := context.Background() + + e := echo.New() + e.Use(echoTestLogger(t, e)) + e.Use(errorMiddleware) + + e.GET("/test", func(c echo.Context) error { + errType := c.QueryParam("error") + + select { + case <-c.Request().Context().Done(): + return c.Request().Context().Err() + case <-time.After(time.Second): + } + + switch errType { + case "": + case "echo": + return echo.ErrTeapot + case "other": + return io.ErrUnexpectedEOF + } + + return nil + }) + + type testinput struct { + path string + delay time.Duration + } + + testCases := []testingx.TestCase[testinput, *httptest.ResponseRecorder]{ + { + Name: "NotCanceled", + Input: testinput{ + path: "/test", + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusOK, res.Success.Code) + }, + }, + { + Name: "EchoError", + Input: testinput{ + path: "/test?error=echo", + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusTeapot, res.Success.Code) + }, + }, + { + Name: "OtherError", + Input: testinput{ + path: "/test?error=other", + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusInternalServerError, res.Success.Code) + }, + }, + { + Name: "Canceled", + Input: testinput{ + path: "/test", + delay: time.Second / 2, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + require.NoError(t, res.Err) + require.NotNil(t, res.Success) + + assert.Equal(t, http.StatusUnprocessableEntity, res.Success.Code) + }, + }, + } + + testFn := func(ctx context.Context, input testinput) testingx.TestResult[*httptest.ResponseRecorder] { + result := testingx.TestResult[*httptest.ResponseRecorder]{} + + ctx, cancel := context.WithCancel(ctx) + + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, input.path, nil) + if err != nil { + result.Err = err + + return result + } + + resp := httptest.NewRecorder() + + if input.delay != 0 { + go func() { + time.Sleep(input.delay) + + cancel() + }() + } + + e.ServeHTTP(resp, req) + + result.Success = resp + + return result + } + + testingx.RunTests(ctx, t, testCases, testFn) +} diff --git a/internal/query/errors.go b/internal/query/errors.go index 7373102e..57212232 100644 --- a/internal/query/errors.go +++ b/internal/query/errors.go @@ -7,6 +7,9 @@ var ( // the given request. ErrActionNotAssigned = errors.New("the subject does not have permissions to complete this request") + // ErrInvalidAction represents an error condition where the action provided is not valid for the provided resource. + ErrInvalidAction = errors.New("invalid action for resource") + // ErrInvalidReference represents an error condition where a given SpiceDB object reference is for some reason invalid. ErrInvalidReference = errors.New("invalid reference") diff --git a/internal/query/mock/mock.go b/internal/query/mock/mock.go index c9e9a589..dc68b901 100644 --- a/internal/query/mock/mock.go +++ b/internal/query/mock/mock.go @@ -3,7 +3,6 @@ package mock import ( "context" "errors" - "time" "go.infratographer.com/permissions-api/internal/iapl" "go.infratographer.com/permissions-api/internal/query" @@ -28,12 +27,16 @@ type Engine struct { // AssignSubjectRole does nothing but satisfies the Engine interface. func (e *Engine) AssignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) error { - return nil + args := e.Called() + + return args.Error(0) } // UnassignSubjectRole does nothing but satisfies the Engine interface. func (e *Engine) UnassignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) error { - return nil + args := e.Called() + + return args.Error(0) } // CreateRelationships does nothing but satisfies the Engine interface. @@ -45,27 +48,16 @@ func (e *Engine) CreateRelationships(ctx context.Context, rels []types.Relations // CreateRole creates a Role object and does not persist it anywhere. func (e *Engine) CreateRole(ctx context.Context, actor, res types.Resource, name string, actions []string) (types.Role, error) { - // Copy actions instead of using the given slice - outActions := make([]string, len(actions)) - - copy(outActions, actions) - - role := types.Role{ - ID: gidx.MustNewID(query.ApplicationPrefix), - Name: name, - Actions: outActions, - CreatedBy: actor.ID, - UpdatedBy: actor.ID, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } + args := e.Called() - return role, nil + retRole := args.Get(0).(types.Role) + + return retRole, args.Error(1) } // UpdateRole returns the provided mock results. func (e *Engine) UpdateRole(ctx context.Context, actor, roleResource types.Resource, newName string, newActions []string) (types.Role, error) { - args := e.Called(actor, roleResource, newName, newActions) + args := e.Called() retRole := args.Get(0).(types.Role) @@ -74,17 +66,29 @@ func (e *Engine) UpdateRole(ctx context.Context, actor, roleResource types.Resou // GetRole returns nothing but satisfies the Engine interface. func (e *Engine) GetRole(ctx context.Context, roleResource types.Resource) (types.Role, error) { - return types.Role{}, nil + args := e.Called() + + retRole := args.Get(0).(types.Role) + + return retRole, args.Error(1) } // GetRoleResource returns nothing but satisfies the Engine interface. func (e *Engine) GetRoleResource(ctx context.Context, roleResource types.Resource) (types.Resource, error) { - return types.Resource{}, nil + args := e.Called() + + retResc := args.Get(0).(types.Resource) + + return retResc, args.Error(1) } // ListAssignments returns nothing but satisfies the Engine interface. func (e *Engine) ListAssignments(ctx context.Context, role types.Role) ([]types.Resource, error) { - return nil, nil + args := e.Called() + + ret := args.Get(0).([]types.Resource) + + return ret, args.Error(1) } // ListRelationshipsFrom returns nothing but satisfies the Engine interface. diff --git a/internal/query/relations.go b/internal/query/relations.go index 4c3b8ab6..452b5590 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -14,6 +14,7 @@ import ( "go.opentelemetry.io/otel/trace" "go.uber.org/multierr" "go.uber.org/zap" + "golang.org/x/exp/slices" "go.infratographer.com/permissions-api/internal/storage" "go.infratographer.com/permissions-api/internal/types" @@ -67,6 +68,28 @@ func resourceToSpiceDBRef(namespace string, r types.Resource) *pb.ObjectReferenc } } +func (e *engine) validateResourceActions(resource types.Resource, actions ...string) error { + var invalidActions []string + + for _, action := range actions { + containsFn := func(sliceAction types.Action) bool { + return sliceAction.Name == action + } + + rescType := e.schemaTypeMap[resource.Type] + + if !slices.ContainsFunc(rescType.Actions, containsFn) { + invalidActions = append(invalidActions, action) + } + } + + if len(invalidActions) == 0 { + return nil + } + + return fmt.Errorf("%w: %s for %s", ErrInvalidAction, strings.Join(invalidActions, ","), resource.Type) +} + // SubjectHasPermission checks if the given subject can do the given action on the given resource func (e *engine) SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource) error { ctx, span := e.tracer.Start( @@ -98,16 +121,21 @@ func (e *engine) SubjectHasPermission(ctx context.Context, subject types.Resourc ), ) - req := &pb.CheckPermissionRequest{ - Consistency: consistency, - Resource: resourceToSpiceDBRef(e.namespace, resource), - Permission: action, - Subject: &pb.SubjectReference{ - Object: resourceToSpiceDBRef(e.namespace, subject), - }, - } + err := e.validateResourceActions(resource, action) - err := e.checkPermission(ctx, req) + // Only check permissions if the requested action exists in the policy. + if err == nil { + req := &pb.CheckPermissionRequest{ + Consistency: consistency, + Resource: resourceToSpiceDBRef(e.namespace, resource), + Permission: action, + Subject: &pb.SubjectReference{ + Object: resourceToSpiceDBRef(e.namespace, subject), + }, + } + + err = e.checkPermission(ctx, req) + } switch { case err == nil: @@ -117,7 +145,7 @@ func (e *engine) SubjectHasPermission(ctx context.Context, subject types.Resourc outcomeAllowed, ), ) - case errors.Is(err, ErrActionNotAssigned): + case errors.Is(err, ErrActionNotAssigned), errors.Is(err, ErrInvalidAction): span.SetAttributes( attribute.String( "permissions.outcome", @@ -281,6 +309,10 @@ func (e *engine) CreateRole(ctx context.Context, actor, res types.Resource, role defer span.End() + if err := e.validateResourceActions(res, actions...); err != nil { + return types.Role{}, err + } + roleName = strings.TrimSpace(roleName) role := newRole(roleName, actions) @@ -373,6 +405,10 @@ func (e *engine) UpdateRole(ctx context.Context, actor, roleResource types.Resou defer span.End() + if err := e.validateResourceActions(roleResource, newActions...); err != nil { + return types.Role{}, err + } + dbCtx, err := e.store.BeginContext(ctx) if err != nil { return types.Role{}, err @@ -1090,6 +1126,10 @@ func (e *engine) DeleteRole(ctx context.Context, roleResource types.Resource) er if err != nil { logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + if errors.Is(err, storage.ErrNoRoleFound) { + return ErrRoleNotFound + } + return err } diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go index 2235c612..30da7688 100644 --- a/internal/query/relations_test.go +++ b/internal/query/relations_test.go @@ -811,7 +811,7 @@ func TestSubjectActions(t *testing.T) { }, }, { - Name: "BadAction", + Name: "DeniedAction", Input: testInput{ resource: tenRes, action: "loadbalancer_delete", @@ -820,6 +820,16 @@ func TestSubjectActions(t *testing.T) { assert.ErrorIs(t, res.Err, ErrActionNotAssigned) }, }, + { + Name: "BadAction", + Input: testInput{ + resource: tenRes, + action: "bad_action", + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[any]) { + assert.ErrorIs(t, res.Err, ErrInvalidAction) + }, + }, { Name: "Success", Input: testInput{ diff --git a/internal/testauth/claims.go b/internal/testauth/claims.go new file mode 100644 index 00000000..d240e738 --- /dev/null +++ b/internal/testauth/claims.go @@ -0,0 +1,34 @@ +package testauth + +import "github.com/go-jose/go-jose/v4/jwt" + +// ClaimOption is a claim option definition. +type ClaimOption func(*jwt.Claims) + +// Subject lets you specify a subject claim option. +func Subject(v string) ClaimOption { + return func(c *jwt.Claims) { + c.Subject = v + } +} + +// Audience lets you specify an audience claim option. +func Audience(v ...string) ClaimOption { + return func(c *jwt.Claims) { + c.Audience = jwt.Audience(v) + } +} + +// Expiry lets you specify an expiry claim option. +func Expiry(v *jwt.NumericDate) ClaimOption { + return func(c *jwt.Claims) { + c.Expiry = v + } +} + +// NotBefore lets you specify a not before claim option. +func NotBefore(v *jwt.NumericDate) ClaimOption { + return func(c *jwt.Claims) { + c.NotBefore = v + } +} diff --git a/internal/testauth/testauth.go b/internal/testauth/testauth.go new file mode 100644 index 00000000..188f2425 --- /dev/null +++ b/internal/testauth/testauth.go @@ -0,0 +1,185 @@ +// Package testauth implements a simple JWKS file server and token signer +// for use in test packages when jwt validation is required. +package testauth + +import ( + "crypto/rand" + "crypto/rsa" + "errors" + "fmt" + "net" + "net/http" + "sync" + "testing" + "time" + + "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/require" +) + +const ( + keySize = 2048 +) + +// Server handles serving JSON Web Key Set and signing tokens. +type Server struct { + t *testing.T + + started sync.Once + hasStarted bool + + cleanup []func() + stopped sync.Once + + engine *echo.Echo + + kid string + privKey *rsa.PrivateKey + + signer jose.Signer + + Issuer string +} + +// Start starts an unstarted server. +func (s *Server) Start() { + s.started.Do(func() { + e := echo.New() + + e.GET("/.well-known/openid-configuration", s.handleOIDC) + e.GET("/.well-known/jwks.json", s.handleJWKS) + + listener, err := net.Listen("tcp", ":0") + + require.NoError(s.t, err) + + srv := &http.Server{ + Handler: e, + } + + go s.serve(srv, listener) + + s.cleanup = append(s.cleanup, func() { + _ = srv.Close() //nolint:errcheck // error check not needed + }) + + s.engine = e + s.Issuer = fmt.Sprintf("http://127.0.0.1:%d", listener.Addr().(*net.TCPAddr).Port) + s.hasStarted = true + }) +} + +// Stop shuts down the auth server. +func (s *Server) Stop() { + // Don't stop unless we've started. + if !s.hasStarted { + return + } + + s.stopped.Do(func() { + for i := len(s.cleanup) - 1; i > 0; i-- { + s.cleanup[i]() + } + }) +} + +func (s *Server) serve(srv *http.Server, listener net.Listener) { + err := srv.Serve(listener) + + switch { + case err == nil: + case errors.Is(err, http.ErrServerClosed): + default: + s.t.Error("unexpected error from Server:", err) + s.t.Fail() + } +} + +func (s *Server) handleOIDC(c echo.Context) error { + return c.JSON(http.StatusOK, echo.Map{ + "jwks_uri": fmt.Sprintf(s.Issuer + "/.well-known/jwks.json"), + }) +} + +func (s *Server) handleJWKS(c echo.Context) error { + return c.JSON(http.StatusOK, jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + KeyID: s.kid, + Key: s.privKey, + }, + }, + }) +} + +func (s *Server) buildClaims(options ...ClaimOption) jwt.Builder { + claims := jwt.Claims{ + Issuer: s.Issuer, + NotBefore: jwt.NewNumericDate(time.Now().Add(-2 * time.Hour)), + } + + for _, opt := range options { + opt(&claims) + } + + return jwt.Signed(s.signer).Claims(claims) +} + +// TSignSubject returns a new token string with the provided subject. +// Additional claims may be provided as options. +// Any errors produced will result in the passed test argument failing. +func (s *Server) TSignSubject(t *testing.T, subject string, options ...ClaimOption) string { + options = append(options, Subject(subject)) + + claims := s.buildClaims(options...) + + token, err := claims.Serialize() + + require.NoError(t, err) + + return token +} + +// SignSubject returns a new token string with the provided subject. +// Additional claims may be provided as options. +// Any errors produced will result in the test passed when initializing Server to fail. +func (s *Server) SignSubject(subject string, options ...ClaimOption) string { + return s.TSignSubject(s.t, subject, options...) +} + +// NewUnstartedServer creates a new Server without starting it. +func NewUnstartedServer(t *testing.T) *Server { + t.Helper() + + kid := "test" + key, err := rsa.GenerateKey(rand.Reader, keySize) + + require.NoError(t, err) + + signer, err := jose.NewSigner( + jose.SigningKey{ + Algorithm: jose.RS256, + Key: key, + }, (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", kid), + ) + + require.NoError(t, err) + + return &Server{ + t: t, + kid: kid, + privKey: key, + signer: signer, + } +} + +// NewServer creates a new Server and starts it. +func NewServer(t *testing.T) *Server { + s := NewUnstartedServer(t) + + s.Start() + + return s +}