Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v14] Allow access requests to use user login state. #33350

Merged
merged 1 commit into from Oct 12, 2023
Merged

[v14] Allow access requests to use user login state. #33350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 1 addition & 11 deletions lib/auth/auth.go
View file
Open in desktop
Expand Up @@ -1746,17 +1746,7 @@ func certRequestDeviceExtensions(ext tlsca.DeviceExtensions) certRequestOption {

// GetUserOrLoginState will return the given user or the login state associated with the user.
func (a *Server) GetUserOrLoginState(ctx context.Context, username string) (services.UserState, error) {
uls, err := a.GetUserLoginState(ctx, username)
if err != nil && !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
}

if err == nil {
return uls, nil
}

user, err := a.GetUser(username, false)
return user, trace.Wrap(err)
return services.GetUserOrLoginState(ctx, a, username)
}

func (a *Server) GenerateOpenSSHCert(ctx context.Context, req *proto.OpenSSHCertRequest) (*proto.OpenSSHCert, error) {
Expand Down
57 changes: 29 additions & 28 deletions lib/services/access_request.go
View file
Open in desktop
Expand Up @@ -243,7 +243,7 @@ func (m *RequestValidator) applicableSearchAsRoles(ctx context.Context, resource
rolesToRequest = append(rolesToRequest, roleName)
}
if len(rolesToRequest) == 0 {
return nil, trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user %q`, m.user.GetName())
return nil, trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user %q`, m.userState.GetName())
}

// Prune the list of roles to request to only those which may be necessary
Expand Down Expand Up @@ -377,7 +377,7 @@ func ValidateAccessPredicates(role types.Role) error {
}

