-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
Update AccessList Proto, AccessList & AccessListMember types, add Hierarchy utils #47828
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,10 @@ message AccessListOwner { | |
// ineligible_status describes if this owner is eligible or not | ||
// and if not, describes how they're lacking eligibility. | ||
IneligibleStatus ineligible_status = 3; | ||
|
||
// membership_kind describes the type of membership, either | ||
// `MEMBERSHIP_KIND_USER` or `MEMBERSHIP_KIND_LIST`. | ||
MembershipKind membership_kind = 4; | ||
} | ||
|
||
// AccessListAudit describes the audit configuration for an Access List. | ||
|
@@ -197,6 +201,21 @@ message MemberSpec { | |
// ineligible_status describes if this member is eligible or not | ||
// and if not, describes how they're lacking eligibility. | ||
IneligibleStatus ineligible_status = 7; | ||
|
||
// membership_kind describes the type of membership, either | ||
// `MEMBERSHIP_KIND_USER` or `MEMBERSHIP_KIND_LIST`. | ||
MembershipKind membership_kind = 9; | ||
} | ||
|
||
// MembershipKind represents the different kinds of list membership | ||
enum MembershipKind { | ||
// MEMBERSHIP_KIND_UNSPECIFIED represents list members that are of | ||
// unknown membership kind, defaulting to being treated as type USER | ||
MEMBERSHIP_KIND_UNSPECIFIED = 0; | ||
// MEMBERSHIP_KIND_USER represents list members that are normal users | ||
MEMBERSHIP_KIND_USER = 1; | ||
// MEMBERSHIP_KIND_LIST represents list members that are nested Access Lists | ||
MEMBERSHIP_KIND_LIST = 2; | ||
} | ||
|
||
// IneligibleStatus describes how the user is ineligible. | ||
|
@@ -268,6 +287,12 @@ message ReviewChanges { | |
|
||
// AccessListStatus contains dynamic fields calculated during retrieval. | ||
message AccessListStatus { | ||
// member_count is the number of members in the in the Access List. | ||
// member_count is the number of members in the Access List. | ||
optional uint32 member_count = 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this the total number of users including users that have access to it through nested access lists or just direct access list memberships? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just direct memberships. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make it clear? |
||
// member_list_count is the number of nested list members in the Access List. | ||
optional uint32 member_list_count = 2; | ||
// owner_of describes Access Lists where this Access List is an explicit owner. | ||
repeated string owner_of = 3; | ||
// member_of describes Access Lists where this Access List is an explicit member. | ||
repeated string member_of = 4; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ import ( | |
"github.com/gravitational/trace" | ||
"github.com/jonboulle/clockwork" | ||
|
||
accesslistv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1" | ||
"github.com/gravitational/teleport/api/types" | ||
"github.com/gravitational/teleport/api/types/header" | ||
"github.com/gravitational/teleport/api/types/header/convert/legacy" | ||
|
@@ -75,6 +76,20 @@ func parseReviewFrequency(input string) ReviewFrequency { | |
return 0 | ||
} | ||
|
||
// MaxAllowedDepth is the maximum allowed depth for nested access lists. | ||
const MaxAllowedDepth = 10 | ||
|
||
var ( | ||
// MembershipKindUnspecified is the default membership kind (treated as 'user'). | ||
MembershipKindUnspecified = accesslistv1.MembershipKind_MEMBERSHIP_KIND_UNSPECIFIED.String() | ||
|
||
// MembershipKindUser is the user membership kind. | ||
MembershipKindUser = accesslistv1.MembershipKind_MEMBERSHIP_KIND_USER.String() | ||
|
||
// MembershipKindList is the list membership kind. | ||
MembershipKindList = accesslistv1.MembershipKind_MEMBERSHIP_KIND_LIST.String() | ||
) | ||
|
||
// ReviewDayOfMonth is the day of month the review should be repeated on. | ||
type ReviewDayOfMonth int | ||
|
||
|
@@ -123,7 +138,7 @@ type AccessList struct { | |
Spec Spec `json:"spec" yaml:"spec"` | ||
|
||
// Status contains dynamically calculated fields. | ||
Status Status `json:"-" yaml:"-"` | ||
Status Status `json:"status" yaml:"status"` | ||
} | ||
|
||
// Spec is the specification for an access list. | ||
|
@@ -167,6 +182,10 @@ type Owner struct { | |
|
||
// IneligibleStatus describes the reason why this owner is not eligible. | ||
IneligibleStatus string `json:"ineligible_status" yaml:"ineligible_status"` | ||
|
||
// MembershipKind describes the kind of ownership, | ||
// either "MEMBERSHIP_KIND_USER" or "MEMBERSHIP_KIND_LIST". | ||
MembershipKind string `json:"membership_kind" yaml:"membership_kind"` | ||
} | ||
|
||
// Audit describes the audit configuration for an access list. | ||
|
@@ -224,7 +243,14 @@ type Grants struct { | |
// Status contains dynamic fields calculated during retrieval. | ||
type Status struct { | ||
// MemberCount is the number of members in the access list. | ||
MemberCount *uint32 | ||
MemberCount *uint32 `json:"-" yaml:"-"` | ||
// MemberListCount is the number of members in the access list that are lists themselves. | ||
MemberListCount *uint32 `json:"-" yaml:"-"` | ||
|
||
// OwnerOf is a list of Access List UUIDs where this access list is an explicit owner. | ||
OwnerOf []string `json:"owner_of" yaml:"owner_of"` | ||
// MemberOf is a list of Access List UUIDs where this access list is an explicit member. | ||
MemberOf []string `json:"member_of" yaml:"member_of"` | ||
} | ||
|
||
// NewAccessList will create a new access list. | ||
|
@@ -286,10 +312,6 @@ func (a *AccessList) CheckAndSetDefaults() error { | |
a.Spec.Audit.Notifications.Start = twoWeeks | ||
} | ||
|
||
if len(a.Spec.Grants.Roles) == 0 && len(a.Spec.Grants.Traits) == 0 { | ||
return trace.BadParameter("grants must specify at least one role or trait") | ||
} | ||
Comment on lines
-289
to
-291
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this validation change is intended ? This change will allow to create a access list without any grants role and traits role. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, creating a list without any roles or traits was part of the initial RFD |
||
|
||
// Deduplicate owners. The backend will currently prevent this, but it's possible that access lists | ||
// were created with duplicated owners before the backend checked for duplicate owners. In order to | ||
// ensure that these access lists are backwards compatible, we'll deduplicate them here. | ||
|
@@ -299,6 +321,9 @@ func (a *AccessList) CheckAndSetDefaults() error { | |
if owner.Name == "" { | ||
return trace.BadParameter("owner name is missing") | ||
} | ||
if owner.MembershipKind == "" { | ||
owner.MembershipKind = MembershipKindUser | ||
} | ||
|
||
if _, ok := ownerMap[owner.Name]; ok { | ||
continue | ||
|
@@ -317,7 +342,7 @@ func (a *AccessList) GetOwners() []Owner { | |
return a.Spec.Owners | ||
} | ||
|
||
// GetOwners returns the list of owners from the access list. | ||
// SetOwners sets the owners of the access list. | ||
func (a *AccessList) SetOwners(owners []Owner) { | ||
a.Spec.Owners = owners | ||
} | ||
|
@@ -337,6 +362,11 @@ func (a *AccessList) GetGrants() Grants { | |
return a.Spec.Grants | ||
} | ||
|
||
// GetOwnerGrants returns the owner grants from the access list. | ||
func (a *AccessList) GetOwnerGrants() Grants { | ||
return a.Spec.OwnerGrants | ||
} | ||
|
||
// GetMetadata returns metadata. This is specifically for conforming to the Resource interface, | ||
// and should be removed when possible. | ||
func (a *AccessList) GetMetadata() types.Metadata { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like according to
proto
field order it should field nr. 8There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc there was a reason it was set to field 9, let me see if I can find it..