Skip to content

Commit

Permalink
[A2-835] make custom endpoints projects aware for iam v2.1 (#380)
Browse files Browse the repository at this point in the history
When we've introduced projects getting passed along from authz to the domain services, we've done that for the general case (read: GRPC). This PR extends that treatment to the special case, our hand-written HTTP endpoints defined in automate-gateway.
Caveats

⚠️ This is in the code path of data ingestion, so let's keep an eye on performance after merging this.

* automate-gateway/authorizer: rough in projects
* automate-gateway: finish wiring up the authorizer refactor
* automate-gateway: refactor projects-from-metadata logic into auth_context method
* auth_context: fix ContextWithoutProjects helper

There's no need to translate incoming MD to outgoing MD. Outgoing
metadata is only required if another grpc CLIENT call uses that context,
and the key-val pairs are supposed to be sent along.

Also fixes the comment.

Signed-off-by: Stephan Renatus <[email protected]>
  • Loading branch information
srenatus authored May 28, 2019
1 parent 99e3425 commit 21e2d9c
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 53 deletions.
3 changes: 2 additions & 1 deletion components/automate-gateway/gateway/datacollector.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ func (s *Server) dataCollectorHandler(w http.ResponseWriter, r *http.Request) {
actionV2 = "infra:ingest:create"
)

if err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2); err != nil {
ctx, err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2)
if err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"strings"

"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
"github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/sirupsen/logrus"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -42,7 +41,7 @@ type SwitchingAuthorizationHandler interface {
GRPCAuthorizationHandler
SwitchingFilterHandler
IsAuthorized(ctx context.Context, subjects []string,
resourceV1, actionV1, resourceV2, actionV2 string) (AnnotatedAuthorizationResponse, error)
resourceV1, actionV1, resourceV2, actionV2 string, projects []string) (AnnotatedAuthorizationResponse, error)
}

