Skip to content

Commit

Permalink
Add support for requiring reason for access requests
Browse files Browse the repository at this point in the history
  • Loading branch information
kopiczko committed Nov 18, 2024
1 parent 6d1cb64 commit adeaa39
Show file tree
Hide file tree
Showing 7 changed files with 2,836 additions and 2,295 deletions.
12 changes: 12 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3440,6 +3440,18 @@ message AccessRequestConditions {
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "kubernetes_resources,omitempty"
];

// Reason defines settings for the reason for the access provided by the user.
AccessRequestConditionsReason Reason = 9 [(gogoproto.jsontag) = "reason,omitempty"];
}

// AccessRequestConditionsReason defines settings for the reason for the access provided by the
// user.
message AccessRequestConditionsReason {
// Required indicates that reason is required for all access requests requesting allowed roles or
// resources searchable with search_as_roles. It applies only to users who have this role
// assigned.
bool Required = 1 [(gogoproto.jsontag) = "required,omitempty"];
}

// AccessReviewConditions is a matcher for allow/deny restrictions on
Expand Down
4,757 changes: 2,491 additions & 2,266 deletions api/types/types.pb.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion e
Submodule e updated from 576535 to e8f142
92 changes: 74 additions & 18 deletions lib/services/access_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,14 @@ func CalculateAccessCapabilities(ctx context.Context, clock clockwork.Clock, clt
caps.SuggestedReviewers = v.SuggestedReviewers
}

caps.RequireReason = v.requireReason
// If empty-string reason is valid, then reason is not required.
err = v.validateReason(ctx, "", caps.RequestableRoles, req.ResourceIDs)
if trace.IsBadParameter(err) {
caps.RequireReason = true
} else if err != nil {
return nil, trace.Wrap(err)
}

caps.RequestPrompt = v.prompt
caps.AutoRequest = v.autoRequest

