Skip to content

Commit

Permalink
Revert "Only allow access request deletion through static roles' perm…
Browse files Browse the repository at this point in the history
…issions (#9540)" (#11221)

This reverts commit 8db6aa5.
  • Loading branch information
espadolini authored Mar 17, 2022
1 parent ae91595 commit 53f6e90
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 116 deletions.
29 changes: 1 addition & 28 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,6 @@ func (a *ServerWithRoles) withOptions(opts ...actionOption) actionConfig {
return cfg
}

func (a *ServerWithRoles) withStaticRoles() (actionConfig, error) {
user, err := a.authServer.GetUser(a.context.User.GetName(), false)
if err != nil {
return actionConfig{}, trace.Wrap(err)
}

checker, err := services.FetchRoles(user.GetRoles(), a.authServer, user.GetTraits())
if err != nil {
return actionConfig{}, trace.Wrap(err)
}

return actionConfig{context: Context{
User: user,
Checker: checker,
}}, nil
}

func (c actionConfig) action(namespace, resource string, verbs ...string) error {
if len(verbs) == 0 {
return trace.BadParameter("no verbs provided for authorization check on resource %q", resource)
Expand Down Expand Up @@ -1665,17 +1648,7 @@ func (a *ServerWithRoles) getProxyPublicAddr() string {
}

func (a *ServerWithRoles) DeleteAccessRequest(ctx context.Context, name string) error {
cfg, err := a.withStaticRoles()
if err != nil {
return err
}
if err := cfg.action(apidefaults.Namespace, types.KindAccessRequest, types.VerbDelete); err != nil {
if trace.IsAccessDenied(err) {
if a.withOptions(quietAction(true)).action(apidefaults.Namespace, types.KindAccessRequest, types.VerbDelete) == nil {
// the user would've had permission with the roles granted by access requests
return trace.WrapWithMessage(err, "access request deletion through elevated roles is not allowed")
}
}
if err := a.action(apidefaults.Namespace, types.KindAccessRequest, types.VerbDelete); err != nil {
return trace.Wrap(err)
}
return a.authServer.DeleteAccessRequest(ctx, name)
Expand Down
88 changes: 0 additions & 88 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1910,94 +1910,6 @@ func TestKindClusterConfig(t *testing.T) {
})
}

func TestNoElevatedAccessRequestDeletion(t *testing.T) {
t.Parallel()
ctx := context.Background()

srv, err := NewTestAuthServer(TestAuthServerConfig{Dir: t.TempDir()})
require.NoError(t, err)
t.Cleanup(func() { srv.Close() })

deleterRole, err := types.NewRole("deleter", types.RoleSpecV5{
Allow: types.RoleConditions{
Rules: []types.Rule{{
Resources: []string{"access_request"},
Verbs: []string{"delete"},
}},
},
})
require.NoError(t, err)
deleterUser, err := CreateUser(srv.AuthServer, "deletey", deleterRole)
require.NoError(t, err)

requesterRole, err := types.NewRole("requester", types.RoleSpecV5{
Allow: types.RoleConditions{
Request: &types.AccessRequestConditions{
Roles: []string{deleterRole.GetName()},
},
},
})
require.NoError(t, err)
requesterUser, err := CreateUser(srv.AuthServer, "requesty", requesterRole)
require.NoError(t, err)

request, err := services.NewAccessRequest(requesterUser.GetName(), deleterRole.GetName())
require.NoError(t, err)
// the request must be for an allowed user/role combination or it will get rejected
err = srv.AuthServer.CreateAccessRequest(ctx, request)
require.NoError(t, err)

// requesty has used some other unspecified access request to get the
// deleter role in this identity
requesterAuthContext, err := srv.Authorizer.Authorize(context.WithValue(ctx,
ContextUser,
LocalUser{
Username: requesterUser.GetName(),
Identity: tlsca.Identity{
Username: requesterUser.GetName(),
Groups: []string{requesterRole.GetName(), deleterRole.GetName()},
// a tlsca.Identity must have a nonempty Traits field or the
// roles will be reloaded from the backend during Authorize
Traits: map[string][]string{"nonempty": {}},
},
},
))
require.NoError(t, err)
requesterAuth := &ServerWithRoles{
authServer: srv.AuthServer,
sessions: srv.SessionServer,
alog: srv.AuditLog,
context: *requesterAuthContext,
}

err = requesterAuth.DeleteAccessRequest(ctx, request.GetName())
require.True(t, trace.IsAccessDenied(err))
// matches the message in lib/auth/auth_with_roles.go:(*ServerWithRoles).DeleteAccessRequest()
require.Contains(t, err.Error(), "deletion through elevated roles")

deleterAuthContext, err := srv.Authorizer.Authorize(context.WithValue(ctx,
ContextUser,
LocalUser{
Username: deleterUser.GetName(),
Identity: tlsca.Identity{
Username: deleterUser.GetName(),
Groups: []string{deleterRole.GetName()},
Traits: map[string][]string{"nonempty": {}},
},
},
))
require.NoError(t, err)
deleterAuth := &ServerWithRoles{
authServer: srv.AuthServer,
sessions: srv.SessionServer,
alog: srv.AuditLog,
context: *deleterAuthContext,
}

err = deleterAuth.DeleteAccessRequest(ctx, request.GetName())
require.NoError(t, err)
}

func TestGetAndList_KubeServices(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down

0 comments on commit 53f6e90

Please sign in to comment.