Remove MustAuthorize pattern
Signed-off-by: Bryan Frimin <bryan@probo.com>
This commit is contained in:
@@ -131,9 +131,11 @@ if err := authorize(ctx, thirdPartyID, probo.ActionThirdPartyGet); err != nil {
|
||||
}
|
||||
```
|
||||
|
||||
**MCP resolvers** use `MustAuthorize` which panics (caught by middleware):
|
||||
**MCP resolvers** use `Authorize` and return early on error:
|
||||
```go
|
||||
r.MustAuthorize(ctx, input.ID, probo.ActionThirdPartyGet)
|
||||
if err := r.Authorize(ctx, input.ID, probo.ActionThirdPartyGet); err != nil {
|
||||
return nil, types.GetThirdPartyOutput{}, err
|
||||
}
|
||||
```
|
||||
|
||||
## File locations
|
||||
@@ -181,7 +183,7 @@ When adding a new entity that needs authorization:
|
||||
2. **Role policies** — wire actions into the appropriate role policies in `pkg/probo/policies.go` (`OwnerPolicy`, `AdminPolicy`, `ViewerPolicy`, etc.) with `organization_id` condition
|
||||
3. **`AuthorizationAttributes`** — implement on the `coredata` entity struct, returning at minimum `{"organization_id": ...}` (use the denormalized `OrganizationID` field — see coredata doc)
|
||||
4. **Entity type registry** — register in `pkg/coredata/entity_type_reg.go` and `NewEntityFromID` so the authorizer can construct the entity from its GID
|
||||
5. **Resolver calls** — add `r.authorize(ctx, id, probo.ActionEntityGet)` in GraphQL resolvers and `r.MustAuthorize(ctx, id, probo.ActionEntityGet)` in MCP resolvers
|
||||
5. **Resolver calls** — add `r.authorize(ctx, id, probo.ActionEntityGet)` in GraphQL resolvers and `if err := r.Authorize(ctx, id, probo.ActionEntityGet); err != nil { return nil, types.GetEntityOutput{}, err }` in MCP resolvers
|
||||
|
||||
## Key patterns
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ MCP tools are defined in `pkg/server/api/mcp/v1/specification.yaml` and generate
|
||||
|
||||
**Hand-written** (edit these):
|
||||
- `specification.yaml` — tool definitions, input/output schemas, component schemas
|
||||
- `resolver.go` — `Resolver` struct, `MustAuthorize`, service accessors
|
||||
- `resolver.go` — `Resolver` struct, `Authorize`, service accessors
|
||||
- `helpers.go` — pagination helpers, `UnwrapOmittable`
|
||||
- `types/*.go` (except `types/types.go`) — type conversion helpers (`NewThirdParty()`, `NewListThirdPartiesOutput()`, etc.)
|
||||
- `schema.resolvers.go` — tool implementation bodies (stubs generated, you edit the bodies)
|
||||
@@ -58,14 +58,16 @@ func (r *Resolver) ListThirdPartiesTool(
|
||||
) (*mcp.CallToolResult, types.ListThirdPartiesOutput, error)
|
||||
```
|
||||
|
||||
First return is always `nil`. Errors are either returned (for recoverable) or panicked (for authorization and unexpected failures).
|
||||
First return is always `nil`. Authorization errors are returned and handled like other recoverable tool errors.
|
||||
|
||||
## Authorization
|
||||
|
||||
Use `MustAuthorize` which panics on failure (caught by middleware):
|
||||
Use `Authorize` with an early return:
|
||||
|
||||
```go
|
||||
r.MustAuthorize(ctx, input.OrganizationID, probo.ActionThirdPartyList)
|
||||
if err := r.Authorize(ctx, input.OrganizationID, probo.ActionThirdPartyList); err != nil {
|
||||
return nil, types.ListThirdPartiesOutput{}, err
|
||||
}
|
||||
```
|
||||
|
||||
## Common resolver patterns
|
||||
@@ -73,7 +75,9 @@ r.MustAuthorize(ctx, input.OrganizationID, probo.ActionThirdPartyList)
|
||||
**List with pagination:**
|
||||
```go
|
||||
func (r *Resolver) ListThirdPartiesTool(ctx context.Context, req *mcp.CallToolRequest, input *types.ListThirdPartiesInput) (*mcp.CallToolResult, types.ListThirdPartiesOutput, error) {
|
||||
r.MustAuthorize(ctx, input.OrganizationID, probo.ActionThirdPartyList)
|
||||
if err := r.Authorize(ctx, input.OrganizationID, probo.ActionThirdPartyList); err != nil {
|
||||
return nil, types.ListThirdPartiesOutput{}, err
|
||||
}
|
||||
|
||||
prb := r.ProboService(ctx, input.OrganizationID)
|
||||
|
||||
@@ -102,7 +106,10 @@ func (r *Resolver) ListThirdPartiesTool(ctx context.Context, req *mcp.CallToolRe
|
||||
**Get single resource:**
|
||||
```go
|
||||
func (r *Resolver) GetRiskTool(ctx context.Context, req *mcp.CallToolRequest, input *types.GetRiskInput) (*mcp.CallToolResult, types.GetRiskOutput, error) {
|
||||
r.MustAuthorize(ctx, input.ID, probo.ActionRiskGet)
|
||||
if err := r.Authorize(ctx, input.ID, probo.ActionRiskGet); err != nil {
|
||||
return nil, types.GetRiskOutput{}, err
|
||||
}
|
||||
|
||||
prb := r.ProboService(ctx, input.ID)
|
||||
|
||||
risk, err := prb.Risks.Get(ctx, input.ID)
|
||||
@@ -117,7 +124,10 @@ func (r *Resolver) GetRiskTool(ctx context.Context, req *mcp.CallToolRequest, in
|
||||
**Create:**
|
||||
```go
|
||||
func (r *Resolver) AddRiskTool(ctx context.Context, req *mcp.CallToolRequest, input *types.AddRiskInput) (*mcp.CallToolResult, types.AddRiskOutput, error) {
|
||||
r.MustAuthorize(ctx, input.OrganizationID, probo.ActionRiskCreate)
|
||||
if err := r.Authorize(ctx, input.OrganizationID, probo.ActionRiskCreate); err != nil {
|
||||
return nil, types.AddRiskOutput{}, err
|
||||
}
|
||||
|
||||
svc := r.ProboService(ctx, input.OrganizationID)
|
||||
|
||||
risk, err := svc.Risks.Create(ctx, probo.CreateRiskRequest{
|
||||
|
||||
@@ -19,11 +19,13 @@ package mcp_v1
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/accessreview"
|
||||
"go.probo.inc/probo/pkg/cookiebanner"
|
||||
"go.probo.inc/probo/pkg/coredata"
|
||||
"go.probo.inc/probo/pkg/gid"
|
||||
"go.probo.inc/probo/pkg/iam"
|
||||
"go.probo.inc/probo/pkg/probo"
|
||||
@@ -55,7 +57,7 @@ func markdownToProseMirrorJSON(markdown string) (string, error) {
|
||||
return string(out), nil
|
||||
}
|
||||
|
||||
func (r *Resolver) MustAuthorize(ctx context.Context, entityID gid.GID, action iam.Action) {
|
||||
func (r *Resolver) Authorize(ctx context.Context, entityID gid.GID, action iam.Action) error {
|
||||
identity := authn.IdentityFromContext(ctx)
|
||||
|
||||
err := r.iamSvc.Authorizer.Authorize(
|
||||
@@ -66,7 +68,23 @@ func (r *Resolver) MustAuthorize(ctx context.Context, entityID gid.GID, action i
|
||||
Action: action,
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
if _, ok := errors.AsType[*iam.ErrInsufficientPermissions](err); ok {
|
||||
return fmt.Errorf("permission denied")
|
||||
}
|
||||
|
||||
if _, ok := errors.AsType[*iam.ErrAssumptionRequired](err); ok {
|
||||
return fmt.Errorf("assumption required")
|
||||
}
|
||||
|
||||
if errors.Is(err, coredata.ErrResourceNotFound) {
|
||||
return fmt.Errorf("resource not found")
|
||||
}
|
||||
|
||||
r.logger.ErrorCtx(ctx, "cannot authorize MCP request", log.Error(err))
|
||||
|
||||
return fmt.Errorf("internal server error")
|
||||
}
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user