Expand All @@ -302,6 +309,8 @@ func (m *RequestValidator) allowedSearchAsRoles() ([]string, error) {

// applicableSearchAsRoles prunes the search_as_roles and only returns those
// applicable for the given list of resourceIDs.
//
// If loginHint is provided, it will attempt to prune the list to a single role.
func (m *RequestValidator) applicableSearchAsRoles(ctx context.Context, resourceIDs []types.ResourceID, loginHint string) ([]string, error) {
rolesToRequest, err := m.allowedSearchAsRoles()
if err != nil {
Expand Down Expand Up @@ -1041,10 +1050,11 @@ func (c *ReviewPermissionChecker) push(role types.Role) error {
// a set of simple Allow/Deny datastructures. These, in turn,
// are used to validate and expand the access request.
type RequestValidator struct {
clock clockwork.Clock
getter RequestValidatorGetter
userState UserState
requireReason bool
clock clockwork.Clock
getter RequestValidatorGetter
userState UserState
requireReasonForAllRoles bool
requiringReasonRoles map[string]struct{}
// Used to enforce that the configuration found in the static
// role that defined the search_as_role, is respected.
// An empty map or list means nothing was configured.
Expand Down Expand Up @@ -1097,6 +1107,8 @@ func NewRequestValidator(ctx context.Context, clock clockwork.Clock, getter Requ
getter: getter,
userState: uls,
logger: slog.With(teleport.ComponentKey, "request.validator"),

requiringReasonRoles: make(map[string]struct{}),
}
for _, opt := range opts {
opt(&m)
Expand All @@ -1118,7 +1130,7 @@ func NewRequestValidator(ctx context.Context, clock clockwork.Clock, getter Requ
if err != nil {
return RequestValidator{}, trace.Wrap(err)
}
if err := m.push(ctx, role); err != nil {
if err := m.push(ctx, uls, role); err != nil {
return RequestValidator{}, trace.Wrap(err)
}
}
Expand All @@ -1132,8 +1144,9 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest
return trace.BadParameter("request validator configured for different user (this is a bug)")
}

if m.requireReason && req.GetRequestReason() == "" {
return trace.BadParameter("request reason must be specified (required by static role configuration)")
err := m.validateReason(ctx, req.GetRequestReason(), req.GetRoles(), req.GetRequestedResourceIDs())
if err != nil {
return trace.Wrap(err)
}

if !req.GetState().IsPromoted() && req.GetPromotedAccessListTitle() != "" {
Expand Down Expand Up @@ -1313,6 +1326,41 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest
return nil
}

// validateReason checks if the reason is valid and returns BadParameterError if it is not. It may
// return any other error.
func (m *RequestValidator) validateReason(ctx context.Context, reason string, requestedRoles []string, requestedResourceIDs []types.ResourceID) error {
// Non-empty reason is always valid.
if len(reason) > 0 {
return nil
}

if m.requireReasonForAllRoles {
return trace.BadParameter("request reason must be specified (required request_access option in one of the roles)")
}

allApplicableRoles := requestedRoles
if len(requestedResourceIDs) > 0 {
// Do not provide loginHint. We want all matching search_as_roles for those resources.
roles, err := m.applicableSearchAsRoles(ctx, requestedResourceIDs, "")
if err != nil {
return trace.Wrap(err)
}
if len(allApplicableRoles) == 0 {
allApplicableRoles = roles
} else {
allApplicableRoles = append(allApplicableRoles, roles...)
}
}

for _, r := range allApplicableRoles {
if _, ok := m.requiringReasonRoles[r]; ok {
return trace.BadParameter("request reason must be specified (required for role %q)", r)
}
}

return nil
}

// calculateMaxAccessDuration calculates the maximum time for the access request.
// The max duration time is the minimum of the max_duration time set on the request
// and the max_duration time set on the request role.
Expand Down Expand Up @@ -1518,7 +1566,7 @@ func (m *RequestValidator) GetRequestableRoles(ctx context.Context, identity tls

roleAllowsAccess := true
for _, resource := range filteredResources {
access, err := m.roleAllowsResource(ctx, role, resource, loginHint)
access, err := m.roleAllowsResource(role, resource, loginHint)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -1561,17 +1609,26 @@ func setAllowRequestKubeResourceLookup(allowKubernetesResources []types.RequestK
// push compiles a role's configuration into the request validator.
// All of the requesting user's statically assigned roles must be pushed
// before validation begins.
func (m *RequestValidator) push(ctx context.Context, role types.Role) error {
func (m *RequestValidator) push(ctx context.Context, userState UserState, role types.Role) error {
var err error

m.requireReason = m.requireReason || role.GetOptions().RequestAccess.RequireReason()
m.requireReasonForAllRoles = m.requireReasonForAllRoles || role.GetOptions().RequestAccess.RequireReason()
m.autoRequest = m.autoRequest || role.GetOptions().RequestAccess.ShouldAutoRequest()
if m.prompt == "" {
m.prompt = role.GetOptions().RequestPrompt
}

allow, deny := role.GetAccessRequestConditions(types.Allow), role.GetAccessRequestConditions(types.Deny)

if allow.Reason != nil && allow.Reason.Required {
for _, r := range allow.Roles {
m.requiringReasonRoles[r] = struct{}{}
}
for _, r := range allow.SearchAsRoles {
m.requiringReasonRoles[r] = struct{}{}
}
}

setAllowRequestKubeResourceLookup(allow.KubernetesResources, allow.SearchAsRoles, m.kubernetesResource.allow)

if len(deny.KubernetesResources) > 0 {
Expand Down Expand Up @@ -1951,18 +2008,18 @@ func (m *RequestValidator) SystemAnnotations(req types.AccessRequest) (map[strin

type ValidateRequestOption func(*RequestValidator)

// ExpandVars toggles variable expansion during request validation. Variable expansion
// includes expanding wildcard requests, setting system annotations, and gathering
// threshold information. Variable expansion should be run by the auth server prior
// to storing an access request for the first time.
// ExpandVars toggles variable expansion during request validation. Variable expansion includes
// expanding wildcard requests, setting system annotations, finding applicable roles for
// resource-based requests and gathering threshold information. Variable expansion should be run
// by the auth server prior to storing an access request for the first time.
func ExpandVars(expand bool) ValidateRequestOption {
return func(v *RequestValidator) {
v.opts.expandVars = expand
}
}

// ValidateAccessRequestForUser validates an access request against the associated users's
// *statically assigned* roles. If expandRoles is true, it will also expand wildcard
// *statically assigned* roles. If [[ExpandVars]] is set to true, it will also expand wildcard
// requests, setting their role list to include all roles the user is allowed to request.
// Expansion should be performed before an access request is initially placed in the backend.
func ValidateAccessRequestForUser(ctx context.Context, clock clockwork.Clock, getter RequestValidatorGetter, req types.AccessRequest, identity tlsca.Identity, opts ...ValidateRequestOption) error {
Expand Down Expand Up @@ -2121,7 +2178,7 @@ func (m *RequestValidator) pruneResourceRequestRoles(
}

for _, role := range allRoles {
roleAllowsAccess, err := m.roleAllowsResource(ctx, role, resource, loginHint, resourceMatcherToMatcherSlice(resourceMatcher)...)
roleAllowsAccess, err := m.roleAllowsResource(role, resource, loginHint, resourceMatcherToMatcherSlice(resourceMatcher)...)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -2228,7 +2285,6 @@ func getAllowedKubeResourceKinds(allowedKinds []string, deniedKinds []string) []
}

func (m *RequestValidator) roleAllowsResource(
ctx context.Context,
role types.Role,
resource types.ResourceWithLabels,
loginHint string,
Expand Down
Loading

0 comments on commit adeaa39

Please sign in to comment.