Skip to content

Commit 85e05c3

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Merge branch 'groups' of github.com:coder/coder into groups
2 parents 2c4fd8d + c2e1196 commit 85e05c3

File tree

12 files changed

+186
-55
lines changed

12 files changed

+186
-55
lines changed

‎agent/agent_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -481,19 +481,6 @@ func TestAgent(t *testing.T) {
481481
}
482482
})
483483

484-
t.Run("Tailnet", func(t *testing.T) {
485-
t.Parallel()
486-
derpMap := tailnettest.RunDERPAndSTUN(t)
487-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{
488-
DERPMap: derpMap,
489-
}, 0)
490-
defer conn.Close()
491-
require.Eventually(t, func() bool {
492-
_, err := conn.Ping()
493-
return err == nil
494-
}, testutil.WaitLong, testutil.IntervalFast)
495-
})
496-
497484
t.Run("Speedtest", func(t *testing.T) {
498485
t.Parallel()
499486
if testing.Short() {

‎cli/server.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,10 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
328328
}
329329

330330
defaultRegion := &tailcfg.DERPRegion{
331-
RegionID: derpServerRegionID,
332-
RegionCode: derpServerRegionCode,
333-
RegionName: derpServerRegionName,
331+
EmbeddedRelay: true,
332+
RegionID: derpServerRegionID,
333+
RegionCode: derpServerRegionCode,
334+
RegionName: derpServerRegionName,
334335
Nodes: []*tailcfg.DERPNode{{
335336
Name: fmt.Sprintf("%db", derpServerRegionID),
336337
RegionID: derpServerRegionID,

‎coderd/coderdtest/coderdtest.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,10 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance
196196
DERPMap: &tailcfg.DERPMap{
197197
Regions: map[int]*tailcfg.DERPRegion{
198198
1: {
199-
RegionID: 1,
200-
RegionCode: "coder",
201-
RegionName: "Coder",
199+
EmbeddedRelay: true,
200+
RegionID: 1,
201+
RegionCode: "coder",
202+
RegionName: "Coder",
202203
Nodes: []*tailcfg.DERPNode{{
203204
Name: "1a",
204205
RegionID: 1,

‎coderd/rbac/authz.go

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@ type PreparedAuthorized interface {
2323
}
2424

2525
// Filter takes in a list of objects, and will filter the list removing all
26-
// the elements the subject does not have permission for. This function slows
27-
// down if the list contains objects of multiple types. Attempt to only
28-
// filter objects of the same type for faster performance.
26+
// the elements the subject does not have permission for. All objects must be
27+
// of the same type.
2928
func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, scope Scope, groups []string, action Action, objects []O) ([]O, error) {
3029
ctx, span := tracing.StartSpan(ctx, trace.WithAttributes(
3130
attribute.String("subject_id", subjID),
@@ -38,26 +37,20 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub
3837
// Nothing to filter
3938
return objects, nil
4039
}
40+
objectType := objects[0].RBACObject().Type
4141

4242
filtered := make([]O, 0)
43-
prepared := make(map[string]PreparedAuthorized)
44-
45-
for i := range objects {
46-
object := objects[i]
47-
objectType := object.RBACObject().Type
48-
// objectAuth is the prepared authorization for the object type.
49-
objectAuth, ok := prepared[object.RBACObject().Type]
50-
if !ok {
51-
var err error
52-
objectAuth, err = auth.PrepareByRoleName(ctx, subjID, subjRoles, scope, groups, action, objectType)
53-
if err != nil {
54-
return nil, xerrors.Errorf("prepare: %w", err)
55-
}
56-
prepared[objectType] = objectAuth
57-
}
43+
prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, scope, groups, action, objectType)
44+
if err != nil {
45+
return nil, xerrors.Errorf("prepare: %w", err)
46+
}
5847

48+
for _, object := range objects {
5949
rbacObj := object.RBACObject()
60-
err := objectAuth.Authorize(ctx, rbacObj)
50+
if rbacObj.Type != objectType {
51+
return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, object.RBACObject().Type)
52+
}
53+
err := prepared.Authorize(ctx, rbacObj)
6154
if err == nil {
6255
filtered = append(filtered, object)
6356
}

‎coderd/rbac/authz_internal_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ func (w fakeObject) RBACObject() Object {
4040
}
4141

4242
// TODO: @emyrk Bring back this test when private/public templates are removed
43-
// in favor of groups.
44-
//func TestFilterError(t *testing.T) {
45-
// t.Parallel()
46-
// auth, err := NewAuthorizer()
47-
// require.NoError(t, err)
4843
//
49-
// _, err = Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace})
50-
// require.ErrorContains(t, err, "object types must be uniform")
51-
//}
44+
// in favor of groups.
45+
func TestFilterError(t *testing.T) {
46+
t.Parallel()
47+
auth := NewAuthorizer()
48+
49+
_, err := Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace})
50+
require.ErrorContains(t, err, "object types must be uniform")
51+
}
5252

5353
// TestFilter ensures the filter acts the same as an individual authorize.
5454
// It generates a random set of objects, then runs the Filter batch function

‎coderd/rbac/partial.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"golang.org/x/xerrors"
77

8+
"github.com/open-policy-agent/opa/ast"
89
"github.com/open-policy-agent/opa/rego"
910

1011
"github.com/coder/coder/coderd/tracing"
@@ -32,6 +33,18 @@ func (pa *PartialAuthorizer) Authorize(ctx context.Context, object Object) error
3233
return nil
3334
}
3435

36+
// No queries means always false
37+
if len(pa.preparedQueries) == 0 {
38+
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), pa.input, nil)
39+
}
40+
41+
parsed, err := ast.InterfaceToValue(map[string]interface{}{
42+
"object": object,
43+
})
44+
if err != nil {
45+
return xerrors.Errorf("parse object: %w", err)
46+
}
47+
3548
// How to interpret the results of the partial queries.
3649
// We have a list of queries that are along the lines of:
3750
// `input.object.org_owner = ""; "me" = input.object.owner`
@@ -45,9 +58,7 @@ func (pa *PartialAuthorizer) Authorize(ctx context.Context, object Object) error
4558
EachQueryLoop:
4659
for _, q := range pa.preparedQueries {
4760
// We need to eval each query with the newly known fields.
48-
results, err := q.Eval(ctx, rego.EvalInput(map[string]interface{}{
49-
"object": object,
50-
}))
61+
results, err := q.Eval(ctx, rego.EvalParsedInput(parsed))
5162
if err != nil {
5263
continue EachQueryLoop
5364
}

‎coderd/users.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1018,9 +1018,27 @@ func (api *API) postLogout(rw http.ResponseWriter, r *http.Request) {
10181018
}
10191019
http.SetCookie(rw, cookie)
10201020

1021+
// This code should be removed after Jan 1 2023.
1022+
// This code logs out of the old session cookie before we renamed it
1023+
// if it is a valid coder token. Otherwise, this old cookie hangs around
1024+
// and we never log out of the user.
1025+
oldCookie, err := r.Cookie("session_token")
1026+
if err == nil && oldCookie != nil {
1027+
_, _, err := httpmw.SplitAPIToken(oldCookie.Value)
1028+
if err == nil {
1029+
cookie := &http.Cookie{
1030+
// MaxAge < 0 means to delete the cookie now.
1031+
MaxAge: -1,
1032+
Name: "session_token",
1033+
Path: "/",
1034+
}
1035+
http.SetCookie(rw, cookie)
1036+
}
1037+
}
1038+
10211039
// Delete the session token from database.
10221040
apiKey := httpmw.APIKey(r)
1023-
err := api.Database.DeleteAPIKeyByID(ctx, apiKey.ID)
1041+
err = api.Database.DeleteAPIKeyByID(ctx, apiKey.ID)
10241042
if err != nil {
10251043
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
10261044
Message: "Internal error deleting API key.",

‎coderd/workspaceagents.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,44 @@ func (api *API) dialWorkspaceAgentTailnet(r *http.Request, agentID uuid.UUID) (*
226226
_ = serverConn.Close()
227227
}()
228228

229+
derpMap := api.DERPMap.Clone()
230+
for _, region := range derpMap.Regions {
231+
if !region.EmbeddedRelay {
232+
continue
233+
}
234+
var node *tailcfg.DERPNode
235+
for _, n := range region.Nodes {
236+
if n.STUNOnly {
237+
continue
238+
}
239+
node = n
240+
break
241+
}
242+
if node == nil {
243+
continue
244+
}
245+
// TODO: This should dial directly to execute the
246+
// DERP server instead of contacting localhost.
247+
//
248+
// This requires modification of Tailscale internals
249+
// to pipe through a proxy function per-region, so
250+
// this is an easy and mostly reliable hack for now.
251+
cloned := node.Clone()
252+
// Add p for proxy.
253+
// This first node supports TLS.
254+
cloned.Name += "p"
255+
cloned.IPv4 = "127.0.0.1"
256+
cloned.InsecureForTests = true
257+
region.Nodes = append(region.Nodes, cloned.Clone())
258+
// This second node forces HTTP.
259+
cloned.Name += "-http"
260+
cloned.ForceHTTP = true
261+
region.Nodes = append(region.Nodes, cloned)
262+
}
263+
229264
conn, err := tailnet.NewConn(&tailnet.Options{
230265
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
231-
DERPMap: api.DERPMap,
266+
DERPMap: derpMap,
232267
Logger: api.Logger.Named("tailnet").Leveled(slog.LevelDebug),
233268
})
234269
if err != nil {

‎codersdk/workspaceagents.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/http"
1111
"net/http/cookiejar"
1212
"net/netip"
13+
"strconv"
1314
"time"
1415

1516
"cloud.google.com/go/compute/metadata"
@@ -208,7 +209,42 @@ func (c *Client) WorkspaceAgentMetadata(ctx context.Context) (WorkspaceAgentMeta
208209
return WorkspaceAgentMetadata{}, readBodyAsError(res)
209210
}
210211
var agentMetadata WorkspaceAgentMetadata
211-
return agentMetadata, json.NewDecoder(res.Body).Decode(&agentMetadata)
212+
err = json.NewDecoder(res.Body).Decode(&agentMetadata)
213+
if err != nil {
214+
return WorkspaceAgentMetadata{}, err
215+
}
216+
accessingPort := c.URL.Port()
217+
if accessingPort == "" {
218+
accessingPort = "80"
219+
if c.URL.Scheme == "https" {
220+
accessingPort = "443"
221+
}
222+
}
223+
accessPort, err := strconv.Atoi(accessingPort)
224+
if err != nil {
225+
return WorkspaceAgentMetadata{}, xerrors.Errorf("convert accessing port %q: %w", accessingPort, err)
226+
}
227+
// Agents can provide an arbitrary access URL that may be different
228+
// that the globally configured one. This breaks the built-in DERP,
229+
// which would continue to reference the global access URL.
230+
//
231+
// This converts all built-in DERPs to use the access URL that the
232+
// metadata request was performed with.
233+
for _, region := range agentMetadata.DERPMap.Regions {
234+
if !region.EmbeddedRelay {
235+
continue
236+
}
237+
238+
for _, node := range region.Nodes {
239+
if node.STUNOnly {
240+
continue
241+
}
242+
node.HostName = c.URL.Hostname()
243+
node.DERPPort = accessPort
244+
node.ForceHTTP = c.URL.Scheme == "http"
245+
}
246+
}
247+
return agentMetadata, nil
212248
}
213249

214250
func (c *Client) ListenWorkspaceAgentTailnet(ctx context.Context) (net.Conn, error) {

‎codersdk/workspaceagents_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package codersdk_test
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/url"
8+
"strconv"
9+
"testing"
10+
11+
"github.com/stretchr/testify/require"
12+
"tailscale.com/tailcfg"
13+
14+
"github.com/coder/coder/coderd/httpapi"
15+
"github.com/coder/coder/codersdk"
16+
)
17+
18+
func TestWorkspaceAgentMetadata(t *testing.T) {
19+
t.Parallel()
20+
// This test ensures that the DERP map returned properly
21+
// mutates built-in DERPs with the client access URL.
22+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
23+
httpapi.Write(context.Background(), w, http.StatusOK, codersdk.WorkspaceAgentMetadata{
24+
DERPMap: &tailcfg.DERPMap{
25+
Regions: map[int]*tailcfg.DERPRegion{
26+
1: {
27+
EmbeddedRelay: true,
28+
RegionID: 1,
29+
Nodes: []*tailcfg.DERPNode{{
30+
HostName: "bananas.org",
31+
DERPPort: 1,
32+
}},
33+
},
34+
},
35+
},
36+
})
37+
}))
38+
parsed, err := url.Parse(srv.URL)
39+
require.NoError(t, err)
40+
client := codersdk.New(parsed)
41+
metadata, err := client.WorkspaceAgentMetadata(context.Background())
42+
require.NoError(t, err)
43+
region := metadata.DERPMap.Regions[1]
44+
require.True(t, region.EmbeddedRelay)
45+
require.Len(t, region.Nodes, 1)
46+
node := region.Nodes[0]
47+
require.Equal(t, parsed.Hostname(), node.HostName)
48+
require.Equal(t, parsed.Port(), strconv.Itoa(node.DERPPort))
49+
}

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
4040

4141
// There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
4242
// https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
43-
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220914175845-85b85d9a52ee
43+
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220926024748-50f068456c6c
4444

4545
// Switch to our fork that imports fixes from http://github.com/tailscale/ssh.
4646
// See: https://github.com/coder/coder/issues/3371

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,8 @@ github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY=
349349
github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY=
350350
github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko=
351351
github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
352-
github.com/coder/tailscale v1.1.1-0.20220914175845-85b85d9a52ee h1:77WUcIAL5FRQtd97/gOV66MJHLPhsPw+3vMxoEvcadI=
353-
github.com/coder/tailscale v1.1.1-0.20220914175845-85b85d9a52ee/go.mod h1:5amxy08qijEa8bcTW2SeIy4MIqcmd7LMsuOxqOlj2Ak=
352+
github.com/coder/tailscale v1.1.1-0.20220926024748-50f068456c6c h1:xa6lr5Pj87Is26tgpzwBsEGKL7aVz7/fRGgY9QIbf3E=
353+
github.com/coder/tailscale v1.1.1-0.20220926024748-50f068456c6c/go.mod h1:5amxy08qijEa8bcTW2SeIy4MIqcmd7LMsuOxqOlj2Ak=
354354
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=
355355
github.com/containerd/aufs v0.0.0-20201003224125-76a6863f2989/go.mod h1:AkGGQs9NM2vtYHaUen+NljV0/baGCAPELGm2q9ZXpWU=
356356
github.com/containerd/aufs v0.0.0-20210316121734-20793ff83c97/go.mod h1:kL5kd6KM5TzQjR79jljyi4olc1Vrx6XBlcyj3gNv2PU=

0 commit comments

Comments
 (0)