type SwitchingFilterHandler interface {
Expand Down Expand Up @@ -71,7 +70,7 @@ type GRPCAuthorizationHandler interface {
}

type HTTPAuthorizationHandler interface {
IsAuthorized(ctx context.Context, subjects []string, resource, action string) (AuthorizationResponse, error)
IsAuthorized(ctx context.Context, subjects []string, resource, action string, projects []string) (AuthorizationResponse, error)
}

type IntrospectionHandler interface {
Expand All @@ -85,10 +84,12 @@ type AuthorizationHandler interface {
}

type AuthorizationResponse interface {
Ctx() context.Context
GetAuthorized() bool
}

type AnnotatedAuthorizationResponse interface {
AuthorizationResponse
Err() error
}

Expand Down Expand Up @@ -143,8 +144,7 @@ func (a *authInterceptor) UnaryServerInterceptor() grpc.UnaryServerInterceptor {
subs = append(authResponse.Teams, authResponse.Subject)
}

projectHeaderEntries := md.Get(runtime.MetadataPrefix + "projects")
projects := getProjectsFromMetadata(projectHeaderEntries)
projects := auth_context.ProjectsFromMetadata(md)

ctx, err = a.authz.Handle(authCtx, subs, projects, req)
if err != nil {
Expand Down Expand Up @@ -177,7 +177,7 @@ func getProjectsFromMetadata(projectHeaderEntries []string) []string {
for _, entry := range projectHeaderEntries {
for _, project := range strings.Split(entry, ",") {
newProject := strings.TrimSpace(project)
if _, value := keys[newProject]; !value {
if !keys[newProject] {
keys[newProject] = true
projects = append(projects, newProject)
}
Expand Down
13 changes: 12 additions & 1 deletion components/automate-gateway/gateway/middleware/authv1/authv1.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,24 @@ func (c *client) Handle(ctx context.Context, subjects []string, _ []string,
return auth_context.NewContext(ctx, subjects, projects, resource, action, middleware.AuthV1.String()), nil
}

type resp struct {
ctx context.Context
*authz.IsAuthorizedResp
}

func (r *resp) Ctx() context.Context {
return r.ctx
}

func (c *client) IsAuthorized(ctx context.Context, subjects []string, resource, action string,
_ []string, // projects aren't used
) (middleware.AuthorizationResponse, error) {
return c.client.IsAuthorized(ctx, &authz.IsAuthorizedReq{
r, err := c.client.IsAuthorized(ctx, &authz.IsAuthorizedReq{
Subjects: subjects,
Resource: resource,
Action: action,
})
return &resp{IsAuthorizedResp: r, ctx: ctx}, err
}

func (c *client) FilterAuthorizedPairs(ctx context.Context, subjects []string, inputPairs []*pairs.Pair,
Expand Down
41 changes: 36 additions & 5 deletions components/automate-gateway/gateway/middleware/authv2/authv2.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,44 @@ func (c *client) Handle(ctx context.Context, subjects []string, projectsToFilter
return auth_context.NewContext(ctx, subjects, projects, resource, action, middleware.AuthV2.String()), nil
}

type resp struct {
ctx context.Context
authorized bool
}


func (r *resp) Ctx() context.Context {
return r.ctx
}

func (r *resp) GetAuthorized() bool {
return r.authorized
}

func (c *client) IsAuthorized(ctx context.Context, subjects []string, resource, action string,
) (middleware.AuthorizationResponse, error) {
return c.client.IsAuthorized(ctx, &authz.IsAuthorizedReq{
Subjects: subjects,
Resource: resource,
Action: action,
projectsToFilter []string) (middleware.AuthorizationResponse, error) {
log := ctxlogrus.Extract(ctx)
filteredResp, err := c.client.ProjectsAuthorized(ctx, &authz.ProjectsAuthorizedReq{
Subjects: subjects,
Resource: resource,
Action: action,
ProjectsFilter: projectsToFilter,
})
if err != nil {
if status.Convert(err).Code() == codes.FailedPrecondition {
return nil, err
}
log.WithError(err).Error("error authorizing request")
return nil, status.Errorf(codes.PermissionDenied,
"error authorizing action %q on resource %q for subjects %q: %s",
action, resource, subjects, err.Error())
}
projects := filteredResp.Projects

return &resp{
ctx: auth_context.NewContext(ctx, subjects, projects, resource, action, middleware.AuthV2.String()),
authorized: len(projects) != 0,
}, nil
}

func (c *client) FilterAuthorizedPairs(ctx context.Context, subjects []string, inputPairs []*pairs.Pair,
Expand Down
46 changes: 22 additions & 24 deletions components/automate-gateway/gateway/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (

// anything else
"github.com/chef/automate/components/automate-gateway/gateway/middleware"
"github.com/chef/automate/lib/grpc/auth_context"
"github.com/chef/automate/lib/grpc/service_authn"
)

Expand Down Expand Up @@ -372,7 +373,8 @@ func (s *Server) ProfileCreateHandler(w http.ResponseWriter, r *http.Request) {
)
resourceV1 := fmt.Sprintf("compliance:profiles:storage:%s", owner)
resourceV2 := fmt.Sprintf("compliance:profiles:%s", owner)
if err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2); err != nil {
ctx, err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2)
if err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
Expand Down Expand Up @@ -460,7 +462,8 @@ func (s *Server) ProfileTarHandler(w http.ResponseWriter, r *http.Request) {
resourceV2 = "compliance:profiles:market"
}

if err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2); err != nil {
ctx, err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2)
if err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
Expand Down Expand Up @@ -506,15 +509,15 @@ func (s *Server) ReportExportHandler(w http.ResponseWriter, r *http.Request) {
actionV2 = "compliance:reports:export"
)

if err := s.authRequest(r, resource, actionV1, resource, actionV2); err != nil {
ctx, err := s.authRequest(r, resource, actionV1, resource, actionV2)
if err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}

decoder := json.NewDecoder(r.Body)
var query reporting.Query
err := decoder.Decode(&query)
if err != nil {
if err := decoder.Decode(&query); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -554,15 +557,15 @@ func (s *Server) configMgmtNodeExportHandler(w http.ResponseWriter, r *http.Requ
resourceV2 = "infra:nodes"
)

if err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2); err != nil {
ctx, err := s.authRequest(r, resourceV1, actionV1, resourceV2, actionV2)
if err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}

decoder := json.NewDecoder(r.Body)
var nodeExportRequest cfgmgmt_request.NodeExport
err := decoder.Decode(&nodeExportRequest)
if err != nil {
if err := decoder.Decode(&nodeExportRequest); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -606,7 +609,7 @@ func init() {

func (s *Server) authRequest(r *http.Request,
resourceV1, actionV1, resourceV2, actionV2 string,
) error {
) (context.Context, error) {
subjects := []string{}
// Create a context with the request headers metadata. Normally grpc-gateway
// does this, but since this is being used in a custom handler we've got do
Expand Down Expand Up @@ -638,34 +641,29 @@ func (s *Server) authRequest(r *http.Request,
if len(subjects) < 1 {
authnClient, err := s.clientsFactory.AuthenticationClient()
if err != nil {
return errors.Wrap(err, "authn-service unavailable")
return nil, errors.Wrap(err, "authn-service unavailable")
}

authnResp, err := authnClient.Authenticate(ctx, &authn.AuthenticateRequest{})
if err != nil {
return errors.Wrap(err, "authn-service error")
return nil, errors.Wrap(err, "authn-service error")
}

subjects = append(authnResp.Teams, authnResp.Subject)
}

if len(subjects) < 1 {
return errors.New("no policy subject detected in headers or verified certificates")
return nil, errors.New("no policy subject detected in headers or verified certificates")
}

authzResp, err := s.authorizer.IsAuthorized(ctx, subjects, resourceV1, actionV1, resourceV2, actionV2)
projects := auth_context.ProjectsFromMetadata(md)

authzResp, err := s.authorizer.IsAuthorized(ctx, subjects, resourceV1, actionV1, resourceV2, actionV2, projects)
if err != nil {
return errors.Wrap(err, "authz-service error")
return nil, errors.Wrap(err, "authz-service error")
}

// returns nil if authorization succeeded
return authzResp.Err()
}

func logError(err error, resource, action string) {
log.WithFields(log.Fields{
"error": err,
"resource": resource,
"action": action,
}).Error("error authorizing request")
// err is nil if authorization succeeded
// Note: if we need all the auth info, use auth_context.NewOutgoingContext
return auth_context.NewOutgoingProjectsContext(authzResp.Ctx()), authzResp.Err()
}
17 changes: 9 additions & 8 deletions components/automate-gateway/pkg/authorizer/authorizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,20 @@ func (a *state) Handle(ctx context.Context,
}

func (a *state) IsAuthorized(ctx context.Context, subjects []string,
resourceV1, actionV1, resourceV2, actionV2 string,
resourceV1, actionV1, resourceV2, actionV2 string, projects []string,
) (middleware.AnnotatedAuthorizationResponse, error) {
var (
resp middleware.AuthorizationResponse
err error
)
switch a.next {
case a.v1:
resp, err = a.v1.IsAuthorized(ctx, subjects, resourceV1, actionV1)
resp, err = a.v1.IsAuthorized(ctx, subjects, resourceV1, actionV1, nil) // projects are not used here
if err == nil {
return annotate(resp, subjects, resourceV1, actionV1), nil
}
case a.v2:
resp, err = a.v2.IsAuthorized(ctx, subjects, resourceV2, actionV2)
resp, err = a.v2.IsAuthorized(ctx, subjects, resourceV2, actionV2, projects)
if err == nil {
return annotate(resp, subjects, resourceV2, actionV2), nil
}
Expand All @@ -61,7 +61,7 @@ func (a *state) IsAuthorized(ctx context.Context, subjects []string,
switch st.Code() {
case codes.FailedPrecondition:
if a.fromStatus(st) {
return a.IsAuthorized(ctx, subjects, resourceV1, actionV1, resourceV2, actionV2)
return a.IsAuthorized(ctx, subjects, resourceV1, actionV1, resourceV2, actionV2, projects)
}
fallthrough
default: // any other error status
Expand Down Expand Up @@ -114,20 +114,21 @@ func (a *state) FilterAuthorizedPairs(ctx context.Context, subjects []string,
}

type annotated struct {
r middleware.AuthorizationResponse
middleware.AuthorizationResponse
err error
}

func (r *annotated) Err() error {
if r.r.GetAuthorized() {
if r.GetAuthorized() {
return nil
}
return r.err
}

func annotate(resp middleware.AuthorizationResponse, subjects []string, resource, action string) middleware.AnnotatedAuthorizationResponse {
return &annotated{r: resp, err: fmt.Errorf("subject %q is not authorized to %q resource %q",
subjects, action, resource)}
return &annotated{AuthorizationResponse: resp,
err: fmt.Errorf("subject %q is not authorized to %q resource %q",
subjects, action, resource)}
}

func (a *state) fromStatus(st *status.Status) bool {
Expand Down
17 changes: 13 additions & 4 deletions components/automate-gateway/pkg/authorizer/authorizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ func TestAuthorizerHandle(t *testing.T) {

func TestAuthorizerIsAuthorized(t *testing.T) {
ctx := context.Background()
args := func() (context.Context, []string, string, string, string, string) {
return ctx, []string{"user:local:admin"}, "resource:v1", "v1action", "svc:type:v2resource", "svc:type:v2action"
args := func() (context.Context, []string, string, string, string, string, []string) {
return ctx, []string{"user:local:admin"}, "resource:v1", "v1action", "svc:type:v2resource", "svc:type:v2action",
[]string{"foo-project", "bar-project"}
}

t.Run("if first succeeds, doesn't call second", func(t *testing.T) {
Expand Down Expand Up @@ -127,7 +128,7 @@ func (*success) Handle(ctx context.Context, _ []string, _ []string, _ interface{
return ctx, nil
}

func (s *success) IsAuthorized(context.Context, []string, string, string) (middleware.AuthorizationResponse, error) {
func (s *success) IsAuthorized(context.Context, []string, string, string, []string) (middleware.AuthorizationResponse, error) {
return authzSuccess, nil
}

Expand All @@ -143,7 +144,7 @@ func (f *failure) Handle(ctx context.Context, _ []string, _ []string, _ interfac
return ctx, f.Err()
}

func (f *failure) IsAuthorized(context.Context, []string, string, string) (middleware.AuthorizationResponse, error) {
func (f *failure) IsAuthorized(context.Context, []string, string, string, []string) (middleware.AuthorizationResponse, error) {
return nil, f.Err()
}

Expand All @@ -165,6 +166,14 @@ func (*authorized) GetAuthorized() bool {
return true
}

func (*authorized) Ctx() context.Context {
return nil
}

func (*notAuthorized) GetAuthorized() bool {
return false
}

func (*notAuthorized) Ctx() context.Context {
return nil
}
Loading

0 comments on commit 21e2d9c

Please sign in to comment.