Skip to content

Commit db04d67

Browse files
EmyrkEmyrk
committed
Fix broken tests
1 parent b831260 commit db04d67

File tree

5 files changed

+46
-28
lines changed

5 files changed

+46
-28
lines changed

‎coderd/coderd_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@ package coderd_test
22

33
import (
44
"context"
5+
"fmt"
6+
"net/http"
57
"testing"
68

9+
"github.com/coder/coder/coderd"
10+
"github.com/go-chi/chi/v5"
11+
712
"go.uber.org/goleak"
813

914
"github.com/stretchr/testify/require"
@@ -24,3 +29,12 @@ func TestBuildInfo(t *testing.T) {
2429
require.Equal(t, buildinfo.ExternalURL(), buildInfo.ExternalURL, "external URL")
2530
require.Equal(t, buildinfo.Version(), buildInfo.Version, "version")
2631
}
32+
33+
func TestWalk(t *testing.T) {
34+
r, _ := coderd.New(&coderd.Options{})
35+
chiRouter := r.(chi.Router)
36+
chi.Walk(chiRouter, func(method string, route string, handler http.Handler, middlewares ...func(http.Handler) http.Handler) error {
37+
fmt.Println(method, route)
38+
return nil
39+
})
40+
}

‎coderd/rbac/builtin.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ func IsOrgRole(roleName string) (string, bool) {
151151
// the list from the builtins.
152152
func OrganizationRoles(organizationID uuid.UUID) []string {
153153
var roles []string
154-
for role := range builtInRoles {
154+
for _, roleF := range builtInRoles {
155+
role := roleF(organizationID.String()).Name
155156
_, scope, err := roleSplit(role)
156157
if err != nil {
157158
// This should never happen

‎coderd/roles.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package coderd
33
import (
44
"net/http"
55

6+
"github.com/coder/coder/coderd/httpmw"
7+
68
"github.com/coder/coder/coderd/httpapi"
79
"github.com/coder/coder/coderd/rbac"
810
)
@@ -19,6 +21,7 @@ func (api *api) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
1921
func (api *api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
2022
// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
2123
// role of the user.
22-
roles := rbac.SiteRoles()
24+
organization := httpmw.OrganizationParam(r)
25+
roles := rbac.OrganizationRoles(organization.ID)
2326
httpapi.Write(rw, http.StatusOK, roles)
2427
}

‎coderd/roles_test.go

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,12 @@ import (
88
"github.com/coder/coder/coderd/coderdtest"
99
"github.com/coder/coder/coderd/rbac"
1010
"github.com/coder/coder/codersdk"
11-
"github.com/google/uuid"
1211
"github.com/stretchr/testify/require"
1312
)
1413

1514
func TestListRoles(t *testing.T) {
1615
t.Parallel()
1716

18-
requireUnauthorized := func(t *testing.T, err error) {
19-
var apiErr *codersdk.Error
20-
require.ErrorAs(t, err, &apiErr)
21-
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
22-
require.Contains(t, apiErr.Message, "unauthorized")
23-
}
24-
2517
ctx := context.Background()
2618
client := coderdtest.New(t, nil)
2719
// Create admin, member, and org admin
@@ -41,73 +33,78 @@ func TestListRoles(t *testing.T) {
4133
)
4234
require.NoError(t, err, "update org member roles")
4335

36+
otherOrg, err := client.CreateOrganization(ctx, admin.UserID, codersdk.CreateOrganizationRequest{
37+
Name: "other",
38+
})
39+
require.NoError(t, err, "create org")
40+
41+
const unauth = "unauthorized"
42+
const notMember = "not a member of the organization"
43+
4444
testCases := []struct {
45-
Name string
46-
Client *codersdk.Client
47-
APICall func() ([]string, error)
48-
ExpectedRoles []string
49-
Authorized bool
45+
Name string
46+
Client *codersdk.Client
47+
APICall func() ([]string, error)
48+
ExpectedRoles []string
49+
AuthorizedError string
5050
}{
5151
{
5252
Name: "MemberListSite",
5353
APICall: func() ([]string, error) {
5454
x, err := member.ListSiteRoles(ctx)
5555
return x, err
5656
},
57-
Authorized: false,
57+
AuthorizedError: unauth,
5858
},
5959
{
6060
Name: "OrgMemberListOrg",
6161
APICall: func() ([]string, error) {
6262
return member.ListOrganizationRoles(ctx, admin.OrganizationID)
6363
},
64-
Authorized: false,
64+
AuthorizedError: unauth,
6565
},
6666
{
6767
Name: "NonOrgMemberListOrg",
6868
APICall: func() ([]string, error) {
69-
return member.ListOrganizationRoles(ctx, uuid.New())
69+
return member.ListOrganizationRoles(ctx, otherOrg.ID)
7070
},
71-
Authorized: false,
71+
AuthorizedError: notMember,
7272
},
7373
// Org admin
7474
{
7575
Name: "OrgAdminListSite",
7676
APICall: func() ([]string, error) {
7777
return orgAdmin.ListSiteRoles(ctx)
7878
},
79-
Authorized: false,
79+
AuthorizedError: unauth,
8080
},
8181
{
8282
Name: "OrgAdminListOrg",
8383
APICall: func() ([]string, error) {
8484
return orgAdmin.ListOrganizationRoles(ctx, admin.OrganizationID)
8585
},
86-
Authorized: true,
8786
ExpectedRoles: rbac.OrganizationRoles(admin.OrganizationID),
8887
},
8988
{
9089
Name: "OrgAdminListOtherOrg",
9190
APICall: func() ([]string, error) {
92-
return orgAdmin.ListOrganizationRoles(ctx, uuid.New())
91+
return orgAdmin.ListOrganizationRoles(ctx, otherOrg.ID)
9392
},
94-
Authorized: false,
93+
AuthorizedError: notMember,
9594
},
9695
// Admin
9796
{
9897
Name: "AdminListSite",
9998
APICall: func() ([]string, error) {
10099
return client.ListSiteRoles(ctx)
101100
},
102-
Authorized: true,
103101
ExpectedRoles: rbac.SiteRoles(),
104102
},
105103
{
106104
Name: "AdminListOrg",
107105
APICall: func() ([]string, error) {
108106
return client.ListOrganizationRoles(ctx, admin.OrganizationID)
109107
},
110-
Authorized: true,
111108
ExpectedRoles: rbac.OrganizationRoles(admin.OrganizationID),
112109
},
113110
}
@@ -117,8 +114,11 @@ func TestListRoles(t *testing.T) {
117114
t.Run(c.Name, func(t *testing.T) {
118115
t.Parallel()
119116
roles, err := c.APICall()
120-
if !c.Authorized {
121-
requireUnauthorized(t, err)
117+
if c.AuthorizedError != "" {
118+
var apiErr *codersdk.Error
119+
require.ErrorAs(t, err, &apiErr)
120+
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
121+
require.Contains(t, apiErr.Message, c.AuthorizedError)
122122
} else {
123123
require.NoError(t, err)
124124
require.Equal(t, c.ExpectedRoles, roles)

‎codersdk/roles.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func (c *Client) ListSiteRoles(ctx context.Context) ([]string, error) {
2323
}
2424

2525
func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]string, error) {
26-
res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/members/roles", org.String()), nil)
26+
res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/members/roles/", org.String()), nil)
2727
if err != nil {
2828
return nil, err
2929
}

0 commit comments

Comments
 (0)