feat(oauth2): remove unique constraint on app names for RFC 7591 compliance

ThomasK33 · ThomasK33 · commit 43ac8a090288 · 2025-07-02T16:05:47.000+02:00
Change-Id: Iae7a1a06546fbc8de541a52e291f8a4510d57e8a
Signed-off-by: Thomas Kosiewski &lt;tk@coder.com&gt;
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -8975,12 +8975,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,
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000347_remove_oauth2_app_name_unique_constraint.down.sql b/coderd/database/migrations/000347_remove_oauth2_app_name_unique_constraint.down.sql
@@ -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);
diff --git a/coderd/database/migrations/000347_remove_oauth2_app_name_unique_constraint.up.sql b/coderd/database/migrations/000347_remove_oauth2_app_name_unique_constraint.up.sql
@@ -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;
diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go
diff --git a/coderd/oauth2_test.go b/coderd/oauth2_test.go
@@ -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) {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+-- Restore unique constraint on oauth2_provider_apps.name for rollback`
	`2`	`+-- Note: This rollback may fail if duplicate names exist in the database`
	`3`	`+ALTER TABLE oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_name_key UNIQUE (name);`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+-- Remove unique constraint on oauth2_provider_apps.name to comply with RFC 7591`
	`2`	`+-- RFC 7591 does not require unique client names, only unique client IDs`
	`3`	`+ALTER TABLE oauth2_provider_apps DROP CONSTRAINT oauth2_provider_apps_name_key;`