Skip to content

Commit bc9cfb8

Browse files
ThomasK33ThomasK33
committed
review improvements
Change-Id: I7868299308cdf4fca36092025bf2692cfe652640 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent b5ed0bd commit bc9cfb8

17 files changed

+74
-110
lines changed

‎cli/testdata/coder_provisioner_list_--output_json.golden

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"last_seen_at": "====[timestamp]=====",
88
"name": "test",
99
"version": "v0.0.0-devel",
10-
"api_version": "1.4",
10+
"api_version": "1.5",
1111
"provisioners": [
1212
"echo"
1313
],

‎coderd/coderd.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -804,11 +804,6 @@ func New(options *Options) *API {
804804
DB: options.Database,
805805
Optional: false,
806806
})
807-
// Same as above but optional
808-
workspaceAgentInfoOptional := httpmw.ExtractWorkspaceAgentAndLatestBuild(httpmw.ExtractWorkspaceAgentAndLatestBuildConfig{
809-
DB: options.Database,
810-
Optional: true,
811-
})
812807

813808
// API rate limit middleware. The counter is local and not shared between
814809
// replicas or instances of this middleware.
@@ -1029,7 +1024,6 @@ func New(options *Options) *API {
10291024
r.Route("/external-auth", func(r chi.Router) {
10301025
r.Use(
10311026
apiKeyMiddleware,
1032-
workspaceAgentInfoOptional,
10331027
)
10341028
// Get without a specific external auth ID will return all external auths.
10351029
r.Get("/", api.listUserExternalAuths)
@@ -1265,12 +1259,8 @@ func New(options *Options) *API {
12651259
r.Get("/", api.workspaceByOwnerAndName)
12661260
r.Get("/builds/{buildnumber}", api.workspaceBuildByBuildNumber)
12671261
})
1268-
r.With(
1269-
workspaceAgentInfoOptional,
1270-
).Get("/gitsshkey", api.gitSSHKey)
1271-
r.With(
1272-
workspaceAgentInfoOptional,
1273-
).Put("/gitsshkey", api.regenerateGitSSHKey)
1262+
r.Get("/gitsshkey", api.gitSSHKey)
1263+
r.Put("/gitsshkey", api.regenerateGitSSHKey)
12741264
r.Route("/notifications", func(r chi.Router) {
12751265
r.Route("/preferences", func(r chi.Router) {
12761266
r.Get("/", api.userNotificationPreferences)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3988,7 +3988,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
39883988
check.Args(database.InsertWorkspaceAgentParams{
39893989
ID: uuid.New(),
39903990
Name: "dev",
3991-
APIKeyScope: database.ApiKeyScopeEnumDefault,
3991+
APIKeyScope: database.AgentKeyScopeEnumAll,
39923992
}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
39933993
}))
39943994
s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) {

‎coderd/database/dbgen/dbgen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func WorkspaceAgent(t testing.TB, db database.Store, orig database.WorkspaceAgen
210210
MOTDFile: takeFirst(orig.TroubleshootingURL, ""),
211211
DisplayApps: append([]database.DisplayApp{}, orig.DisplayApps...),
212212
DisplayOrder: takeFirst(orig.DisplayOrder, 1),
213-
APIKeyScope: takeFirst(orig.APIKeyScope, database.ApiKeyScopeEnumDefault),
213+
APIKeyScope: takeFirst(orig.APIKeyScope, database.AgentKeyScopeEnumAll),
214214
})
215215
require.NoError(t, err, "insert workspace agent")
216216
return agt

‎coderd/database/dump.sql

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000320_add_api_key_scope_to_workspace_agents.down.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ ALTER TABLE workspace_agents
33
DROP COLUMN IF EXISTS api_key_scope;
44

55
-- Drop the enum type for API key scope
6-
DROP TYPE IF EXISTS api_key_scope_enum;
6+
DROP TYPE IF EXISTS agent_key_scope_enum;

‎coderd/database/migrations/000320_add_api_key_scope_to_workspace_agents.up.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
-- Create the enum type for API key scope
2-
CREATE TYPE api_key_scope_enum AS ENUM ('default', 'no_user_data');
2+
CREATE TYPE agent_key_scope_enum AS ENUM ('all', 'no_user_data');
33

44
-- Add the api_key_scope column to the workspace_agents table
5-
-- It defaults to 'default' to maintain existing behavior for current agents.
5+
-- It defaults to 'all' to maintain existing behavior for current agents.
66
ALTER TABLE workspace_agents
7-
ADD COLUMN api_key_scope api_key_scope_enum NOT NULL DEFAULT 'default';
7+
ADD COLUMN api_key_scope agent_key_scope_enum NOT NULL DEFAULT 'all';
88

99
-- Add a comment explaining the purpose of the column
10-
COMMENT ON COLUMN workspace_agents.api_key_scope IS 'Defines the scope of the API key associated with the agent. ''default'' allows access to everything, ''no_user_data'' restricts it to exclude user data.';
10+
COMMENT ON COLUMN workspace_agents.api_key_scope IS 'Defines the scope of the API key associated with the agent. ''all'' allows access to everything, ''no_user_data'' restricts it to exclude user data.';

‎coderd/database/models.go

Lines changed: 24 additions & 24 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/externalauth_test.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ func TestExternalAuthCallback(t *testing.T) {
713713
apiKeyScope string
714714
expectsError bool
715715
}{
716-
{apiKeyScope: "default", expectsError: false},
716+
{apiKeyScope: "all", expectsError: false},
717717
{apiKeyScope: "no_user_data", expectsError: true},
718718
} {
719719
t.Run(tt.apiKeyScope, func(t *testing.T) {
@@ -746,6 +746,15 @@ func TestExternalAuthCallback(t *testing.T) {
746746
token, err := agentClient.ExternalAuth(t.Context(), agentsdk.ExternalAuthRequest{
747747
Match: "github.com/asd/asd",
748748
})
749+
750+
if tt.expectsError {
751+
require.Error(t, err)
752+
var sdkErr *codersdk.Error
753+
require.ErrorAs(t, err, &sdkErr)
754+
require.Equal(t, http.StatusForbidden, sdkErr.StatusCode())
755+
return
756+
}
757+
749758
require.NoError(t, err)
750759
require.NotEmpty(t, token.URL)
751760

@@ -756,13 +765,8 @@ func TestExternalAuthCallback(t *testing.T) {
756765
Match: "github.com/asd/asd",
757766
Listen: true,
758767
})
759-
if tt.expectsError {
760-
assert.Error(t, err)
761-
close(tokenChan)
762-
} else {
763-
assert.NoError(t, err)
764-
tokenChan <- token
765-
}
768+
assert.NoError(t, err)
769+
tokenChan <- token
766770
}()
767771

768772
time.Sleep(250 * time.Millisecond)
@@ -771,9 +775,6 @@ func TestExternalAuthCallback(t *testing.T) {
771775
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
772776

773777
token = <-tokenChan
774-
if tt.expectsError {
775-
return
776-
}
777778
require.Equal(t, "access_token", token.Username)
778779

779780
token, err = agentClient.ExternalAuth(t.Context(), agentsdk.ExternalAuthRequest{

0 commit comments

Comments
 (0)