8000 Merge branch 'main' of github.com:/coder/coder into dk/logreplacements · coder/coder@11a2c5a · GitHub
[go: up one dir, main page]
Skip to content

Commit 11a2c5a

Browse files
dannykoppingdannykopping
committed
Merge branch 'main' of github.com:/coder/coder into dk/logreplacements
2 parents 70f9a53 + bd65914 commit 11a2c5a

File tree

81 files changed

+1641
-795
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+1641
-795
lines changed

‎.github/workflows/weekly-docs.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ jobs:
3636
reporter: github-pr-review
3737
config_file: ".github/.linkspector.yml"
3838
fail_on_error: "true"
39-
filter_mode: "nofilter"
39+
filter_mode: "file"
4040

4141
- name: Send Slack notification
4242
if: failure() && github.event_name == 'schedule'

‎cli/configssh.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,11 @@ func (r *RootCmd) configSSH() *serpent.Command {
440440
}
441441

442442
if !bytes.Equal(configRaw, configModified) {
443+
sshDir := filepath.Dir(sshConfigFile)
444+
if err := os.MkdirAll(sshDir, 0700); err != nil {
445+
return xerrors.Errorf("failed to create directory %q: %w", sshDir, err)
446+
}
447+
443448
err = atomic.WriteFile(sshConfigFile, bytes.NewReader(configModified))
444449
if err != nil {
445450
return xerrors.Errorf("write ssh config failed: %w", err)

‎cli/configssh_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,47 @@ func TestConfigSSH(t *testing.T) {
169169
<-copyDone
170170
}
171171

172+
func TestConfigSSH_MissingDirectory(t *testing.T) {
173+
t.Parallel()
174+
175+
if runtime.GOOS == "windows" {
176+
t.Skip("See coder/internal#117")
177+
}
178+
179+
client := coderdtest.New(t, nil)
180+
_ = coderdtest.CreateFirstUser(t, client)
181+
182+
// Create a temporary directory but don't create .ssh subdirectory
183+
tmpdir := t.TempDir()
184+
sshConfigPath := filepath.Join(tmpdir, ".ssh", "config")
185+
186+
// Run config-ssh with a non-existent .ssh directory
187+
args := []string{
188+
"config-ssh",
189+
"--ssh-config-file", sshConfigPath,
190+
"--yes", // Skip confirmation prompts
191+
}
192+
inv, root := clitest.New(t, args...)
193+
clitest.SetupConfig(t, client, root)
194+
195+
err := inv.Run()
196+
require.NoError(t, err, "config-ssh should succeed with non-existent directory")
197+
198+
// Verify that the .ssh directory was created
199+
sshDir := filepath.Dir(sshConfigPath)
200+
_, err = os.Stat(sshDir)
201+
require.NoError(t, err, ".ssh directory should exist")
202+
203+
// Verify that the config file was created
204+
_, err = os.Stat(sshConfigPath)
205+
require.NoError(t, err, "config file should exist")
206+
207+
// Check that the directory has proper permissions (0700)
208+
sshDirInfo, err := os.Stat(sshDir)
209+
require.NoError(t, err)
210+
require.Equal(t, os.FileMode(0700), sshDirInfo.Mode().Perm(), "directory should have 0700 permissions")
211+
}
212+
172213
func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
173214
t.Parallel()
174215

‎cli/update_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ func TestUpdateValidateRichParameters(t *testing.T) {
757757
err := inv.Run()
758758
// TODO: improve validation so we catch this problem before it reaches the server
759759
// but for now just validate that the server actually catches invalid monotonicity
760-
assert.ErrorContains(t, err, fmt.Sprintf("parameter value must be equal or greater than previous value: %s", tempVal))
760+
assert.ErrorContains(t, err, "parameter value '1' must be equal or greater than previous value: 2")
761761
}()
762762

763763
matches := []string{

‎coderd/coderd.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,26 +1189,32 @@ func New(options *Options) *API {
11891189
})
11901190
r.Route("/{user}", func(r chi.Router) {
11911191
r.Group(func(r chi.Router) {
1192-
r.Use(httpmw.ExtractUserParamOptional(options.Database))
1192+
r.Use(httpmw.ExtractOrganizationMembersParam(options.Database, api.HTTPAuth.Authorize))
11931193
// Creating workspaces does not require permissions on the user, only the
11941194
// organization member. This endpoint should match the authz story of
11951195
// postWorkspacesByOrganization
11961196
r.Post("/workspaces", api.postUserWorkspaces)
1197+
r.Route("/workspace/{workspacename}", func(r chi.Router) {
1198+
r.Get("/", api.workspaceByOwnerAndName)
1199+
r.Get("/builds/{buildnumber}", api.workspaceBuildByBuildNumber)
1200+
})
1201+
})
1202+
1203+
r.Group(func(r chi.Router) {
1204+
r.Use(httpmw.ExtractUserParam(options.Database))
11971205

11981206
// Similarly to creating a workspace, evaluating parameters for a
11991207
// new workspace should also match the authz story of
12001208
// postWorkspacesByOrganization
1209+
// TODO: Do not require site wide read user permission. Make this work
1210+
// with org member permissions.
12011211
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
12021212
r.Use(
12031213
httpmw.ExtractTemplateVersionParam(options.Database),
12041214
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters),
12051215
)
12061216
r.Get("/parameters", api.templateVersionDynamicParameters)
12071217
})
1208-
})
1209-
1210-
r.Group(func(r chi.Router) {
1211-
r.Use(httpmw.ExtractUserParam(options.Database))
12121218

12131219
r.Post("/convert-login", api.postConvertLoginType)
12141220
r.Delete("/", api.deleteUser)
@@ -1250,10 +1256,7 @@ func New(options *Options) *API {
12501256
r.Get("/", api.organizationsByUser)
12511257
r.Get("/{organizationname}", api.organizationByUserAndName)
12521258
})
1253-
r.Route("/workspace/{workspacename}", func(r chi.Router) {
1254-
r.Get("/", api.workspaceByOwnerAndName)
1255-
r.Get("/builds/{buildnumber}", api.workspaceBuildByBuildNumber)
1256-
})
1259+
12571260
r.Get("/gitsshkey", api.gitSSHKey)
12581261
r.Put("/gitsshkey", api.regenerateGitSSHKey)
12591262
r.Route("/notifications", func(r chi.Router) {

‎coderd/httpmw/organizationparam.go

Lines changed: 128 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@ import (
1111
"github.com/coder/coder/v2/coderd/database"
1212
"github.com/coder/coder/v2/coderd/database/dbauthz"
1313
"github.com/coder/coder/v2/coderd/httpapi"
14+
"github.com/coder/coder/v2/coderd/rbac"
15+
"github.com/coder/coder/v2/coderd/rbac/policy"
1416
"github.com/coder/coder/v2/codersdk"
1517
)
1618

1719
type (
18-
organizationParamContextKey struct{}
19-
organizationMemberParamContextKey struct{}
20+
organizationParamContextKey struct{}
21+
organizationMemberParamContextKey struct{}
22+
organizationMembersParamContextKey struct{}
2023
)
2124

2225
// OrganizationParam returns the organization from the ExtractOrganizationParam handler.
@@ -38,6 +41,14 @@ func OrganizationMemberParam(r *http.Request) OrganizationMember {
3841
return organizationMember
3942
}
4043

44+
func OrganizationMembersParam(r *http.Request) OrganizationMembers {
45+
organizationMembers, ok := r.Context().Value(organizationMembersParamContextKey{}).(OrganizationMembers)
46+
if !ok {
47+
panic("developer error: organization members param middleware not provided")
48+
}
49+
return organizationMembers
50+
}
51+
4152
// ExtractOrganizationParam grabs an organization from the "organization" URL parameter.
4253
// This middleware requires the API key middleware higher in the call stack for authentication.
4354
func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler {
@@ -111,35 +122,23 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
111122
return func(next http.Handler) http.Handler {
112123
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
113124
ctx := r.Context()
114-
// We need to resolve the `{user}` URL parameter so that we can get the userID and
115-
// username. We do this as SystemRestricted since the caller might have permission
116-
// to access the OrganizationMember object, but *not* the User object. So, it is
117-
// very important that we do not add the User object to the request context or otherwise
118-
// leak it to the API handler.
119-
// nolint:gocritic
120-
user, ok := ExtractUserContext(dbauthz.AsSystemRestricted(ctx), db, rw, r)
121-
if !ok {
122-
return
123-
}
124125
organization := OrganizationParam(r)
125-
126-
organizationMember, err := database.ExpectOne(db.OrganizationMembers(ctx, database.OrganizationMembersParams{
127-
OrganizationID: organization.ID,
128-
UserID: user.ID,
129-
IncludeSystem: false,
130-
}))
131-
if httpapi.Is404Error(err) {
132-
httpapi.ResourceNotFound(rw)
126+
_, members, done := ExtractOrganizationMember(ctx, nil, rw, r, db, organization.ID)
127+
if done {
133128
return
134129
}
135-
if err != nil {
130+
131+
if len(members) != 1 {
136132
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
137133
Message: "Internal error fetching organization member.",
138-
Detail: err.Error(),
134+
// This is a developer error and should never happen.
135+
Detail: fmt.Sprintf("Expected exactly one organization member, but got %d.", len(members)),
139136
})
140137
return
141138
}
142139

140+
organizationMember := members[0]
141+
143142
ctx = context.WithValue(ctx, organizationMemberParamContextKey{}, OrganizationMember{
144143
OrganizationMember: organizationMember.OrganizationMember,
145144
// Here we're making two exceptions to the rule about not leaking data about the user
@@ -151,8 +150,113 @@ func ExtractOrganizationMemberParam(db database.Store) func(http.Handler) http.H
151150
// API handlers need this information for audit logging and returning the owner's
152151
// username in response to creating a workspace. Additionally, the frontend consumes
153152
// the Avatar URL and this allows the FE to avoid an extra request.
154-
Username: user.Username,
155-
AvatarURL: user.AvatarURL,
153+
Username: organizationMember.Username,
154+
AvatarURL: organizationMember.AvatarURL,
155+
})
156+
157+
next.ServeHTTP(rw, r.WithContext(ctx))
158+
})
159+
}
160+
}
161+
162+
// ExtractOrganizationMember extracts all user memberships from the "user" URL
163+
// parameter. If orgID is uuid.Nil, then it will return all memberships for the
164+
// user, otherwise it will only return memberships to the org.
165+
//
166+
// If `user` is returned, that means the caller can use the data. This is returned because
167+
// it is possible to have a user with 0 organizations. So the user != nil, with 0 memberships.
168+
func ExtractOrganizationMember(ctx context.Context, auth func(r *http.Request, action policy.Action, object rbac.Objecter) bool, rw http.ResponseWriter, r *http.Request, db database.Store, orgID uuid.UUID) (*database.User, []database.OrganizationMembersRow, bool) {
169+
// We need to resolve the `{user}` URL parameter so that we can get the userID and
170+
// username. We do this as SystemRestricted since the caller might have permission
171+
// to access the OrganizationMember object, but *not* the User object. So, it is
172+
// very important that we do not add the User object to the request context or otherwise
173+
// leak it to the API handler.
174+
// nolint:gocritic
175+
user, ok := ExtractUserContext(dbauthz.AsSystemRestricted(ctx), db, rw, r)
176+
if !ok {
177+
return nil, nil, true
178+
}
179+
180+
organizationMembers, err := db.OrganizationMembers(ctx, database.OrganizationMembersParams{
181+
OrganizationID: orgID,
182+
UserID: user.ID,
183+
IncludeSystem: false,
184+
})
185+
if httpapi.Is404Error(err) {
186+
httpapi.ResourceNotFound(rw)
187+
return nil, nil, true
188+
}
189+
if err != nil {
190+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
191+
Message: "Internal error fetching organization member.",
192+
Detail: err.Error(),
193+
})
194+
return nil, nil, true
195+
}
196+
197+
// Only return the user data if the caller can read the user object.
198+
if auth != nil && auth(r, policy.ActionRead, user) {
199+
return &user, organizationMembers, false
200+
}
201+
202+
// If the user cannot be read and 0 memberships exist, throw a 404 to not
203+
// leak the user existence.
204+
if len(organizationMembers) == 0 {
205+
httpapi.ResourceNotFound(rw)
206+
return nil, nil, true
207+
}
208+
209+
return nil, organizationMembers, false
210+
}
211+
212+
type OrganizationMembers struct {
213+
// User is `nil` if the caller is not allowed access to the site wide
214+
// user object.
215+
User *database.User
216+
// Memberships can only be length 0 if `user != nil`. If `user == nil`, then
217+
// memberships will be at least length 1.
218+
Memberships []OrganizationMember
219+
}
220+
221+
func (om OrganizationMembers) UserID() uuid.UUID {
222+
if om.User != nil {
223+
return om.User.ID
224+
}
225+
226+
if len(om.Memberships) > 0 {
227+
return om.Memberships[0].UserID
228+
}
229+
return uuid.Nil
230+
}
231+
232+
// ExtractOrganizationMembersParam grabs all user organization memberships.
233+
// Only requires the "user" URL parameter.
234+
//
235+
// Use this if you want to grab as much information for a user as you can.
236+
// From an organization context, site wide user information might not available.
237+
func ExtractOrganizationMembersParam(db database.Store, auth func(r *http.Request, action policy.Action, object rbac.Objecter) bool) func(http.Handler) http.Handler {
238+
return func(next http.Handler) http.Handler {
239+
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
240+
ctx := r.Context()
241+
242+
// Fetch all memberships
243+
user, members, done := ExtractOrganizationMember(ctx, auth, rw, r, db, uuid.Nil)
244+
if done {
245+
return
246+
}
247+
248+
orgMembers := make([]OrganizationMember, 0, len(members))
249+
for _, organizationMember := range members {
250+
orgMembers = append(orgMembers, OrganizationMember{
251+
OrganizationMember: organizationMember.OrganizationMember,
252+
Username: organizationMember.Username,
253+
AvatarURL: organizationMember.AvatarURL,
254+
})
255+
}
256+
257+
ctx = context.WithValue(ctx, organizationMembersParamContextKey{}, OrganizationMembers{
258+
User: user,
259+
Memberships: orgMembers,
156260
})
157261
next.ServeHTTP(rw, r.WithContext(ctx))
158262
})