// ApplyAccessReview attempts to apply the specified access review to the specified request.
func ApplyAccessReview(req types.AccessRequest, rev types.AccessReview, author types.User) error {
func ApplyAccessReview(req types.AccessRequest, rev types.AccessReview, author UserState) error {
if rev.Author != author.GetName() {
return trace.BadParameter("mismatched review author (expected %q, got %q)", rev.Author, author)
}
Expand Down Expand Up @@ -489,7 +489,7 @@ func checkReviewCompat(req types.AccessRequest, rev types.AccessReview) error {

// collectReviewThresholdIndexes aggregates the indexes of all thresholds whose filters match
// the supplied review (part of review application logic).
func collectReviewThresholdIndexes(req types.AccessRequest, rev types.AccessReview, author types.User) ([]uint32, error) {
func collectReviewThresholdIndexes(req types.AccessRequest, rev types.AccessReview, author UserState) ([]uint32, error) {
parser, err := newThresholdFilterParser(req, rev, author)
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -538,7 +538,7 @@ func accessReviewThresholdMatchesFilter(t types.AccessReviewThreshold, parser pr

// newThresholdFilterParser creates a custom parser context which exposes a simplified view of the review author
// and the request for evaluation of review threshold filters.
func newThresholdFilterParser(req types.AccessRequest, rev types.AccessReview, author types.User) (BoolPredicateParser, error) {
func newThresholdFilterParser(req types.AccessRequest, rev types.AccessReview, author UserState) (BoolPredicateParser, error) {
return NewJSONBoolParser(thresholdFilterContext{
Reviewer: reviewAuthorContext{
Roles: author.GetRoles(),
Expand Down Expand Up @@ -727,6 +727,7 @@ type ResourceLister interface {
// RequestValidatorGetter is the interface required by the request validation
// functions used to get necessary resources.
type RequestValidatorGetter interface {
UserLoginStatesGetter
UserGetter
RoleGetter
ResourceLister
Expand Down Expand Up @@ -783,8 +784,8 @@ func insertAnnotations(annotations map[string][]string, conditions types.AccessR
// ReviewPermissionChecker is a helper for validating whether a user
// is allowed to review specific access requests.
type ReviewPermissionChecker struct {
User types.User
Roles struct {
UserState UserState
Roles struct {
// allow/deny mappings sort role matches into lists based on their
// constraining predicate (where) expression.
AllowReview, DenyReview map[string][]parse.Matcher
Expand All @@ -811,7 +812,7 @@ func (c *ReviewPermissionChecker) CanReviewRequest(req types.AccessRequest) (boo
// adding role subselection support.

// user cannot review their own request
if c.User.GetName() == req.GetUser() {
if c.UserState.GetName() == req.GetUser() {
return false, nil
}

Expand All @@ -821,8 +822,8 @@ func (c *ReviewPermissionChecker) CanReviewRequest(req types.AccessRequest) (boo

parser, err := NewJSONBoolParser(reviewPermissionContext{
Reviewer: reviewAuthorContext{
Roles: c.User.GetRoles(),
Traits: c.User.GetTraits(),
Roles: c.UserState.GetRoles(),
Traits: c.UserState.GetTraits(),
},
Request: reviewRequestContext{
Roles: requestedRoles,
Expand Down Expand Up @@ -907,21 +908,21 @@ Outer:
}

func NewReviewPermissionChecker(ctx context.Context, getter RequestValidatorGetter, username string) (ReviewPermissionChecker, error) {
user, err := getter.GetUser(username, false)
uls, err := GetUserOrLoginState(ctx, getter, username)
if err != nil {
return ReviewPermissionChecker{}, trace.Wrap(err)
}

c := ReviewPermissionChecker{
User: user,
UserState: uls,
}

c.Roles.AllowReview = make(map[string][]parse.Matcher)
c.Roles.DenyReview = make(map[string][]parse.Matcher)

// load all statically assigned roles for the user and
// use them to build our checker state.
for _, roleName := range c.User.GetRoles() {
for _, roleName := range c.UserState.GetRoles() {
role, err := getter.GetRole(ctx, roleName)
if err != nil {
return ReviewPermissionChecker{}, trace.Wrap(err)
Expand All @@ -939,12 +940,12 @@ func (c *ReviewPermissionChecker) push(role types.Role) error {

var err error

c.Roles.DenyReview[deny.Where], err = appendRoleMatchers(c.Roles.DenyReview[deny.Where], deny.Roles, deny.ClaimsToRoles, c.User.GetTraits())
c.Roles.DenyReview[deny.Where], err = appendRoleMatchers(c.Roles.DenyReview[deny.Where], deny.Roles, deny.ClaimsToRoles, c.UserState.GetTraits())
if err != nil {
return trace.Wrap(err)
}

c.Roles.AllowReview[allow.Where], err = appendRoleMatchers(c.Roles.AllowReview[allow.Where], allow.Roles, allow.ClaimsToRoles, c.User.GetTraits())
c.Roles.AllowReview[allow.Where], err = appendRoleMatchers(c.Roles.AllowReview[allow.Where], allow.Roles, allow.ClaimsToRoles, c.UserState.GetTraits())
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -961,7 +962,7 @@ func (c *ReviewPermissionChecker) push(role types.Role) error {
type RequestValidator struct {
clock clockwork.Clock
getter RequestValidatorGetter
user types.User
userState UserState
requireReason bool
autoRequest bool
prompt string
Expand All @@ -988,15 +989,15 @@ type RequestValidator struct {

// NewRequestValidator configures a new RequestValidator for the specified user.
func NewRequestValidator(ctx context.Context, clock clockwork.Clock, getter RequestValidatorGetter, username string, opts ...ValidateRequestOption) (RequestValidator, error) {
user, err := getter.GetUser(username, false)
uls, err := GetUserOrLoginState(ctx, getter, username)
if err != nil {
return RequestValidator{}, trace.Wrap(err)
}

m := RequestValidator{
clock: clock,
getter: getter,
user: user,
clock: clock,
getter: getter,
userState: uls,
}
for _, opt := range opts {
opt(&m)
Expand All @@ -1011,7 +1012,7 @@ func NewRequestValidator(ctx context.Context, clock clockwork.Clock, getter Requ

// load all statically assigned roles for the user and
// use them to build our validation state.
for _, roleName := range m.user.GetRoles() {
for _, roleName := range m.userState.GetRoles() {
role, err := m.getter.GetRole(ctx, roleName)
if err != nil {
return RequestValidator{}, trace.Wrap(err)
Expand All @@ -1026,7 +1027,7 @@ func NewRequestValidator(ctx context.Context, clock clockwork.Clock, getter Requ
// Validate validates an access request and potentially modifies it depending on how
// the validator was configured.
func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest, identity tlsca.Identity) error {
if m.user.GetName() != req.GetUser() {
if m.userState.GetName() != req.GetUser() {
return trace.BadParameter("request validator configured for different user (this is a bug)")
}

Expand Down Expand Up @@ -1311,7 +1312,7 @@ func (m *RequestValidator) GetRequestableRoles() ([]string, error) {

var expanded []string
for _, role := range allRoles {
if n := role.GetName(); !slices.Contains(m.user.GetRoles(), n) && m.CanRequestRole(n) {
if n := role.GetName(); !slices.Contains(m.userState.GetRoles(), n) && m.CanRequestRole(n) {
// user does not currently hold this role, and is allowed to request it.
expanded = append(expanded, n)
}
Expand All @@ -1333,7 +1334,7 @@ func (m *RequestValidator) push(role types.Role) error {

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

m.Roles.DenyRequest, err = appendRoleMatchers(m.Roles.DenyRequest, deny.Roles, deny.ClaimsToRoles, m.user.GetTraits())
m.Roles.DenyRequest, err = appendRoleMatchers(m.Roles.DenyRequest, deny.Roles, deny.ClaimsToRoles, m.userState.GetTraits())
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -1342,7 +1343,7 @@ func (m *RequestValidator) push(role types.Role) error {
// matchers for this role, if it applies any.
astart := len(m.Roles.AllowRequest)

m.Roles.AllowRequest, err = appendRoleMatchers(m.Roles.AllowRequest, allow.Roles, allow.ClaimsToRoles, m.user.GetTraits())
m.Roles.AllowRequest, err = appendRoleMatchers(m.Roles.AllowRequest, allow.Roles, allow.ClaimsToRoles, m.userState.GetTraits())
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -1381,8 +1382,8 @@ func (m *RequestValidator) push(role types.Role) error {
// validation process for incoming access requests requires
// generating system annotations to be attached to the request
// before it is inserted into the backend.
insertAnnotations(m.Annotations.Deny, deny, m.user.GetTraits())
insertAnnotations(m.Annotations.Allow, allow, m.user.GetTraits())
insertAnnotations(m.Annotations.Deny, deny, m.userState.GetTraits())
insertAnnotations(m.Annotations.Allow, allow, m.userState.GetTraits())

m.SuggestedReviewers = append(m.SuggestedReviewers, allow.SuggestedReviewers...)
}
Expand Down Expand Up @@ -1680,7 +1681,7 @@ func (m *RequestValidator) pruneResourceRequestRoles(
}
}

allRoles, err := FetchRoles(roles, m.getter, m.user.GetTraits())
allRoles, err := FetchRoles(roles, m.getter, m.userState.GetTraits())
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -1797,7 +1798,7 @@ func (m *RequestValidator) roleAllowsResource(
matchers = append(matchers, NewLoginMatcher(loginHint))
}
matchers = append(matchers, extraMatchers...)
err := roleSet.checkAccess(resource, m.user.GetTraits(), AccessState{MFAVerified: true}, matchers...)
err := roleSet.checkAccess(resource, m.userState.GetTraits(), AccessState{MFAVerified: true}, matchers...)
if trace.IsAccessDenied(err) {
// Access denied, this role does not allow access to this resource, no
// unexpected error to report.
Expand Down