Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit60b08f0

Browse files
authored
fix: remove unique constraint on OAuth2 provider app names (#18669)
# Remove unique constraint on OAuth2 provider app namesThis 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 constantsChange-Id: Iae7a1a06546fbc8de541a52e291f8a4510d57e8aSigned-off-by: Thomas Kosiewski <tk@coder.com>
1 parent90a875d commit60b08f0

File tree

6 files changed

+66
-27
lines changed

6 files changed

+66
-27
lines changed

‎coderd/database/dbmem/dbmem.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8983,12 +8983,6 @@ func (q *FakeQuerier) InsertOAuth2ProviderApp(_ context.Context, arg database.In
89838983
q.mutex.Lock()
89848984
deferq.mutex.Unlock()
89858985

8986-
for_,app:=rangeq.oauth2ProviderApps {
8987-
ifapp.Name==arg.Name {
8988-
return database.OAuth2ProviderApp{},errUniqueConstraint
8989-
}
8990-
}
8991-
89928986
//nolint:gosimple // Go wants database.OAuth2ProviderApp(arg), but we cannot be sure the structs will remain identical.
89938987
app:= database.OAuth2ProviderApp{
89948988
ID:arg.ID,

‎coderd/database/dump.sql‎

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff 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+
ALTERTABLE oauth2_provider_apps ADDCONSTRAINT oauth2_provider_apps_name_key UNIQUE (name);
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff 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+
ALTERTABLE oauth2_provider_apps DROPCONSTRAINT oauth2_provider_apps_name_key;

‎coderd/database/unique_constraint.go‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/oauth2_test.go‎

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,6 @@ func TestOAuth2ProviderApps(t *testing.T) {
6464
CallbackURL:"http://localhost:3000",
6565
},
6666
},
67-
{
68-
name:"NameTaken",
69-
req: codersdk.PostOAuth2ProviderAppRequest{
70-
Name:"taken",
71-
CallbackURL:"http://localhost:3000",
72-
},
73-
},
7467
{
7568
name:"URLMissing",
7669
req: codersdk.PostOAuth2ProviderAppRequest{
@@ -135,17 +128,8 @@ func TestOAuth2ProviderApps(t *testing.T) {
135128
},
136129
}
137130

138-
// Generate an application for testing name conflicts.
139-
req:= codersdk.PostOAuth2ProviderAppRequest{
140-
Name:"taken",
141-
CallbackURL:"http://coder.com",
142-
}
143-
//nolint:gocritic // OAauth2 app management requires owner permission.
144-
_,err:=client.PostOAuth2ProviderApp(ctx,req)
145-
require.NoError(t,err)
146-
147131
// Generate an application for testing PUTs.
148-
req= codersdk.PostOAuth2ProviderAppRequest{
132+
req:= codersdk.PostOAuth2ProviderAppRequest{
149133
Name:fmt.Sprintf("quark-%d",time.Now().UnixNano()%1000000),
150134
CallbackURL:"http://coder.com",
151135
}
@@ -271,6 +255,65 @@ func TestOAuth2ProviderApps(t *testing.T) {
271255
require.NoError(t,err)
272256
require.Len(t,apps,0)
273257
})
258+
259+
t.Run("DuplicateNames",func(t*testing.T) {
260+
t.Parallel()
261+
client:=coderdtest.New(t,nil)
262+
_=coderdtest.CreateFirstUser(t,client)
263+
ctx:=testutil.Context(t,testutil.WaitLong)
264+
265+
// Create multiple OAuth2 apps with the same name to verify RFC 7591 compliance
266+
// RFC 7591 allows multiple apps to have the same name
267+
appName:=fmt.Sprintf("duplicate-name-%d",time.Now().UnixNano()%1000000)
268+
269+
// Create first app
270+
//nolint:gocritic // OAuth2 app management requires owner permission.
271+
app1,err:=client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
272+
Name:appName,
273+
CallbackURL:"http://localhost:3001",
274+
})
275+
require.NoError(t,err)
276+
require.Equal(t,appName,app1.Name)
277+
278+
// Create second app with the same name
279+
//nolint:gocritic // OAuth2 app management requires owner permission.
280+
app2,err:=client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
281+
Name:appName,
282+
CallbackURL:"http://localhost:3002",
283+
})
284+
require.NoError(t,err)
285+
require.Equal(t,appName,app2.Name)
286+
287+
// Create third app with the same name
288+
//nolint:gocritic // OAuth2 app management requires owner permission.
289+
app3,err:=client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
290+
Name:appName,
291+
CallbackURL:"http://localhost:3003",
292+
})
293+
require.NoError(t,err)
294+
require.Equal(t,appName,app3.Name)
295+
296+
// Verify all apps have different IDs but same name
297+
require.NotEqual(t,app1.ID,app2.ID)
298+
require.NotEqual(t,app1.ID,app3.ID)
299+
require.NotEqual(t,app2.ID,app3.ID)
300+
require.Equal(t,app1.Name,app2.Name)
301+
require.Equal(t,app1.Name,app3.Name)
302+
303+
// Verify all apps can be retrieved and have the same name
304+
//nolint:gocritic // OAuth2 app management requires owner permission.
305+
apps,err:=client.OAuth2ProviderApps(ctx, codersdk.OAuth2ProviderAppFilter{})
306+
require.NoError(t,err)
307+
308+
// Count apps with our duplicate name
309+
duplicateNameCount:=0
310+
for_,app:=rangeapps {
311+
ifapp.Name==appName {
312+
duplicateNameCount++
313+
}
314+
}
315+
require.Equal(t,3,duplicateNameCount,"Should have exactly 3 apps with the duplicate name")
316+
})
274317
}
275318

276319
funcTestOAuth2ProviderAppSecrets(t*testing.T) {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp