Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions coderd/database/dbmem/dbmem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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 change: 0 additions & 1 deletion coderd/database/unique_constraint.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 1 addition & 17 deletions coderd/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)

Comment on lines -138 to -146
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should validate that multiple OAuth2 apps can have the same name.

// 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",
}
Expand Down
Loading