Skip to content

Commit 74aade9

Browse files
Enforce user API capability limits at API key lookup time.
Depends on #11452
1 parent c6e13a9 commit 74aade9

File tree

3 files changed

+594
-6
lines changed

3 files changed

+594
-6
lines changed

enterprise/server/backends/authdb/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ go_library(
2424
"//server/util/perms",
2525
"//server/util/query_builder",
2626
"//server/util/random",
27+
"//server/util/role",
2728
"//server/util/status",
2829
"//server/util/subdomain",
2930
"//third_party/singleflight",

enterprise/server/backends/authdb/authdb.go

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/buildbuddy-io/buildbuddy/server/util/perms"
2525
"github.com/buildbuddy-io/buildbuddy/server/util/query_builder"
2626
"github.com/buildbuddy-io/buildbuddy/server/util/random"
27+
"github.com/buildbuddy-io/buildbuddy/server/util/role"
2728
"github.com/buildbuddy-io/buildbuddy/server/util/status"
2829
"github.com/buildbuddy-io/buildbuddy/server/util/subdomain"
2930
"github.com/buildbuddy-io/buildbuddy/third_party/singleflight"
@@ -270,6 +271,8 @@ type apiKeyGroupFullData struct {
270271
EnforceIPRules bool
271272
IsParent bool
272273
GroupStatus int32 `gorm:"column:group_status"`
274+
// Role from either direct or user-list membership for user-owned keys.
275+
MembershipRole *uint32 `gorm:"column:membership_role"`
273276
}
274277

275278
func (r *apiKeyGroupFullData) toAPIKeyGroup() *apiKeyGroup {
@@ -554,27 +557,100 @@ func (d *AuthDB) fetchAPIKeys(ctx context.Context, queryName, subDomain string,
554557
}
555558
q, args := qb.Build()
556559
rq := d.h.NewQueryWithOpts(ctx, queryName, dbOpts).Raw(q, args...)
557-
return db.ScanAll(rq, &apiKeyGroupFullData{})
560+
rows, err := db.ScanAll(rq, &apiKeyGroupFullData{})
561+
if err != nil {
562+
return nil, err
563+
}
564+
565+
// For user API keys, the above data may contain multiple rows per API key
566+
// per each membership row. We need to through the full data set and generate
567+
// one row per API key. The membership role information is used to enforce
568+
// maximum capabilities on user API keys such that the API key capabilities
569+
// never exceed the capabilities of the owning user.
570+
type consolidatedAPIKeyData struct {
571+
row *apiKeyGroupFullData
572+
// User membership information for user API keys.
573+
userMembershipRoles []role.Role
574+
}
575+
576+
// Aggregate the user memberships for each API key.
577+
consolidated := make(map[string]*consolidatedAPIKeyData, len(rows))
578+
for _, row := range rows {
579+
apiKeyID := row.APIKeyID
580+
entry, ok := consolidated[apiKeyID]
581+
if !ok {
582+
c := *row
583+
c.MembershipRole = nil
584+
entry = &consolidatedAPIKeyData{row: &c}
585+
consolidated[apiKeyID] = entry
586+
}
587+
if row.MembershipRole != nil {
588+
entry.userMembershipRoles = append(entry.userMembershipRoles, role.Role(*row.MembershipRole))
589+
}
590+
}
591+
592+
// Apply capabilities mask for user API keys based on user membership
593+
// information. User API key capabilities cannot exceed user
594+
// membership capabilities.
595+
out := make([]*apiKeyGroupFullData, 0, len(consolidated))
596+
for apiKeyID, entry := range consolidated {
597+
row := entry.row
598+
if row.UserID != "" {
599+
mask := int32(0)
600+
for _, membershipRole := range entry.userMembershipRoles {
601+
roleCaps, err := role.ToCapabilities(membershipRole)
602+
if err != nil {
603+
return nil, status.InternalErrorf("invalid membership role %d for API key %q: %s", membershipRole, apiKeyID, err)
604+
}
605+
mask |= capabilities.ToInt(roleCaps)
606+
}
607+
// User-owned keys without memberships should be treated as non-existent.
608+
// (This should already be enforced by SQL predicates.)
609+
if len(entry.userMembershipRoles) == 0 {
610+
continue
611+
}
612+
row.Capabilities &= mask
613+
}
614+
out = append(out, row)
615+
}
616+
slices.SortFunc(out, func(a, b *apiKeyGroupFullData) int {
617+
return strings.Compare(a.Label, b.Label)
618+
})
619+
return out, nil
558620
}
559621

560622
func (d *AuthDB) newAPIKeyLookupQuery(subDomain string) *query_builder.Query {
561-
qb := query_builder.NewQuery(`
623+
// This query may return multiple rows per API key for user API keys
624+
// when a user has multiple memberships in a group (e.g. direct membership
625+
// and indirect membership through a user list).
626+
qb := query_builder.NewQuery(fmt.Sprintf(`
562627
SELECT
563628
ak.*,
629+
membership.role AS membership_role,
564630
g.use_group_owned_executors,
565631
g.cache_encryption_enabled,
566632
g.enforce_ip_rules,
567633
g.is_parent,
568634
g.status AS group_status
569-
FROM "Groups" AS g,
570-
"APIKeys" AS ak
571-
`)
572-
qb.AddWhereClause(`ak.group_id = g.group_id`)
635+
FROM "APIKeys" AS ak
636+
JOIN "Groups" AS g ON ak.group_id = g.group_id
637+
LEFT JOIN (
638+
%s
639+
) AS membership
640+
ON membership.user_id = ak.user_id AND membership.group_id = ak.group_id
641+
`, d.userMembershipRolesQuery()))
573642
qb.AddWhereClause(`expiry_usec = 0 OR expiry_usec > ?`, d.clock.Now().UnixMicro())
574643

575644
if subDomain != "" {
576645
qb.AddWhereClause("url_identifier = ?", subDomain)
577646
}
647+
// User-owned keys should only be considered if the owning user still has a
648+
// valid direct or indirect membership in the key's group.
649+
qb.AddWhereClause(`(
650+
ak.user_id = ''
651+
OR ak.user_id IS NULL
652+
OR membership.user_id IS NOT NULL
653+
)`)
578654

579655
if *userOwnedKeysEnabled {
580656
// Note: the org can disable user-owned keys at any time, and the
@@ -594,6 +670,31 @@ func (d *AuthDB) newAPIKeyLookupQuery(subDomain string) *query_builder.Query {
594670
return qb
595671
}
596672

673+
func (d *AuthDB) userMembershipRolesQuery() string {
674+
memberStatus := int32(grpb.GroupMembershipStatus_MEMBER)
675+
q := fmt.Sprintf(`
676+
SELECT
677+
ug.user_user_id AS user_id,
678+
ug.group_group_id AS group_id,
679+
ug.role
680+
FROM "UserGroups" AS ug
681+
WHERE ug.membership_status = %d
682+
`, memberStatus)
683+
if authutil.UserListsEnabled() {
684+
q += `
685+
UNION ALL
686+
SELECT
687+
uu.user_user_id AS user_id,
688+
ulg.group_group_id AS group_id,
689+
ulg.role
690+
FROM "UserUserLists" AS uu
691+
JOIN "UserListGroups" AS ulg
692+
ON ulg.user_list_user_list_id = uu.user_list_user_list_id
693+
`
694+
}
695+
return q
696+
}
697+
597698
func redactInvalidAPIKey(key string) string {
598699
if len(key) < 8 {
599700
return "***"

0 commit comments

Comments
 (0)