Skip to content

Commit 6ee45c4

Browse files
jnschaeffermikemrm
andauthored
Fix client code and use resource-level authorization (#75)
* Fix client code and use resource-level authorization The provided client package was not using the correct authorization HTTP resource. This commit fixes that, and also removes tenant as a query param because we no longer need it to make authorization decisions. Signed-off-by: John Schaeffer <[email protected]> * Update pkg/client/v1/auth.go Co-authored-by: Mike Mason <[email protected]> Signed-off-by: John Schaeffer <[email protected]> * Update pkg/client/v1/auth.go Co-authored-by: Mike Mason <[email protected]> Signed-off-by: John Schaeffer <[email protected]> * remove unused path import Signed-off-by: Mike Mason <[email protected]> --------- Signed-off-by: John Schaeffer <[email protected]> Signed-off-by: John Schaeffer <[email protected]> Signed-off-by: Mike Mason <[email protected]> Co-authored-by: Mike Mason <[email protected]> Co-authored-by: Mike Mason <[email protected]>
1 parent ea03a0b commit 6ee45c4

File tree

2 files changed

+26
-35
lines changed

2 files changed

+26
-35
lines changed

internal/api/permissions.go

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import (
1010
"go.infratographer.com/x/urnx"
1111
)
1212

13-
// checkAction will check if a subject is allowed to perform an action on a resource
14-
// scoped to the tenant.
13+
// checkAction will check if a subject is allowed to perform an action on a resource.
1514
// This is the permissions check endpoint.
1615
// It will return a 200 if the subject is allowed to perform the action on the resource.
1716
// It will return a 403 if the subject is not allowed to perform the action on the resource.
@@ -20,49 +19,34 @@ import (
2019
// contain the subject of the request in the "sub" claim.
2120
//
2221
// The following query parameters are required:
23-
// - tenant: the tenant URN to check
24-
// - action: the action to check
25-
//
26-
// The following query parameters are optional:
2722
// - resource: the resource URN to check
23+
// - action: the action to check
2824
func (r *Router) checkAction(c echo.Context) error {
2925
ctx, span := tracer.Start(c.Request().Context(), "api.checkAction")
3026
defer span.End()
3127

32-
// Get the query parameters. These are mandatory.
33-
tenantURNStr, hasQuery := getParam(c, "tenant")
34-
if !hasQuery {
35-
return echo.NewHTTPError(http.StatusBadRequest, "missing tenant query parameter")
36-
}
37-
3828
action, hasQuery := getParam(c, "action")
3929
if !hasQuery {
4030
return echo.NewHTTPError(http.StatusBadRequest, "missing action query parameter")
4131
}
4232

4333
// Optional query parameters
4434
resourceURNStr, hasResourceParam := getParam(c, "resource")
35+
if !hasResourceParam {
36+
return echo.NewHTTPError(http.StatusBadRequest, "missing resource query parameter")
37+
}
4538

4639
// Query parameter validation
47-
// Note that we currently only check the tenant as a scope. The
48-
// resource is not checked as of yet.
49-
tenantURN, err := urnx.Parse(tenantURNStr)
40+
resourceURN, err := urnx.Parse(resourceURNStr)
5041
if err != nil {
51-
return echo.NewHTTPError(http.StatusBadRequest, "error processing tenant URN").SetInternal(err)
42+
return echo.NewHTTPError(http.StatusBadRequest, "error processing resource URN").SetInternal(err)
5243
}
5344

54-
tenantResource, err := r.engine.NewResourceFromURN(tenantURN)
45+
resource, err := r.engine.NewResourceFromURN(resourceURN)
5546
if err != nil {
5647
return echo.NewHTTPError(http.StatusBadRequest, "error processing tenant resource URN").SetInternal(err)
5748
}
5849

59-
if hasResourceParam {
60-
_, err := urnx.Parse(resourceURNStr)
61-
if err != nil {
62-
return echo.NewHTTPError(http.StatusBadRequest, "error processing resource URN").SetInternal(err)
63-
}
64-
}
65-
6650
// Subject validation
6751
subject, err := currentSubject(c)
6852
if err != nil {
@@ -75,10 +59,10 @@ func (r *Router) checkAction(c echo.Context) error {
7559
}
7660

7761
// Check the permissions
78-
err = r.engine.SubjectHasPermission(ctx, subjectResource, action, tenantResource, "")
62+
err = r.engine.SubjectHasPermission(ctx, subjectResource, action, resource, "")
7963
if err != nil && errors.Is(err, query.ErrActionNotAssigned) {
80-
msg := fmt.Sprintf("subject '%s' does not have permission to perform action '%s' on resource '%s' scoped on tenant '%s'",
81-
subject, action, resourceURNStr, tenantURNStr)
64+
msg := fmt.Sprintf("subject '%s' does not have permission to perform action '%s' on resource '%s'",
65+
subject, action, resourceURNStr)
8266

8367
return echo.NewHTTPError(http.StatusForbidden, msg).SetInternal(err)
8468
} else if err != nil {

pkg/client/v1/auth.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"fmt"
99
"net/http"
1010
"net/url"
11-
"path"
1211
"strings"
1312

1413
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
@@ -59,14 +58,18 @@ func New(url string, doerClient Doer) (*Client, error) {
5958
}
6059

6160
// Allowed checks if the client subject is permitted exec the action on the resource
62-
func (c *Client) Allowed(ctx context.Context, action string, resourceURNPrefix string) (bool, error) {
61+
func (c *Client) Allowed(ctx context.Context, action string, resourceURN string) (bool, error) {
6362
ctx, span := tracer.Start(ctx, "SubjectHasAction", trace.WithAttributes(
6463
attribute.String("action", action),
65-
attribute.String("resource", resourceURNPrefix),
64+
attribute.String("resource", resourceURN),
6665
))
6766
defer span.End()
6867

69-
err := c.get(ctx, fmt.Sprintf("/has/%s/on/%s", action, resourceURNPrefix), map[string]string{})
68+
values := url.Values{}
69+
values.Add("action", action)
70+
values.Add("resource", resourceURN)
71+
72+
err := c.get(ctx, "/allow", values, map[string]string{})
7073
if err != nil {
7174
if errors.Is(err, ErrPermissionDenied) {
7275
return false, nil
@@ -85,22 +88,26 @@ type ServerResponse struct {
8588
StatusCode int
8689
}
8790

88-
func (c Client) get(ctx context.Context, endpoint string, resp interface{}) error {
89-
request, err := newGetRequest(ctx, c.url, endpoint)
91+
func (c Client) get(ctx context.Context, endpoint string, query url.Values, resp interface{}) error {
92+
request, err := newGetRequest(ctx, c.url, endpoint, query)
9093
if err != nil {
9194
return err
9295
}
9396

9497
return c.do(request, &resp)
9598
}
9699

97-
func newGetRequest(ctx context.Context, uri, endpoint string) (*http.Request, error) {
100+
func newGetRequest(ctx context.Context, uri, endpoint string, query url.Values) (*http.Request, error) {
98101
u, err := url.Parse(uri)
99102
if err != nil {
100103
return nil, err
101104
}
102105

103-
u.Path = path.Join(apiVersion, endpoint)
106+
u = u.JoinPath(apiVersion, endpoint)
107+
108+
if len(query) > 0 {
109+
u.RawQuery = query.Encode()
110+
}
104111

105112
return http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
106113
}

0 commit comments

Comments
 (0)