From e098250523e9d82462fd56774279140415819c8c Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 18 Sep 2024 17:45:04 +0000 Subject: [PATCH 1/4] feat: no longer require org flag for provisioners --- enterprise/cli/provisionerdaemonstart.go | 67 ++++++-------------- enterprise/coderd/provisionerdaemons.go | 80 +++++++++++------------- 2 files changed, 54 insertions(+), 93 deletions(-) diff --git a/enterprise/cli/provisionerdaemonstart.go b/enterprise/cli/provisionerdaemonstart.go index 42f07002b8e27..3b0c979cd411d 100644 --- a/enterprise/cli/provisionerdaemonstart.go +++ b/enterprise/cli/provisionerdaemonstart.go @@ -4,9 +4,7 @@ package cli import ( "context" - "errors" "fmt" - "net/http" "os" "regexp" "time" @@ -24,7 +22,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 +70,17 @@ 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) - } - - if preSharedKey == "" && provisionerKey == "" { - return xerrors.New("must provide a pre-shared key or provisioner key when not authenticated as a user") + 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 { + return xerrors.Errorf("get organization: %w", err) } - - org = codersdk.Organization{MinimalOrganization: codersdk.MinimalOrganization{ID: uuid.Nil}} + orgID = org.ID + } else { 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}} + return xerrors.New("cannot provide --org value with --psk or --key flags") } } @@ -115,19 +97,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 +201,7 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command { }, Tags: tags, PreSharedKey: preSharedKey, - Organization: org.ID, + Organization: orgID, ProvisionerKey: provisionerKey, }) }, &provisionerd.Options{ @@ -281,6 +250,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 +292,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", diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go index 7b8b30c5bf667..e9ca2fd16eabf 100644 --- a/enterprise/coderd/provisionerdaemons.go +++ b/enterprise/coderd/provisionerdaemons.go @@ -82,77 +82,68 @@ 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) (uuid.UUID, uuid.UUID, map[string]string, 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 uuid.Nil, uuid.Nil, nil, 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 uuid.Nil, uuid.Nil, nil, 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 uuid.Nil, uuid.Nil, nil, 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 + return pk.ID, pk.OrganizationID, 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) + // PSK Auth + if provAuth { + if !org.IsDefault { + return uuid.Nil, uuid.Nil, nil, xerrors.Errorf("PSK auth is only allowed for the default organization '%s'", org.Name) } - 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)) + pskKey, err := uuid.Parse(codersdk.ProvisionerKeyIDPSK) 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) - } + return uuid.Nil, 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) + tags = provisionersdk.MutateTags(uuid.Nil, tags) + return pskKey, org.ID, tags, nil + } - return pskKey, tags, nil - } + // User Auth + if !apiKeyOK { + return uuid.Nil, uuid.Nil, nil, xerrors.New("no API key provided") + } - return userKey, tags, nil + userKey, err := uuid.Parse(codersdk.ProvisionerKeyIDUserAuth) + if err != nil { + return uuid.Nil, uuid.Nil, nil, xerrors.Errorf("parse user provisioner key id: %w", err) } - // PSK Auth - pskKey, err := uuid.Parse(codersdk.ProvisionerKeyIDPSK) + 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, org.ID, tags, nil + } + ua := httpmw.UserAuthorization(r) + err = p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(org.ID)) if err != nil { - return uuid.Nil, nil, xerrors.Errorf("parse psk provisioner key id: %w", err) + return uuid.Nil, uuid.Nil, nil, xerrors.New("user unauthorized") } - tags = provisionersdk.MutateTags(uuid.Nil, tags) - return pskKey, tags, nil + return userKey, org.ID, tags, nil } // Serves the provisioner daemon protobuf API over a WebSocket. @@ -166,7 +157,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 +205,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) + keyID, orgID, tags, 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, @@ -287,7 +277,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request) LastSeenAt: sql.NullTime{Time: now, Valid: true}, Version: versionHdrVal, APIVersion: apiVersion, - OrganizationID: organization.ID, + OrganizationID: orgID, KeyID: keyID, }) if err != nil { @@ -351,7 +341,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request) srvCtx, api.AccessURL, daemon.ID, - organization.ID, + orgID, logger, provisioners, tags, From bb7ec5eaff3ddf43b10bb03f76d56d6a43d7829a Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 19 Sep 2024 15:04:10 +0000 Subject: [PATCH 2/4] fix tests --- enterprise/cli/provisionerdaemonstart.go | 23 ++++-- enterprise/cli/provisionerdaemonstart_test.go | 70 +------------------ .../coder_provisionerd_start_--help.golden | 1 + 3 files changed, 22 insertions(+), 72 deletions(-) diff --git a/enterprise/cli/provisionerdaemonstart.go b/enterprise/cli/provisionerdaemonstart.go index 3b0c979cd411d..e47f1567e18e3 100644 --- a/enterprise/cli/provisionerdaemonstart.go +++ b/enterprise/cli/provisionerdaemonstart.go @@ -4,7 +4,9 @@ package cli import ( "context" + "errors" "fmt" + "net/http" "os" "regexp" "time" @@ -75,12 +77,25 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command { // We can only select an organization if using user auth org, err := orgContext.Selected(inv, client) if err != nil { - return xerrors.Errorf("get organization: %w", err) + var cErr *codersdk.Error + if !errors.As(err, &cErr) || cErr.StatusCode() != http.StatusUnauthorized { + return xerrors.Errorf("current organization: %w", err) + } + + return xerrors.New("must provide a pre-shared key or provisioner key when not authenticated as a user") } + orgID = org.ID - } else { - if orgContext.FlagSelect != "" { - return xerrors.New("cannot provide --org value with --psk or --key flags") + } 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") } } diff --git a/enterprise/cli/provisionerdaemonstart_test.go b/enterprise/cli/provisionerdaemonstart_test.go index 611abddd9d082..4a4a1f9f5ce46 100644 --- a/enterprise/cli/provisionerdaemonstart_test.go +++ b/enterprise/cli/provisionerdaemonstart_test.go @@ -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) diff --git a/enterprise/cli/testdata/coder_provisionerd_start_--help.golden b/enterprise/cli/testdata/coder_provisionerd_start_--help.golden index 3f20d2d04eb72..169b7fca2520c 100644 --- a/enterprise/cli/testdata/coder_provisionerd_start_--help.golden +++ b/enterprise/cli/testdata/coder_provisionerd_start_--help.golden @@ -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. From a4cecbe321a8dc89a609075a380ffc7d449263ac Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 20 Sep 2024 15:57:58 +0000 Subject: [PATCH 3/4] use response struct --- enterprise/coderd/provisionerdaemons.go | 57 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go index e9ca2fd16eabf..ad52925b52a95 100644 --- a/enterprise/coderd/provisionerdaemons.go +++ b/enterprise/coderd/provisionerdaemons.go @@ -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,68 +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, org database.Organization, tags map[string]string) (uuid.UUID, 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, 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, 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 tags != nil && !maps.Equal(tags, map[string]string{}) { - return uuid.Nil, 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, pk.OrganizationID, tags, nil + return provisiionerDaemonAuthResponse{ + keyID: pk.ID, + orgID: pk.OrganizationID, + tags: tags, + }, nil } // PSK Auth if provAuth { if !org.IsDefault { - return uuid.Nil, uuid.Nil, nil, xerrors.Errorf("PSK auth is only allowed for the default organization '%s'", org.Name) + 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 uuid.Nil, uuid.Nil, nil, xerrors.Errorf("parse psk provisioner key id: %w", err) + return provisiionerDaemonAuthResponse{}, xerrors.Errorf("parse psk provisioner key id: %w", err) } tags = provisionersdk.MutateTags(uuid.Nil, tags) - return pskKey, org.ID, tags, nil + + return provisiionerDaemonAuthResponse{ + keyID: pskKey, + orgID: org.ID, + tags: tags, + }, nil } // User Auth if !apiKeyOK { - return uuid.Nil, uuid.Nil, nil, xerrors.New("no API key provided") + return provisiionerDaemonAuthResponse{}, xerrors.New("no API key provided") } userKey, err := uuid.Parse(codersdk.ProvisionerKeyIDUserAuth) if err != nil { - return uuid.Nil, uuid.Nil, nil, xerrors.Errorf("parse user provisioner key id: %w", err) + 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 userKey, org.ID, tags, nil + 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 uuid.Nil, uuid.Nil, nil, xerrors.New("user unauthorized") + return provisiionerDaemonAuthResponse{}, xerrors.New("user unauthorized") } - return userKey, org.ID, tags, nil + return provisiionerDaemonAuthResponse{ + keyID: userKey, + orgID: org.ID, + tags: tags, + }, nil } // Serves the provisioner daemon protobuf API over a WebSocket. @@ -205,7 +228,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request) api.Logger.Warn(ctx, "unnamed provisioner daemon") } - keyID, orgID, tags, err := api.provisionerDaemonAuth.authorize(r, httpmw.OrganizationParam(r), 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, @@ -216,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{ @@ -277,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: orgID, - KeyID: keyID, + OrganizationID: authRes.orgID, + KeyID: authRes.keyID, }) if err != nil { if !xerrors.Is(err, context.Canceled) { From 0ff8c45bc69e50cdec93086417ca9890545a9e22 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 20 Sep 2024 15:58:15 +0000 Subject: [PATCH 4/4] use response struct --- enterprise/coderd/provisionerdaemons.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go index ad52925b52a95..6f8cd1a3ec0b6 100644 --- a/enterprise/coderd/provisionerdaemons.go +++ b/enterprise/coderd/provisionerdaemons.go @@ -366,7 +366,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request) srvCtx, api.AccessURL, daemon.ID, - orgID, + authRes.orgID, logger, provisioners, tags,