Skip to content

Commit 743c19e

Browse files
committed
test: implement test to assert deleted custom roles are omitted
1 parent 819bfd3 commit 743c19e

File tree

2 files changed

+128
-1
lines changed

2 files changed

+128
-1
lines changed

coderd/httpmw/apikey_test.go

Lines changed: 126 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ import (
1414
"testing"
1515
"time"
1616

17+
"github.com/google/uuid"
1718
"github.com/stretchr/testify/assert"
1819
"github.com/stretchr/testify/require"
20+
"golang.org/x/exp/slices"
1921
"golang.org/x/oauth2"
2022

2123
"github.com/coder/coder/v2/coderd/database"
@@ -24,6 +26,7 @@ import (
2426
"github.com/coder/coder/v2/coderd/database/dbtime"
2527
"github.com/coder/coder/v2/coderd/httpapi"
2628
"github.com/coder/coder/v2/coderd/httpmw"
29+
"github.com/coder/coder/v2/coderd/rbac"
2730
"github.com/coder/coder/v2/codersdk"
2831
"github.com/coder/coder/v2/cryptorand"
2932
"github.com/coder/coder/v2/testutil"
@@ -633,7 +636,7 @@ func TestAPIKey(t *testing.T) {
633636
require.Equal(t, sentAPIKey.LoginType, gotAPIKey.LoginType)
634637
})
635638

636-
t.Run("MissongConfig", func(t *testing.T) {
639+
t.Run("MissingConfig", func(t *testing.T) {
637640
t.Parallel()
638641
var (
639642
db = dbmem.New()
@@ -667,4 +670,126 @@ func TestAPIKey(t *testing.T) {
667670
out, _ := io.ReadAll(res.Body)
668671
require.Contains(t, string(out), "Unable to refresh")
669672
})
673+
674+
t.Run("CustomRoles", func(t *testing.T) {
675+
t.Parallel()
676+
var (
677+
db = dbmem.New()
678+
org = dbgen.Organization(t, db, database.Organization{})
679+
customRole = dbgen.CustomRole(t, db, database.CustomRole{
680+
Name: "custom-role",
681+
682+
OrgPermissions: []database.CustomRolePermission{},
683+
OrganizationID: uuid.NullUUID{
684+
UUID: org.ID,
685+
Valid: true,
686+
},
687+
})
688+
user = dbgen.User(t, db, database.User{
689+
RBACRoles: []string{
690+
rbac.ScopedRoleOrgAdmin(org.ID).String(),
691+
customRole.RoleIdentifier().String(),
692+
},
693+
})
694+
sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{
695+
UserID: user.ID,
696+
ExpiresAt: dbtime.Now().AddDate(0, 0, 1),
697+
})
698+
699+
r = httptest.NewRequest("GET", "/", nil)
700+
rw = httptest.NewRecorder()
701+
)
702+
r.Header.Set(codersdk.SessionTokenHeader, token)
703+
704+
httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
705+
DB: db,
706+
RedirectToLogin: false,
707+
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
708+
// Checks that it exists on the context!
709+
_ = httpmw.APIKey(r)
710+
auth := httpmw.UserAuthorization(r)
711+
roles, err := auth.Roles.Expand()
712+
assert.NoError(t, err, "expand user roles")
713+
// Assert built in org role
714+
assert.True(t, slices.ContainsFunc(roles, func(role rbac.Role) bool {
715+
return role.Identifier.Name == rbac.RoleOrgAdmin() && role.Identifier.OrganizationID == org.ID
716+
}), "org admin role")
717+
// Assert custom role
718+
assert.True(t, slices.ContainsFunc(roles, func(role rbac.Role) bool {
719+
return role.Identifier.Name == customRole.Name && role.Identifier.OrganizationID == org.ID
720+
}), "custom org role")
721+
722+
httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.Response{
723+
Message: "It worked!",
724+
})
725+
})).ServeHTTP(rw, r)
726+
res := rw.Result()
727+
defer res.Body.Close()
728+
require.Equal(t, http.StatusOK, res.StatusCode)
729+
730+
gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID)
731+
require.NoError(t, err)
732+
733+
require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt)
734+
})
735+
736+
// There is no sql foreign key constraint to require all assigned roles
737+
// still exist in the database. We need to handle deleted roles.
738+
t.Run("RoleNotExists", func(t *testing.T) {
739+
t.Parallel()
740+
var (
741+
roleNotExistsName = "role-not-exists"
742+
db = dbmem.New()
743+
org = dbgen.Organization(t, db, database.Organization{})
744+
user = dbgen.User(t, db, database.User{
745+
RBACRoles: []string{
746+
rbac.ScopedRoleOrgAdmin(org.ID).String(),
747+
rbac.RoleIdentifier{Name: roleNotExistsName, OrganizationID: org.ID}.String(),
748+
// Also provide an org not exists
749+
rbac.RoleIdentifier{Name: roleNotExistsName, OrganizationID: uuid.New()}.String(),
750+
},
751+
})
752+
sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{
753+
UserID: user.ID,
754+
ExpiresAt: dbtime.Now().AddDate(0, 0, 1),
755+
})
756+
757+
r = httptest.NewRequest("GET", "/", nil)
758+
rw = httptest.NewRecorder()
759+
)
760+
r.Header.Set(codersdk.SessionTokenHeader, token)
761+
762+
httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
763+
DB: db,
764+
RedirectToLogin: false,
765+
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
766+
// Checks that it exists on the context!
767+
_ = httpmw.APIKey(r)
768+
auth := httpmw.UserAuthorization(r)
769+
roles, err := auth.Roles.Expand()
770+
assert.NoError(t, err, "expand user roles")
771+
// Assert built in org role
772+
assert.True(t, slices.ContainsFunc(roles, func(role rbac.Role) bool {
773+
return role.Identifier.Name == rbac.RoleOrgAdmin() && role.Identifier.OrganizationID == org.ID
774+
}), "org admin role")
775+
776+
// Assert the role-not-exists is not returned
777+
assert.False(t, slices.ContainsFunc(roles, func(role rbac.Role) bool {
778+
return role.Identifier.Name == roleNotExistsName
779+
}), "role should not exist")
780+
781+
httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.Response{
782+
Message: "It worked!",
783+
})
784+
})).ServeHTTP(rw, r)
785+
res := rw.Result()
786+
defer res.Body.Close()
787+
require.Equal(t, http.StatusOK, res.StatusCode)
788+
789+
gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID)
790+
require.NoError(t, err)
791+
792+
require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt)
793+
})
794+
670795
}

coderd/rbac/rolestore/rolestore.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ func roleCache(ctx context.Context) *syncmap.Map[string, rbac.Role] {
3939
}
4040

4141
// Expand will expand built in roles, and fetch custom roles from the database.
42+
// If a custom role is defined, but does not exist, the role will be omitted on
43+
// the response. This means deleted roles are silently dropped.
4244
func Expand(ctx context.Context, db database.Store, names []rbac.RoleIdentifier) (rbac.Roles, error) {
4345
if len(names) == 0 {
4446
// That was easy

0 commit comments

Comments
 (0)