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

Conversation

ThomasK33
Copy link
Member

@ThomasK33 ThomasK33 commented Jun 30, 2025

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

@ThomasK33 ThomasK33 changed the title feat(oauth2): remove unique constraint on app names for RFC 7591 compliance fix: remove unique constraint on OAuth2 provider app names Jun 30, 2025
@ThomasK33 ThomasK33 marked this pull request as ready for review June 30, 2025 16:43
@ThomasK33 ThomasK33 force-pushed the thomask33/06-30-feat_oauth2_remove_unique_constraint_on_app_names_for_rfc_7591_compliance branch from 4efb07a to 8a3f7a9 Compare June 30, 2025 16:45
@ThomasK33 ThomasK33 force-pushed the thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows branch from 58b076d to 88f01e4 Compare June 30, 2025 16:45
@ThomasK33 ThomasK33 requested review from johnstcn and mafredri June 30, 2025 16:52
@mafredri
Copy link
Member

@ThomasK33 tests seem unhappy 😔

Copy link
Member Author

@ThomasK33 tests seem unhappy 😔

Yeah, I forgot to update them. On it.

@ThomasK33 ThomasK33 force-pushed the thomask33/06-30-feat_oauth2_remove_unique_constraint_on_app_names_for_rfc_7591_compliance branch from 8a3f7a9 to e7d56c2 Compare June 30, 2025 17:08
@ThomasK33 ThomasK33 force-pushed the thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows branch from 88f01e4 to 0d042a4 Compare June 30, 2025 17:56
@ThomasK33 ThomasK33 force-pushed the thomask33/06-30-feat_oauth2_remove_unique_constraint_on_app_names_for_rfc_7591_compliance branch from e7d56c2 to 10701da Compare June 30, 2025 17:56
@ThomasK33 ThomasK33 force-pushed the thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows branch from 0d042a4 to f4614fd Compare July 1, 2025 09:15
@ThomasK33 ThomasK33 force-pushed the thomask33/06-30-feat_oauth2_remove_unique_constraint_on_app_names_for_rfc_7591_compliance branch 2 times, most recently from a2607a8 to 990f706 Compare July 1, 2025 09:27
@ThomasK33 ThomasK33 force-pushed the thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows branch from f4614fd to 5943be1 Compare July 1, 2025 09:27
@ThomasK33 ThomasK33 force-pushed the thomask33/06-30-feat_oauth2_remove_unique_constraint_on_app_names_for_rfc_7591_compliance branch from 990f706 to 95c0496 Compare July 1, 2025 13:23
@ThomasK33 ThomasK33 force-pushed the thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows branch from 5943be1 to 2e85437 Compare July 1, 2025 13:23
@ThomasK33 ThomasK33 force-pushed the thomask33/06-27-docs_refactor_claude.md_to_use_import_system_and_comprehensive_workflows branch from 2e85437 to aa379a5 Compare July 1, 2025 13:42
…liance

Change-Id: Iae7a1a06546fbc8de541a52e291f8a4510d57e8a
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33 ThomasK33 force-pushed the thomask33/06-30-feat_oauth2_remove_unique_constraint_on_app_names_for_rfc_7591_compliance branch from 95c0496 to b17d907 Compare July 1, 2025 13:43
Comment on lines -138 to -146
// 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)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants