Skip to content

Commit

Permalink
make CockroachDB username case-insensitive (#41823)
Browse files Browse the repository at this point in the history
  • Loading branch information
greedy52 committed May 21, 2024
1 parent 33c1c70 commit 846ed56
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 5 deletions.
13 changes: 13 additions & 0 deletions api/types/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ type Database interface {
// GetCloud gets the cloud this database is running on, or an empty string if it
// isn't running on a cloud provider.
GetCloud() string
// IsUsernameCaseInsensitive returns true if the database username is case
// insensitive.
IsUsernameCaseInsensitive() bool
}

// NewDatabaseV3 creates a new database resource.
Expand Down Expand Up @@ -1043,6 +1046,14 @@ func (d *DatabaseV3) GetEndpointType() string {
return ""
}

// IsUsernameCaseInsensitive returns true if the database username is case
// insensitive.
func (d *DatabaseV3) IsUsernameCaseInsensitive() bool {
// CockroachDB usernames are case-insensitive:
// https://www.cockroachlabs.com/docs/stable/create-user#user-names
return d.GetProtocol() == DatabaseProtocolCockroachDB
}

const (
// DatabaseProtocolPostgreSQL is the PostgreSQL database protocol.
DatabaseProtocolPostgreSQL = "postgres"
Expand All @@ -1054,6 +1065,8 @@ const (
DatabaseProtocolMySQL = "mysql"
// DatabaseProtocolMongoDB is the MongoDB database protocol.
DatabaseProtocolMongoDB = "mongodb"
// DatabaseProtocolCockroachDB is the CockroachDB database protocol.
DatabaseProtocolCockroachDB = "cockroachdb"

// DatabaseTypeSelfHosted is the self-hosted type of database.
DatabaseTypeSelfHosted = "self-hosted"
Expand Down
22 changes: 17 additions & 5 deletions lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,9 +1068,13 @@ func MatchDatabaseName(selectors []string, name string) (bool, string) {
}

// MatchDatabaseUser returns true if provided database user matches selectors.
func MatchDatabaseUser(selectors []string, user string, matchWildcard bool) (bool, string) {
func MatchDatabaseUser(selectors []string, user string, matchWildcard, caseFold bool) (bool, string) {
for _, u := range selectors {
if u == user {
if caseFold {
if strings.EqualFold(u, user) {
return true, "matched"
}
} else if u == user {
return true, "matched"
}
if matchWildcard && u == types.Wildcard {
Expand Down Expand Up @@ -2122,6 +2126,8 @@ type databaseUserMatcher struct {
user string
// alternativeNames is a list of alternative names for the database user.
alternativeNames []string
// caseInsensitive specifies if the username is case insensitive.
caseInsensitive bool
}

// NewDatabaseUserMatcher creates a RoleMatcher that checks whether the role's
Expand All @@ -2131,28 +2137,34 @@ func NewDatabaseUserMatcher(db types.Database, user string) RoleMatcher {
return &databaseUserMatcher{
user: user,
alternativeNames: makeUsernamesForAWSRoleARN(db, user),
caseInsensitive: db.IsUsernameCaseInsensitive(),
}
}

if db.RequireAWSIAMRolesAsUsers() {
return &databaseUserMatcher{
user: user,
alternativeNames: makeAlternativeNamesForAWSRole(db, user),
caseInsensitive: db.IsUsernameCaseInsensitive(),
}
}

return &databaseUserMatcher{user: user}
return &databaseUserMatcher{
user: user,
caseInsensitive: db.IsUsernameCaseInsensitive(),
}
}

// Match matches database account name against provided role and condition.
func (m *databaseUserMatcher) Match(role types.Role, condition types.RoleConditionType) (bool, error) {
selectors := role.GetDatabaseUsers(condition)
if match, _ := MatchDatabaseUser(selectors, m.user, true); match {

if match, _ := MatchDatabaseUser(selectors, m.user, true /*matchWildcard*/, m.caseInsensitive); match {
return true, nil
}

for _, altName := range m.alternativeNames {
if match, _ := MatchDatabaseUser(selectors, altName, false); match {
if match, _ := MatchDatabaseUser(selectors, altName, false /*matchWildcard*/, m.caseInsensitive); match {
return true, nil
}
}
Expand Down
39 changes: 39 additions & 0 deletions lib/services/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3829,6 +3829,23 @@ func TestCheckAccessToDatabaseUser(t *testing.T) {
require.NoError(t, err)
require.True(t, dbSupportAWSRoles.SupportAWSIAMRoleARNAsUsers())

dbCockroachStage, err := types.NewDatabaseV3(types.Metadata{
Name: "cockroachdb",
Labels: map[string]string{"env": "stage"},
}, types.DatabaseSpecV3{
Protocol: "cockroachdb",
URI: "cockroachdb:26257",
})
require.NoError(t, err)
dbCockroachProd, err := types.NewDatabaseV3(types.Metadata{
Name: "cockroachdb",
Labels: map[string]string{"env": "prod"},
}, types.DatabaseSpecV3{
Protocol: "cockroachdb",
URI: "cockroachdb:26257",
})
require.NoError(t, err)

type access struct {
server types.Database
dbUser string
Expand All @@ -3846,6 +3863,7 @@ func TestCheckAccessToDatabaseUser(t *testing.T) {
{server: dbStage, dbUser: "superuser", access: false},
{server: dbStage, dbUser: "dev", access: true},
{server: dbStage, dbUser: "test", access: true},
{server: dbStage, dbUser: "SUPERUSER", access: true},
},
},
{
Expand Down Expand Up @@ -3887,6 +3905,27 @@ func TestCheckAccessToDatabaseUser(t *testing.T) {
{server: dbSupportAWSRoles, dbUser: "unknown-user", access: false},
},
},
{
name: "(case-insensitive db) developer allowed any username in stage except superuser",
roles: RoleSet{roleDevStage, roleDevProd},
access: []access{
{server: dbCockroachStage, dbUser: "dev", access: true},
{server: dbCockroachStage, dbUser: "DEV", access: true},
{server: dbCockroachStage, dbUser: "test", access: true},
{server: dbCockroachStage, dbUser: "superuser", access: false},
{server: dbCockroachStage, dbUser: "SUPERUSER", access: false},
},
},
{
name: "(case-insensitive db) developer allowed only specific username/database in prod database",
roles: RoleSet{roleDevStage, roleDevProd},
access: []access{
{server: dbCockroachProd, dbUser: "dev", access: true},
{server: dbCockroachProd, dbUser: "DEV", access: true},
{server: dbCockroachProd, dbUser: "superuser", access: false},
{server: dbCockroachProd, dbUser: "Superuser", access: false},
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down

0 comments on commit 846ed56

Please sign in to comment.