feat: remove org flag requirement for provisioners (#14722)

This commit is contained in:
Garrett Delfosse
2024-09-20 12:45:31 -04:00
committed by GitHub
parent 96e9a4f85c
commit 50124fefdc
4 changed files with 102 additions and 166 deletions
+30 -44
View File
@@ -24,7 +24,6 @@ import (
"github.com/coder/coder/v2/cli/cliui"
"github.com/coder/coder/v2/cli/cliutil"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/provisionerkey"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/drpc"
"github.com/coder/coder/v2/provisioner/terraform"
@@ -73,32 +72,30 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
interruptCtx, interruptCancel := inv.SignalNotifyContext(ctx, agpl.InterruptSignals...)
defer interruptCancel()
// This can fail to get the current organization
// if the client is not authenticated as a user,
// like when only PSK is provided.
// This will be cleaner once PSK is replaced
// with org scoped authentication tokens.
org, err := orgContext.Selected(inv, client)
if err != nil {
var cErr *codersdk.Error
if !errors.As(err, &cErr) || cErr.StatusCode() != http.StatusUnauthorized {
return xerrors.Errorf("current organization: %w", err)
}
orgID := uuid.Nil
if preSharedKey == "" && provisionerKey == "" {
// We can only select an organization if using user auth
org, err := orgContext.Selected(inv, client)
if err != nil {
var cErr *codersdk.Error
if !errors.As(err, &cErr) || cErr.StatusCode() != http.StatusUnauthorized {
return xerrors.Errorf("current organization: %w", err)
}
if preSharedKey == "" && provisionerKey == "" {
return xerrors.New("must provide a pre-shared key or provisioner key when not authenticated as a user")
}
org = codersdk.Organization{MinimalOrganization: codersdk.MinimalOrganization{ID: uuid.Nil}}
if orgContext.FlagSelect != "" {
// If we are using PSK, we can't fetch the organization
// to validate org name so we need the user to provide
// a valid organization ID.
orgID, err := uuid.Parse(orgContext.FlagSelect)
if err != nil {
return xerrors.New("must provide an org ID when not authenticated as a user and organization is specified")
}
org = codersdk.Organization{MinimalOrganization: codersdk.MinimalOrganization{ID: orgID}}
orgID = org.ID
} else if orgContext.FlagSelect != "" {
return xerrors.New("cannot provide --org value with --psk or --key flags")
}
if provisionerKey != "" {
if preSharedKey != "" {
return xerrors.New("cannot provide both provisioner key --key and pre-shared key --psk")
}
if len(rawTags) > 0 {
return xerrors.New("cannot provide tags when using provisioner key")
}
}
@@ -115,19 +112,6 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
return err
}
if provisionerKey != "" {
if preSharedKey != "" {
return xerrors.New("cannot provide both provisioner key --key and pre-shared key --psk")
}
if len(rawTags) > 0 {
return xerrors.New("cannot provide tags when using provisioner key")
}
err = provisionerkey.Validate(provisionerKey)
if err != nil {
return xerrors.Errorf("validate provisioner key: %w", err)
}
}
logOpts := []clilog.Option{
clilog.WithFilter(logFilter...),
clilog.WithHuman(logHuman),
@@ -232,7 +216,7 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
},
Tags: tags,
PreSharedKey: preSharedKey,
Organization: org.ID,
Organization: orgID,
ProvisionerKey: provisionerKey,
})
}, &provisionerd.Options{
@@ -281,6 +265,13 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
},
}
keyOption := serpent.Option{
Flag: "key",
Env: "CODER_PROVISIONER_DAEMON_KEY",
Description: "Provisioner key to authenticate with Coder server.",
Value: serpent.StringOf(&provisionerKey),
Hidden: true,
}
cmd.Options = serpent.OptionSet{
{
Flag: "cache-dir",
@@ -316,14 +307,9 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
Env: "CODER_PROVISIONER_DAEMON_PSK",
Description: "Pre-shared key to authenticate with Coder server.",
Value: serpent.StringOf(&preSharedKey),
UseInstead: []serpent.Option{keyOption},
},
{
Flag: "key",
Env: "CODER_PROVISIONER_DAEMON_KEY",
Description: "Provisioner key to authenticate with Coder server.",
Value: serpent.StringOf(&provisionerKey),
Hidden: true,
},
keyOption,
{
Flag: "name",
Env: "CODER_PROVISIONER_DAEMON_NAME",
+2 -68
View File
@@ -68,46 +68,6 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
require.Equal(t, proto.CurrentVersion.String(), daemons[0].APIVersion)
})
t.Run("AnotherOrg", func(t *testing.T) {
t.Parallel()
dv := coderdtest.DeploymentValues(t)
dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)}
client, _ := coderdenttest.New(t, &coderdenttest.Options{
Options: &coderdtest.Options{
DeploymentValues: dv,
},
ProvisionerDaemonPSK: "provisionersftw",
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureExternalProvisionerDaemons: 1,
codersdk.FeatureMultipleOrganizations: 1,
},
},
})
anotherOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{})
inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name", "org-daemon", "--org", anotherOrg.ID.String())
err := conf.URL().Write(client.URL.String())
require.NoError(t, err)
pty := ptytest.New(t).Attach(inv)
ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
defer cancel()
clitest.Start(t, inv)
pty.ExpectMatchContext(ctx, "starting provisioner daemon")
var daemons []codersdk.ProvisionerDaemon
require.Eventually(t, func() bool {
daemons, err = client.OrganizationProvisionerDaemons(ctx, anotherOrg.ID)
if err != nil {
return false
}
return len(daemons) == 1
}, testutil.WaitShort, testutil.IntervalSlow)
assert.Equal(t, "org-daemon", daemons[0].Name)
assert.Equal(t, provisionersdk.ScopeOrganization, daemons[0].Tags[provisionersdk.TagScope])
assert.Equal(t, buildinfo.Version(), daemons[0].Version)
assert.Equal(t, proto.CurrentVersion.String(), daemons[0].APIVersion)
})
t.Run("AnotherOrgByNameWithUser", func(t *testing.T) {
t.Parallel()
dv := coderdtest.DeploymentValues(t)
@@ -126,7 +86,7 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
})
anotherOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{})
anotherClient, _ := coderdtest.CreateAnotherUser(t, client, anotherOrg.ID, rbac.RoleTemplateAdmin())
inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name", "org-daemon", "--org", anotherOrg.Name)
inv, conf := newCLI(t, "provisionerd", "start", "--name", "org-daemon", "--org", anotherOrg.Name)
clitest.SetupConfig(t, anotherClient, conf)
pty := ptytest.New(t).Attach(inv)
ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
@@ -135,32 +95,6 @@ func TestProvisionerDaemon_PSK(t *testing.T) {
pty.ExpectMatchContext(ctx, "starting provisioner daemon")
})
t.Run("AnotherOrgByNameNoUser", func(t *testing.T) {
t.Parallel()
dv := coderdtest.DeploymentValues(t)
dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)}
client, _ := coderdenttest.New(t, &coderdenttest.Options{
Options: &coderdtest.Options{
DeploymentValues: dv,
},
ProvisionerDaemonPSK: "provisionersftw",
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureExternalProvisionerDaemons: 1,
codersdk.FeatureMultipleOrganizations: 1,
},
},
})
anotherOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{})
inv, conf := newCLI(t, "provisionerd", "start", "--psk=provisionersftw", "--name", "org-daemon", "--org", anotherOrg.Name)
err := conf.URL().Write(client.URL.String())
require.NoError(t, err)
ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong)
defer cancel()
err = inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "must provide an org ID when not authenticated as a user and organization is specified")
})
t.Run("NoUserNoPSK", func(t *testing.T) {
t.Parallel()
client, _ := coderdenttest.New(t, &coderdenttest.Options{
@@ -467,7 +401,7 @@ func TestProvisionerDaemon_ProvisionerKey(t *testing.T) {
Name: "dont-TEST-me",
})
require.NoError(t, err)
inv, conf := newCLI(t, "provisionerd", "start", "--org", anotherOrg.ID.String(), "--key", res.Key, "--name=matt-daemon")
inv, conf := newCLI(t, "provisionerd", "start", "--key", res.Key, "--name=matt-daemon")
err = conf.URL().Write(client.URL.String())
require.NoError(t, err)
pty := ptytest.New(t).Attach(inv)
@@ -43,6 +43,7 @@ OPTIONS:
--psk string, $CODER_PROVISIONER_DAEMON_PSK
Pre-shared key to authenticate with Coder server.
DEPRECATED: Use --key instead.
-t, --tag string-array, $CODER_PROVISIONERD_TAGS
Tags to filter provisioner jobs by.
+69 -54
View File
@@ -74,6 +74,12 @@ func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.List(daemons, db2sdk.ProvisionerDaemon))
}
type provisiionerDaemonAuthResponse struct {
keyID uuid.UUID
orgID uuid.UUID
tags map[string]string
}
type provisionerDaemonAuth struct {
psk string
db database.Store
@@ -82,77 +88,85 @@ type provisionerDaemonAuth struct {
// authorize returns mutated tags if the given HTTP request is authorized to access the provisioner daemon
// protobuf API, and returns nil, err otherwise.
func (p *provisionerDaemonAuth) authorize(r *http.Request, orgID uuid.UUID, tags map[string]string) (uuid.UUID, map[string]string, error) {
func (p *provisionerDaemonAuth) authorize(r *http.Request, org database.Organization, tags map[string]string) (provisiionerDaemonAuthResponse, error) {
ctx := r.Context()
apiKey, apiKeyOK := httpmw.APIKeyOptional(r)
pk, pkOK := httpmw.ProvisionerKeyAuthOptional(r)
provAuth := httpmw.ProvisionerDaemonAuthenticated(r)
if !provAuth && !apiKeyOK {
return uuid.Nil, nil, xerrors.New("no API key or provisioner key provided")
return provisiionerDaemonAuthResponse{}, xerrors.New("no API key or provisioner key provided")
}
if apiKeyOK && pkOK {
return uuid.Nil, nil, xerrors.New("Both API key and provisioner key authentication provided. Only one is allowed.")
return provisiionerDaemonAuthResponse{}, xerrors.New("Both API key and provisioner key authentication provided. Only one is allowed.")
}
// Provisioner Key Auth
if pkOK {
if pk.OrganizationID != orgID {
return uuid.Nil, nil, xerrors.New("provisioner key unauthorized")
}
if tags != nil && !maps.Equal(tags, map[string]string{}) {
return uuid.Nil, nil, xerrors.New("tags are not allowed when using a provisioner key")
return provisiionerDaemonAuthResponse{}, xerrors.New("tags are not allowed when using a provisioner key")
}
// If using provisioner key / PSK auth, the daemon is, by definition, scoped to the organization.
// Use the provisioner key tags here.
tags = provisionersdk.MutateTags(uuid.Nil, pk.Tags)
return pk.ID, tags, nil
}
// User Auth
if apiKeyOK {
userKey, err := uuid.Parse(codersdk.ProvisionerKeyIDUserAuth)
if err != nil {
return uuid.Nil, nil, xerrors.Errorf("parse user provisioner key id: %w", err)
}
tags = provisionersdk.MutateTags(apiKey.UserID, tags)
if tags[provisionersdk.TagScope] == provisionersdk.ScopeUser {
// Any authenticated user can create provisioner daemons scoped
// for jobs that they own,
return userKey, tags, nil
}
ua := httpmw.UserAuthorization(r)
err = p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(orgID))
if err != nil {
if !provAuth {
return uuid.Nil, nil, xerrors.New("user unauthorized")
}
pskKey, err := uuid.Parse(codersdk.ProvisionerKeyIDPSK)
if err != nil {
return uuid.Nil, nil, xerrors.Errorf("parse psk provisioner key id: %w", err)
}
// Allow fallback to PSK auth if the user is not allowed to create provisioner daemons.
// This is to preserve backwards compatibility with existing user provisioner daemons.
// If using PSK auth, the daemon is, by definition, scoped to the organization.
tags = provisionersdk.MutateTags(uuid.Nil, tags)
return pskKey, tags, nil
}
return userKey, tags, nil
return provisiionerDaemonAuthResponse{
keyID: pk.ID,
orgID: pk.OrganizationID,
tags: tags,
}, nil
}
// PSK Auth
pskKey, err := uuid.Parse(codersdk.ProvisionerKeyIDPSK)
if err != nil {
return uuid.Nil, nil, xerrors.Errorf("parse psk provisioner key id: %w", err)
if provAuth {
if !org.IsDefault {
return provisiionerDaemonAuthResponse{}, xerrors.Errorf("PSK auth is only allowed for the default organization '%s'", org.Name)
}
pskKey, err := uuid.Parse(codersdk.ProvisionerKeyIDPSK)
if err != nil {
return provisiionerDaemonAuthResponse{}, xerrors.Errorf("parse psk provisioner key id: %w", err)
}
tags = provisionersdk.MutateTags(uuid.Nil, tags)
return provisiionerDaemonAuthResponse{
keyID: pskKey,
orgID: org.ID,
tags: tags,
}, nil
}
tags = provisionersdk.MutateTags(uuid.Nil, tags)
return pskKey, tags, nil
// User Auth
if !apiKeyOK {
return provisiionerDaemonAuthResponse{}, xerrors.New("no API key provided")
}
userKey, err := uuid.Parse(codersdk.ProvisionerKeyIDUserAuth)
if err != nil {
return provisiionerDaemonAuthResponse{}, xerrors.Errorf("parse user provisioner key id: %w", err)
}
tags = provisionersdk.MutateTags(apiKey.UserID, tags)
if tags[provisionersdk.TagScope] == provisionersdk.ScopeUser {
// Any authenticated user can create provisioner daemons scoped
// for jobs that they own,
return provisiionerDaemonAuthResponse{
keyID: userKey,
orgID: org.ID,
tags: tags,
}, nil
}
ua := httpmw.UserAuthorization(r)
err = p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(org.ID))
if err != nil {
return provisiionerDaemonAuthResponse{}, xerrors.New("user unauthorized")
}
return provisiionerDaemonAuthResponse{
keyID: userKey,
orgID: org.ID,
tags: tags,
}, nil
}
// Serves the provisioner daemon protobuf API over a WebSocket.
@@ -166,7 +180,6 @@ func (p *provisionerDaemonAuth) authorize(r *http.Request, orgID uuid.UUID, tags
// @Router /organizations/{organization}/provisionerdaemons/serve [get]
func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
organization := httpmw.OrganizationParam(r)
tags := map[string]string{}
if r.URL.Query().Has("tag") {
@@ -215,7 +228,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
api.Logger.Warn(ctx, "unnamed provisioner daemon")
}
keyID, tags, err := api.provisionerDaemonAuth.authorize(r, organization.ID, tags)
authRes, err := api.provisionerDaemonAuth.authorize(r, httpmw.OrganizationParam(r), tags)
if err != nil {
api.Logger.Warn(ctx, "unauthorized provisioner daemon serve request", slog.F("tags", tags), slog.Error(err))
httpapi.Write(ctx, rw, http.StatusForbidden,
@@ -226,6 +239,8 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
)
return
}
tags = authRes.tags
api.Logger.Debug(ctx, "provisioner authorized", slog.F("tags", tags))
if err := provisionerdserver.Tags(tags).Valid(); err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -287,8 +302,8 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
LastSeenAt: sql.NullTime{Time: now, Valid: true},
Version: versionHdrVal,
APIVersion: apiVersion,
OrganizationID: organization.ID,
KeyID: keyID,
OrganizationID: authRes.orgID,
KeyID: authRes.keyID,
})
if err != nil {
if !xerrors.Is(err, context.Canceled) {
@@ -351,7 +366,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
srvCtx,
api.AccessURL,
daemon.ID,
organization.ID,
authRes.orgID,
logger,
provisioners,
tags,