synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-15 22:20:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: remove unique constraint on OAuth2 provider app names (#18669)

Browse Source 
# Remove unique constraint on OAuth2 provider app names

This PR removes the unique constraint on the `name` field in the `oauth2_provider_apps` table to comply with RFC 7591, which only requires unique client IDs, not unique client names.

Changes include:
- Removing the unique constraint from the database schema
- Adding migration files for both up and down migrations
- Removing the name uniqueness check in the in-memory database implementation
- Updating the unique constraint constants

Change-Id: Iae7a1a06546fbc8de541a52e291f8a4510d57e8a
Signed-off-by: Thomas Kosiewski <tk@coder.com>
This commit is contained in:
Thomas Kosiewski
2025-07-03 19:13:13 +02:00
committed by GitHub
parent 90a875d916
commit 60b08f0960
6 changed files with 66 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
coderd/database/dbmem/dbmem.go
View File

@ -8983,12 +8983,6 @@ func (q *FakeQuerier) InsertOAuth2ProviderApp(_ context.Context, arg database.In
q.mutex.Lock()
defer q.mutex.Unlock()
for _, app := range q.oauth2ProviderApps {
if app.Name == arg.Name {
return database.OAuth2ProviderApp{}, errUniqueConstraint
}
}
//nolint:gosimple // Go wants database.OAuth2ProviderApp(arg), but we cannot be sure the structs will remain identical.
app := database.OAuth2ProviderApp{
ID: arg.ID,

3
coderd/database/dump.sql generated
View File

@ -2494,9 +2494,6 @@ ALTER TABLE ONLY oauth2_provider_app_tokens
ALTER TABLE ONLY oauth2_provider_app_tokens
ADD CONSTRAINT oauth2_provider_app_tokens_pkey PRIMARY KEY (id);
ALTER TABLE ONLY oauth2_provider_apps
ADD CONSTRAINT oauth2_provider_apps_name_key UNIQUE (name);
ALTER TABLE ONLY oauth2_provider_apps
ADD CONSTRAINT oauth2_provider_apps_pkey PRIMARY KEY (id);

3
coderd/database/migrations/000348_remove_oauth2_app_name_unique_constraint.down.sql Normal file
View File

@ -0,0 +1,3 @@
-- Restore unique constraint on oauth2_provider_apps.name for rollback
-- Note: This rollback may fail if duplicate names exist in the database
ALTER TABLE oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_name_key UNIQUE (name);

3
coderd/database/migrations/000348_remove_oauth2_app_name_unique_constraint.up.sql Normal file
View File

@ -0,0 +1,3 @@
-- Remove unique constraint on oauth2_provider_apps.name to comply with RFC 7591
-- RFC 7591 does not require unique client names, only unique client IDs
ALTER TABLE oauth2_provider_apps DROP CONSTRAINT oauth2_provider_apps_name_key;

1
coderd/database/unique_constraint.go
View File

@ -36,7 +36,6 @@ const (
UniqueOauth2ProviderAppSecretsSecretPrefixKey UniqueConstraint = "oauth2_provider_app_secrets_secret_prefix_key" // ALTER TABLE ONLY oauth2_provider_app_secrets ADD CONSTRAINT oauth2_provider_app_secrets_secret_prefix_key UNIQUE (secret_prefix);
UniqueOauth2ProviderAppTokensHashPrefixKey UniqueConstraint = "oauth2_provider_app_tokens_hash_prefix_key" // ALTER TABLE ONLY oauth2_provider_app_tokens ADD CONSTRAINT oauth2_provider_app_tokens_hash_prefix_key UNIQUE (hash_prefix);
UniqueOauth2ProviderAppTokensPkey UniqueConstraint = "oauth2_provider_app_tokens_pkey" // ALTER TABLE ONLY oauth2_provider_app_tokens ADD CONSTRAINT oauth2_provider_app_tokens_pkey PRIMARY KEY (id);
UniqueOauth2ProviderAppsNameKey UniqueConstraint = "oauth2_provider_apps_name_key" // ALTER TABLE ONLY oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_name_key UNIQUE (name);
UniqueOauth2ProviderAppsPkey UniqueConstraint = "oauth2_provider_apps_pkey" // ALTER TABLE ONLY oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_pkey PRIMARY KEY (id);
UniqueOrganizationMembersPkey UniqueConstraint = "organization_members_pkey" // ALTER TABLE ONLY organization_members ADD CONSTRAINT organization_members_pkey PRIMARY KEY (organization_id, user_id);
UniqueOrganizationsPkey UniqueConstraint = "organizations_pkey" // ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_pkey PRIMARY KEY (id);

77
coderd/oauth2_test.go
View File

@ -64,13 +64,6 @@ func TestOAuth2ProviderApps(t *testing.T) {
CallbackURL: "http://localhost:3000",
},
},
{
name: "NameTaken",
req: codersdk.PostOAuth2ProviderAppRequest{
Name: "taken",
CallbackURL: "http://localhost:3000",
},
},
{
name: "URLMissing",
req: codersdk.PostOAuth2ProviderAppRequest{
@ -135,17 +128,8 @@ func TestOAuth2ProviderApps(t *testing.T) {
},
}
// Generate an application for testing name conflicts.
req := codersdk.PostOAuth2ProviderAppRequest{
Name: "taken",
CallbackURL: "http://coder.com",
}
//nolint:gocritic // OAauth2 app management requires owner permission.
_, err := client.PostOAuth2ProviderApp(ctx, req)
require.NoError(t, err)
// Generate an application for testing PUTs.
req = codersdk.PostOAuth2ProviderAppRequest{
req := codersdk.PostOAuth2ProviderAppRequest{
Name: fmt.Sprintf("quark-%d", time.Now().UnixNano()%1000000),
CallbackURL: "http://coder.com",
}
@ -271,6 +255,65 @@ func TestOAuth2ProviderApps(t *testing.T) {
require.NoError(t, err)
require.Len(t, apps, 0)
})
t.Run("DuplicateNames", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
ctx := testutil.Context(t, testutil.WaitLong)
// Create multiple OAuth2 apps with the same name to verify RFC 7591 compliance
// RFC 7591 allows multiple apps to have the same name
appName := fmt.Sprintf("duplicate-name-%d", time.Now().UnixNano()%1000000)
// Create first app
//nolint:gocritic // OAuth2 app management requires owner permission.
app1, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
Name: appName,
CallbackURL: "http://localhost:3001",
})
require.NoError(t, err)
require.Equal(t, appName, app1.Name)
// Create second app with the same name
//nolint:gocritic // OAuth2 app management requires owner permission.
app2, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
Name: appName,
CallbackURL: "http://localhost:3002",
})
require.NoError(t, err)
require.Equal(t, appName, app2.Name)
// Create third app with the same name
//nolint:gocritic // OAuth2 app management requires owner permission.
app3, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
Name: appName,
CallbackURL: "http://localhost:3003",
})
require.NoError(t, err)
require.Equal(t, appName, app3.Name)
// Verify all apps have different IDs but same name
require.NotEqual(t, app1.ID, app2.ID)
require.NotEqual(t, app1.ID, app3.ID)
require.NotEqual(t, app2.ID, app3.ID)
require.Equal(t, app1.Name, app2.Name)
require.Equal(t, app1.Name, app3.Name)
// Verify all apps can be retrieved and have the same name
//nolint:gocritic // OAuth2 app management requires owner permission.
apps, err := client.OAuth2ProviderApps(ctx, codersdk.OAuth2ProviderAppFilter{})
require.NoError(t, err)
// Count apps with our duplicate name
duplicateNameCount := 0
for _, app := range apps {
if app.Name == appName {
duplicateNameCount++
}
}
require.Equal(t, 3, duplicateNameCount, "Should have exactly 3 apps with the duplicate name")
})
}
func TestOAuth2ProviderAppSecrets(t *testing.T) {