Skip to content
Merged
Prev Previous commit
Next Next commit
gen
  • Loading branch information
f0ssel committed Sep 16, 2024
commit 683876bccc37a2b7ee0439dd3f633dd4e0e859c5
4 changes: 4 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,11 @@ func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, n
dbTypes = append(dbTypes, database.ProvisionerType(tp))
}

keyID, err := uuid.Parse(codersdk.ProvisionerKeyIDBuiltIn)
if err != nil {
return nil, xerrors.Errorf("failed to parse built-in provisioner key ID: %w", err)
}

//nolint:gocritic // in-memory provisioners are owned by system
daemon, err := api.Database.UpsertProvisionerDaemon(dbauthz.AsSystemRestricted(dialCtx), database.UpsertProvisionerDaemonParams{
Name: name,
Expand All @@ -1501,6 +1506,7 @@ func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, n
LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
Version: buildinfo.Version(),
APIVersion: proto.CurrentVersion.String(),
KeyID: keyID,
})
if err != nil {
return nil, xerrors.Errorf("failed to create in-memory provisioner daemon: %w", err)
Expand Down
6 changes: 5 additions & 1 deletion coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions coderd/database/foreign_key_constraint.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
INSERT INTO provisioner_keys (id, created_at, name, public_key, organization_id) VALUES (gen_random_uuid(), NOW(), 'built-in', '', (SELECT id FROM organizations WHERE is_default = true));
INSERT INTO provisioner_keys (id, created_at, name, public_key, organization_id) VALUES (gen_random_uuid(), NOW(), 'psk', '', (SELECT id FROM organizations WHERE is_default = true));
INSERT INTO provisioner_keys (id, created_at, organization_id, name, hashed_secret, tags) VALUES ('11111111-1111-1111-1111-111111111111'::uuid, NOW(), (SELECT id FROM organizations WHERE is_default = true), 'built-in', ''::bytea, '{}');
INSERT INTO provisioner_keys (id, created_at, organization_id, name, hashed_secret, tags) VALUES ('22222222-2222-2222-2222-222222222222'::uuid, NOW(), (SELECT id FROM organizations WHERE is_default = true), 'user-auth', ''::bytea, '{}');
INSERT INTO provisioner_keys (id, created_at, organization_id, name, hashed_secret, tags) VALUES ('33333333-3333-3333-3333-333333333333'::uuid, NOW(), (SELECT id FROM organizations WHERE is_default = true), 'psk', ''::bytea, '{}');

ALTER TABLE provisioner_daemons ADD COLUMN key_id UUID NOT NULL REFERENCES provisioner_keys(id) DEFAULT (SELECT id FROM provisioner_keys WHERE name = 'built-in');
ALTER TABLE provisioner_daemons ADD COLUMN key_id UUID REFERENCES provisioner_keys(id) ON DELETE CASCADE DEFAULT '11111111-1111-1111-1111-111111111111'::uuid NOT NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have external provisioners running today on some customer deployments. Will this be ok if they are assigned to the built in?

Should we assign the existing rows to the psk at 33333333-3333-3333-3333-33333333333, and have the startup code fix the built in rows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I did try to consider the outputs here and picked this one since it felt like the least impact, but let me know if you feel otherwise:

  • There's no data in the DB that lets us know if it's a built-in or PSK today
  • External provisioners will need to reconnect after a coderd upgrade anyways, and on reconnection they will be assigned the correct 333... id.
  • On the average deployment we would most likely see more built-ins than external, so if we have to pick one, built-in is probably less incorrect
  • I considered allowing null and not back porting the reference but that felt extra messy in the resulting queries and handler logic.

So basically with this migration the external provisioners will be incorrectly labeled as built-in until the provisioners reconnect. Because we only show recent provisioners the ones that do not connect again (and stay incorrectly labeled) will only be shown for a short while before being removed from the list we show users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth noting that right now this key reference is only for grouping in read operations, no mutation logic is performed based on this value today.

1 change: 1 addition & 0 deletions coderd/database/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 14 additions & 6 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions coderd/provisionerdserver/provisionerdserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1956,6 +1956,7 @@ func setup(t *testing.T, ignoreLogErrors bool, ov *overrides) (proto.DRPCProvisi
Version: buildinfo.Version(),
APIVersion: proto.CurrentVersion.String(),
OrganizationID: defOrg.ID,
KeyID: uuid.MustParse(codersdk.ProvisionerKeyIDBuiltIn),
})
require.NoError(t, err)