‎coderd/httpmw/organizationparam_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"github.com/coder/coder/v2/coderd/database/dbmem"
1717
"github.com/coder/coder/v2/coderd/database/dbtime"
1818
"github.com/coder/coder/v2/coderd/httpmw"
19+
"github.com/coder/coder/v2/coderd/rbac"
20+
"github.com/coder/coder/v2/coderd/rbac/policy"
1921
"github.com/coder/coder/v2/codersdk"
2022
"github.com/coder/coder/v2/testutil"
2123
)
@@ -167,6 +169,10 @@ func TestOrganizationParam(t *testing.T) {
167169
httpmw.ExtractOrganizationParam(db),
168170
httpmw.ExtractUserParam(db),
169171
httpmw.ExtractOrganizationMemberParam(db),
172+
httpmw.ExtractOrganizationMembersParam(db, func(r *http.Request, _ policy.Action, _ rbac.Objecter) bool {
173+
// Assume the caller cannot read the member
174+
return false
175+
}),
170176
)
171177
rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) {
172178
org := httpmw.OrganizationParam(r)
@@ -190,6 +196,11 @@ func TestOrganizationParam(t *testing.T) {
190196
assert.NotEmpty(t, orgMem.OrganizationMember.UpdatedAt)
191197
assert.NotEmpty(t, orgMem.OrganizationMember.UserID)
192198
assert.NotEmpty(t, orgMem.OrganizationMember.Roles)
199+
200+
orgMems := httpmw.OrganizationMembersParam(r)
201+
assert.NotZero(t, orgMems)
202+
assert.Equal(t, orgMem.UserID, orgMems.Memberships[0].UserID)
203+
assert.Nil(t, orgMems.User, "user data should not be available, hard coded false authorize")
193204
})
194205

195206
// Try by ID

0 commit comments

Comments
 (0)
0