Skip to content

Commit

Permalink
Allow an Admin API key for a 'parent' group to create new groups. (#7724
Browse files Browse the repository at this point in the history
)

The new groups will inherit the SAML IDP Metadata URL of the original
group.

<!-- Optional: Provide additional context (beyond the PR title). -->

<!-- Optional: link a GitHub issue.
Example: "Fixes #123" will auto-close #123 when the PR is merged. -->

**Related issues**: N/A
  • Loading branch information
vadimberezniker authored Oct 11, 2024
1 parent 926c9b7 commit c3f3c88
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 4 deletions.
4 changes: 3 additions & 1 deletion enterprise/server/backends/userdb/userdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ func (d *UserDB) UpdateGroup(ctx context.Context, g *tables.Group) (string, erro
suggestion_preference = ?,
restrict_clean_workflow_runs_to_admins = ?,
enforce_ip_rules = ?,
is_parent = ?
is_parent = ?,
saml_idp_metadata_url = ?
WHERE group_id = ?`,
g.Name,
g.URLIdentifier,
Expand All @@ -380,6 +381,7 @@ func (d *UserDB) UpdateGroup(ctx context.Context, g *tables.Group) (string, erro
g.RestrictCleanWorkflowRunsToAdmins,
g.EnforceIPRules,
g.IsParent,
g.SamlIdpMetadataUrl,
g.GroupID,
).Exec().Error
if err != nil {
Expand Down
20 changes: 20 additions & 0 deletions enterprise/server/buildbuddy_server/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_test(
name = "buildbuddy_server_test",
srcs = ["buildbuddy_server_test.go"],
deps = [
"//enterprise/server/testutil/enterprise_testauth",
"//enterprise/server/testutil/enterprise_testenv",
"//proto:api_key_go_proto",
"//proto:group_go_proto",
"//server/buildbuddy_server",
"//server/environment",
"//server/tables",
"//server/testutil/testauth",
"//server/util/testing/flags",
"@com_github_stretchr_testify//require",
],
)

package(default_visibility = ["//enterprise:__subpackages__"])
89 changes: 89 additions & 0 deletions enterprise/server/buildbuddy_server/buildbuddy_server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// This is a test for server/buildbuddy_server that exercises enterprise
// features that cannot be referenced in the non-enterprise test.
package buildbuddy_server_test

import (
"context"
"testing"

"github.com/buildbuddy-io/buildbuddy/enterprise/server/testutil/enterprise_testauth"
"github.com/buildbuddy-io/buildbuddy/enterprise/server/testutil/enterprise_testenv"
akpb "github.com/buildbuddy-io/buildbuddy/proto/api_key"
grpb "github.com/buildbuddy-io/buildbuddy/proto/group"
"github.com/buildbuddy-io/buildbuddy/server/buildbuddy_server"
"github.com/buildbuddy-io/buildbuddy/server/environment"
"github.com/buildbuddy-io/buildbuddy/server/tables"
"github.com/buildbuddy-io/buildbuddy/server/testutil/testauth"
"github.com/buildbuddy-io/buildbuddy/server/util/testing/flags"
"github.com/stretchr/testify/require"
)

func authUserCtx(ctx context.Context, env environment.Env, t *testing.T, userID string) context.Context {
auth := env.GetAuthenticator().(*testauth.TestAuthenticator)
ctx, err := auth.WithAuthenticatedUser(ctx, userID)
require.NoError(t, err)
return ctx
}

func getGroup(t *testing.T, ctx context.Context, env environment.Env) *tables.GroupRole {
tu, err := env.GetUserDB().GetUser(ctx)
require.NoError(t, err, "failed to get self-owned group")
require.Len(t, tu.Groups, 1, "getGroup: user must be part of exactly one group")
return tu.Groups[0]
}

func TestCreateGroup(t *testing.T) {
te := enterprise_testenv.New(t)
enterprise_testauth.Configure(t, te)
auth := te.GetAuthenticator()
te.SetAuthenticator(auth)
ctx := context.Background()

flags.Set(t, "app.create_group_per_user", true)
flags.Set(t, "app.no_default_user_group", true)

err := te.GetUserDB().InsertUser(ctx, &tables.User{UserID: "US1", SubID: "US1SubID"})
require.NoError(t, err)
userCtx := authUserCtx(ctx, te, t, "US1")
parentGroup := getGroup(t, userCtx, te).Group
parentGroup.SamlIdpMetadataUrl = "https://some/saml/url"
parentGroup.URLIdentifier = "foo"
_, err = te.GetUserDB().UpdateGroup(userCtx, &parentGroup)
require.NoError(t, err)

adminKey, err := te.GetAuthDB().CreateAPIKey(
userCtx, parentGroup.GroupID, "admin",
[]akpb.ApiKey_Capability{akpb.ApiKey_ORG_ADMIN_CAPABILITY},
false /*=visibleToDevelopers*/)
require.NoError(t, err)
adminKeyCtx := te.GetAuthenticator().AuthContextFromAPIKey(ctx, adminKey.Value)

server, err := buildbuddy_server.NewBuildBuddyServer(te, nil)
require.NoError(t, err)

// Create a new group. The SAML IDP Metadata URL should not be set as the
// first group is not marked as a "parent".
rsp, err := server.CreateGroup(adminKeyCtx, &grpb.CreateGroupRequest{
Name: "test",
UrlIdentifier: "test",
})
require.NoError(t, err)
g, err := te.GetUserDB().GetGroupByID(ctx, rsp.GetId())
require.NoError(t, err)
require.Empty(t, g.SamlIdpMetadataUrl)

// Make the first group a parent and try again.
// The SAML IDP Metadata URL should match that of the original group.
parentGroup.IsParent = true
_, err = te.GetUserDB().UpdateGroup(userCtx, &parentGroup)
require.NoError(t, err)
rsp, err = server.CreateGroup(adminKeyCtx, &grpb.CreateGroupRequest{
Name: "test2",
UrlIdentifier: "test2",
})
require.NoError(t, err)
g, err = te.GetUserDB().GetGroupByID(ctx, rsp.GetId())
require.NoError(t, err)
require.Equal(t, parentGroup.SamlIdpMetadataUrl, g.SamlIdpMetadataUrl)
require.False(t, g.IsParent)
}
28 changes: 25 additions & 3 deletions server/buildbuddy_server/buildbuddy_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ func (s *BuildBuddyServer) CreateGroup(ctx context.Context, req *grpb.CreateGrou
if userDB == nil {
return nil, status.UnimplementedError("Not Implemented")
}
user, err := userDB.GetUser(ctx)
u, err := s.env.GetAuthenticator().AuthenticatedUser(ctx)
if err != nil {
return nil, err
}
Expand All @@ -493,13 +493,21 @@ func (s *BuildBuddyServer) CreateGroup(ctx context.Context, req *grpb.CreateGrou
}

groupOwnedDomain := ""
if req.GetAutoPopulateFromOwnedDomain() {
// Groups created by a user via the UI can have "auto-join by domain"
// enabled for low-friction onboarding. Groups created using an API key
// are intended to be managed manually so we do not currently allow
// joining by domain.
if req.GetAutoPopulateFromOwnedDomain() && u.GetUserID() != "" {
user, err := userDB.GetUser(ctx)
if err != nil {
return nil, err
}
userEmailDomain := getEmailDomain(user.Email)
groupOwnedDomain = userEmailDomain
}

group := &tables.Group{
UserID: user.UserID,
UserID: u.GetUserID(),
Name: groupName,
OwnedDomain: groupOwnedDomain,
SharingEnabled: req.GetSharingEnabled(),
Expand All @@ -509,6 +517,20 @@ func (s *BuildBuddyServer) CreateGroup(ctx context.Context, req *grpb.CreateGrou
DeveloperOrgCreationEnabled: req.GetDeveloperOrgCreationEnabled(),
UseGroupOwnedExecutors: req.GetUseGroupOwnedExecutors(),
}

// For groups created using an API Key allow the SAML IDP Metadata URL
// to be inherited if the API Key group is marked as a 'parent' group.
// This allows the new group to be managed using a parent group API key.
if u.HasCapability(akpb.ApiKey_ORG_ADMIN_CAPABILITY) && u.GetUserID() == "" {
existingGroup, err := userDB.GetGroupByID(ctx, u.GetGroupID())
if err != nil {
return nil, err
}
if existingGroup.IsParent {
group.SamlIdpMetadataUrl = existingGroup.SamlIdpMetadataUrl
}
}

group.URLIdentifier = strings.TrimSpace(req.GetUrlIdentifier())
group.SuggestionPreference = grpb.SuggestionPreference_ENABLED

Expand Down

0 comments on commit c3f3c88

Please sign in to comment.