Expand Down
6 changes: 6 additions & 0 deletions codersdk/provisionerdaemons.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,12 @@ type ProvisionerKey struct {
// HashedSecret - never include the access token in the API response
}

const (
ProvisionerKeyIDBuiltIn = "11111111-1111-1111-1111-111111111111"
ProvisionerKeyIDUserAuth = "22222222-2222-2222-2222-222222222222"
ProvisionerKeyIDPSK = "33333333-3333-3333-3333-333333333333"
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will probably need to be hardcoded on the frontend as well, since this doesn't seem to get generated.

type CreateProvisionerKeyRequest struct {
Name string `json:"name"`
Tags map[string]string `json:"tags"`
Expand Down
1 change: 1 addition & 0 deletions docs/reference/api/debug.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions docs/reference/api/enterprise.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions docs/reference/api/schemas.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

63 changes: 38 additions & 25 deletions enterprise/coderd/provisionerdaemons.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,56 +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) (map[string]string, error) {
func (p *provisionerDaemonAuth) authorize(r *http.Request, orgID uuid.UUID, tags map[string]string) (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 nil, xerrors.New("no API key or provisioner key provided")
return uuid.Nil, nil, xerrors.New("no API key or provisioner key provided")
}
if apiKeyOK && pkOK {
return nil, xerrors.New("Both API key and provisioner key authentication provided. Only one is allowed.")
return 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 nil, xerrors.New("provisioner key unauthorized")
return uuid.Nil, nil, xerrors.New("provisioner key unauthorized")
}
if tags != nil && !maps.Equal(tags, map[string]string{}) {
return nil, xerrors.New("tags are not allowed when using a provisioner key")
return 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 tags, nil
return pk.ID, tags, nil
}

// User Auth
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 tags, nil
}
ua := httpmw.UserAuthorization(r)
err := p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(orgID))
if err != nil {
if !provAuth {
return nil, xerrors.New("user unauthorized")
if apiKeyOK {
userKey, err := uuid.Parse(codersdk.ProvisionerKeyIDUserAuth)
if err != nil {
return uuid.Nil, nil, xerrors.Errorf("parse user 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 tags, nil
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")
}

// 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 userKey, tags, nil
}
}

pskKey, err := uuid.Parse(codersdk.ProvisionerKeyIDPSK)
if err != nil {
return uuid.Nil, nil, xerrors.Errorf("parse psk provisioner key id: %w", err)
}

// User is allowed to create provisioner daemons
return tags, nil
// PSK Auth
return pskKey, tags, nil
}

// Serves the provisioner daemon protobuf API over a WebSocket.
Expand Down Expand Up @@ -194,7 +206,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
api.Logger.Warn(ctx, "unnamed provisioner daemon")
}

tags, err := api.provisionerDaemonAuth.authorize(r, organization.ID, tags)
keyID, tags, err := api.provisionerDaemonAuth.authorize(r, organization.ID, 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,
Expand Down Expand Up @@ -267,6 +279,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
Version: versionHdrVal,
APIVersion: apiVersion,
OrganizationID: organization.ID,
KeyID: keyID,
})
if err != nil {
if !xerrors.Is(err, context.Canceled) {
Expand Down
1 change: 1 addition & 0 deletions site/src/api/typesGenerated.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.