Skip to content

Commit f7ee96c

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

File tree

3 files changed

+605
-7
lines changed

3 files changed

+605
-7
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: 118 additions & 7 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,28 +557,102 @@ 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+
// CACHE_WRITE implies CAS_WRITE, so grant CAS_WRITE if CACHE_WRITE is present.
613+
// This makes sure that CAS_WRITE is preserved after the masking below even if
614+
// CACHE_WRITE is removed.
615+
if row.Capabilities&int32(cappb.Capability_CACHE_WRITE) != 0 {
616+
row.Capabilities |= int32(cappb.Capability_CAS_WRITE)
617+
}
618+
row.Capabilities &= mask
619+
if row.Capabilities&int32(cappb.Capability_CACHE_WRITE) != 0 {
620+
row.Capabilities ^= int32(cappb.Capability_CAS_WRITE)
621+
}
622+
}
623+
out = append(out, row)
624+
}
625+
slices.SortFunc(out, func(a, b *apiKeyGroupFullData) int {
626+
return strings.Compare(a.Label, b.Label)
627+
})
628+
return out, nil
558629
}
559630

560631
func (d *AuthDB) newAPIKeyLookupQuery(subDomain string) *query_builder.Query {
561-
qb := query_builder.NewQuery(`
632+
// This query may return multiple rows per API key for user API keys
633+
// when a user has multiple memberships in a group (e.g. direct membership
634+
// and indirect membership through a user list).
635+
qb := query_builder.NewQuery(fmt.Sprintf(`
562636
SELECT
563637
ak.*,
638+
membership.role AS membership_role,
564639
g.use_group_owned_executors,
565640
g.cache_encryption_enabled,
566641
g.enforce_ip_rules,
567642
g.is_parent,
568643
g.status AS group_status
569-
FROM "Groups" AS g,
570-
"APIKeys" AS ak
571-
`)
572-
qb.AddWhereClause(`ak.group_id = g.group_id`)
644+
FROM "APIKeys" AS ak
645+
JOIN "Groups" AS g ON ak.group_id = g.group_id
646+
LEFT JOIN (
647+
%s
648+
) AS membership
649+
ON membership.user_id = ak.user_id AND membership.group_id = ak.group_id
650+
`, d.userMembershipRolesQuery()))
573651
qb.AddWhereClause(`expiry_usec = 0 OR expiry_usec > ?`, d.clock.Now().UnixMicro())
574652

575653
if subDomain != "" {
576654
qb.AddWhereClause("url_identifier = ?", subDomain)
577655
}
578-
579656
if *userOwnedKeysEnabled {
580657
// Note: the org can disable user-owned keys at any time, and the
581658
// predicate here ensures that existing keys are effectively deactivated
@@ -585,6 +662,15 @@ func (d *AuthDB) newAPIKeyLookupQuery(subDomain string) *query_builder.Query {
585662
OR ak.user_id = ''
586663
OR ak.user_id IS NULL
587664
)`)
665+
666+
// User-owned keys should only be considered if the owning user still has a
667+
// valid direct or indirect membership in the key's group.
668+
qb.AddWhereClause(`(
669+
ak.user_id = ''
670+
OR ak.user_id IS NULL
671+
OR membership.user_id IS NOT NULL
672+
)`)
673+
588674
} else {
589675
qb.AddWhereClause(`(
590676
ak.user_id = ''
@@ -594,6 +680,31 @@ func (d *AuthDB) newAPIKeyLookupQuery(subDomain string) *query_builder.Query {
594680
return qb
595681
}
596682

683+
func (d *AuthDB) userMembershipRolesQuery() string {
684+
memberStatus := int32(grpb.GroupMembershipStatus_MEMBER)
685+
q := fmt.Sprintf(`
686+
SELECT
687+
ug.user_user_id AS user_id,
688+
ug.group_group_id AS group_id,
689+
ug.role
690+
FROM "UserGroups" AS ug
691+
WHERE ug.membership_status = %d
692+
`, memberStatus)
693+
if authutil.UserListsEnabled() {
694+
q += `
695+
UNION ALL
696+
SELECT
697+
uu.user_user_id AS user_id,
698+
ulg.group_group_id AS group_id,
699+
ulg.role
700+
FROM "UserUserLists" AS uu
701+
JOIN "UserListGroups" AS ulg
702+
ON ulg.user_list_user_list_id = uu.user_list_user_list_id
703+
`
704+
}
705+
return q
706+
}
707+
597708
func redactInvalidAPIKey(key string) string {
598709
if len(key) < 8 {
599710
return "***"

0 commit comments

Comments
 (0)