diff --git a/cli/server_fix_oidc_links.go b/cli/server_fix_oidc_links.go index 0baeca881f..1d4bdd7314 100644 --- a/cli/server_fix_oidc_links.go +++ b/cli/server_fix_oidc_links.go @@ -21,10 +21,11 @@ import ( func (r *RootCmd) newFixOIDCLinksCommand() *serpent.Command { var ( - pgURL string - pgAuth string - issuerURL string - dryRun bool + pgURL string + pgAuth string + issuerURL string + dryRun bool + forceResetAll bool ) fixOIDCLinksCmd := &serpent.Command{ Use: "fix-oidc-links", @@ -40,17 +41,29 @@ func (r *RootCmd) newFixOIDCLinksCommand() *serpent.Command { defer cancel() issuerURL = strings.TrimSpace(issuerURL) - if issuerURL == "" { + if forceResetAll && issuerURL != "" { + return xerrors.New("--force-reset-all and --issuer-url are mutually exclusive") + } + if !forceResetAll && issuerURL == "" { return xerrors.Errorf("the --%s flag is required, set it to the OIDC issuer URL (e.g. https://accounts.google.com)", "issuer-url") } - // Resolve the canonical issuer from OIDC discovery. - cliui.Infof(inv.Stdout, "Resolving OIDC issuer from %q...", issuerURL) - // TODO: The default client might not be configured with the right certs to make this request. - issuer, err := authlink.ResolveIssuer(ctx, http.DefaultClient, issuerURL) - if err != nil { - return xerrors.Errorf("resolve issuer: %w", err) + + var issuer string + if forceResetAll { + // Use an unmatchable issuer so the existing analysis shows + // all links as "mismatched" and the reset clears everything. + issuer = authlink.UnmatchableIssuer + } else { + // Resolve the canonical issuer from OIDC discovery. + cliui.Infof(inv.Stdout, "Resolving OIDC issuer from %q...", issuerURL) + // TODO: The default client might not be configured with the right certs to make this request. + resolved, err := authlink.ResolveIssuer(ctx, http.DefaultClient, issuerURL) + if err != nil { + return xerrors.Errorf("resolve issuer: %w", err) + } + issuer = resolved + _, _ = fmt.Fprintf(inv.Stdout, "Resolved OIDC issuer: %q\n\n", issuer) } - _, _ = fmt.Fprintf(inv.Stdout, "Resolved OIDC issuer: %q\n\n", issuer) // Connect to the database. if pgURL == "" { @@ -59,6 +72,7 @@ func (r *RootCmd) newFixOIDCLinksCommand() *serpent.Command { sqlDriver := "postgres" if codersdk.PostgresAuth(pgAuth) == codersdk.PostgresAuthAWSIAMRDS { + var err error sqlDriver, err = awsiamrds.Register(inv.Context(), sqlDriver) if err != nil { return xerrors.Errorf("register aws rds iam auth: %w", err) @@ -150,6 +164,12 @@ func (r *RootCmd) newFixOIDCLinksCommand() *serpent.Command { Description: "Print analysis only, do not modify the database.", Value: serpent.BoolOf(&dryRun), }, + serpent.Option{ + Flag: "force-reset-all", + Env: "CODER_FIX_OIDC_LINKS_FORCE_RESET_ALL", + Description: "Reset all OIDC linked IDs, not just those with a mismatched issuer. Mutually exclusive with --issuer-url.", + Value: serpent.BoolOf(&forceResetAll), + }, ) return fixOIDCLinksCmd diff --git a/cli/server_fix_oidc_links_test.go b/cli/server_fix_oidc_links_test.go index 407ebc4c4f..85a0d06394 100644 --- a/cli/server_fix_oidc_links_test.go +++ b/cli/server_fix_oidc_links_test.go @@ -161,6 +161,126 @@ func TestFixOIDCLinks(t *testing.T) { require.Equal(t, expectedIssuer+"||sub-correct", link.LinkedID) }) + t.Run("ForceResetAll", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) + t.Cleanup(cancel) + + connectionURL, err := dbtestutil.Open(t) + require.NoError(t, err) + + sqlDB, err := sql.Open("postgres", connectionURL) + require.NoError(t, err) + defer sqlDB.Close() + + db := database.New(sqlDB) + + // Seed users with different issuers. + user1 := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: user1.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "https://accounts.google.com||sub-1", + }) + + user2 := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: user2.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "https://old-issuer.example.com||sub-2", + }) + + inv, _ := clitest.New(t, + "server", "fix-oidc-links", + "--postgres-url", connectionURL, + "--force-reset-all", + "--yes", + ) + + stdout := expecter.NewAttachedToInvocation(t, inv) + w := clitest.StartWithWaiter(t, inv) + + stdout.ExpectMatch(ctx, "Linked to other issuers:") + stdout.ExpectMatch(ctx, "Reset 2 linked IDs.") + w.RequireSuccess() + + // Verify both links were reset. + link, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: user1.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "", link.LinkedID) + + link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: user2.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "", link.LinkedID) + }) + + t.Run("ForceResetAllDryRun", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) + t.Cleanup(cancel) + + connectionURL, err := dbtestutil.Open(t) + require.NoError(t, err) + + sqlDB, err := sql.Open("postgres", connectionURL) + require.NoError(t, err) + defer sqlDB.Close() + + db := database.New(sqlDB) + + // Seed users with different issuers. + user1 := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: user1.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "https://accounts.google.com||sub-1", + }) + + user2 := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: user2.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "https://old-issuer.example.com||sub-2", + }) + + inv, _ := clitest.New(t, + "server", "fix-oidc-links", + "--postgres-url", connectionURL, + "--force-reset-all", + "--dry-run", + ) + + stdout := expecter.NewAttachedToInvocation(t, inv) + w := clitest.StartWithWaiter(t, inv) + + stdout.ExpectMatch(ctx, "Total OIDC users:") + stdout.ExpectMatch(ctx, "Linked to other issuers:") + w.RequireSuccess() + + // Verify no changes were made. + link, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: user1.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "https://accounts.google.com||sub-1", link.LinkedID, "dry-run must not modify the database") + + link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: user2.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "https://old-issuer.example.com||sub-2", link.LinkedID, "dry-run must not modify the database") + }) + t.Run("NothingToDo", func(t *testing.T) { t.Parallel() diff --git a/cli/testdata/coder_server_fix-oidc-links_--help.golden b/cli/testdata/coder_server_fix-oidc-links_--help.golden index 201a71435e..496963074f 100644 --- a/cli/testdata/coder_server_fix-oidc-links_--help.golden +++ b/cli/testdata/coder_server_fix-oidc-links_--help.golden @@ -13,6 +13,10 @@ OPTIONS: -n, --dry-run bool, $CODER_FIX_OIDC_LINKS_DRY_RUN Print analysis only, do not modify the database. + --force-reset-all bool, $CODER_FIX_OIDC_LINKS_FORCE_RESET_ALL + Reset all OIDC linked IDs, not just those with a mismatched issuer. + Mutually exclusive with --issuer-url. + --issuer-url string, $CODER_OIDC_ISSUER_URL The OIDC issuer URL. The canonical issuer is resolved via OIDC discovery. diff --git a/coderd/authlink/authlink.go b/coderd/authlink/authlink.go index 8bcd655ebe..1f3f177f55 100644 --- a/coderd/authlink/authlink.go +++ b/coderd/authlink/authlink.go @@ -70,6 +70,12 @@ func ResetMismatchedOIDCLinks(ctx context.Context, db database.Store, expectedIs return count, nil } +// UnmatchableIssuer is a synthetic issuer value that no real OIDC linked_id +// will ever start with. Passing it to AnalyzeOIDCLinks or +// ResetMismatchedOIDCLinks causes every link to be treated as "mismatched", +// which effectively resets all of them. +const UnmatchableIssuer = "00000000-0000-0000-0000-000000000000" + // ResolveIssuer uses OIDC discovery to fetch the canonical issuer string // from the provider's .well-known/openid-configuration endpoint. // This does not require OIDC client credentials. diff --git a/coderd/authlink/authlink_test.go b/coderd/authlink/authlink_test.go index cf7a0725cc..35a2acaea0 100644 --- a/coderd/authlink/authlink_test.go +++ b/coderd/authlink/authlink_test.go @@ -247,6 +247,88 @@ func TestResetMismatchedOIDCLinks(t *testing.T) { }) } +func TestResetMismatchedOIDCLinksWithUnmatchableIssuer(t *testing.T) { + t.Parallel() + + t.Run("ResetsAll", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + db, _ := dbtestutil.NewDB(t) + + // Correctly linked user. + correctUser := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: correctUser.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "https://accounts.google.com||sub-correct", + }) + + // Mismatched user. + mismatchedUser := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: mismatchedUser.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "https://old-issuer.example.com||sub-mismatched", + }) + + // Unlinked user (empty linked_id). + unlinkedUser := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: unlinkedUser.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "", + }) + + count, err := authlink.ResetMismatchedOIDCLinks(ctx, db, authlink.UnmatchableIssuer) + require.NoError(t, err) + require.EqualValues(t, 2, count, "should reset correct + mismatched, not unlinked") + + // Verify the correct link was reset. + link, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: correctUser.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "", link.LinkedID) + + // Verify the mismatched link was reset. + link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: mismatchedUser.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "", link.LinkedID) + + // Verify the unlinked user is still unlinked. + link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: unlinkedUser.ID, + LoginType: database.LoginTypeOIDC, + }) + require.NoError(t, err) + require.Equal(t, "", link.LinkedID) + }) + + t.Run("NothingToReset", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + db, _ := dbtestutil.NewDB(t) + + // Only an unlinked user. + user := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC}) + dbgen.UserLink(t, db, database.UserLink{ + UserID: user.ID, + LoginType: database.LoginTypeOIDC, + LinkedID: "", + }) + + count, err := authlink.ResetMismatchedOIDCLinks(ctx, db, authlink.UnmatchableIssuer) + require.NoError(t, err) + require.EqualValues(t, 0, count) + }) +} + func TestResolveIssuer(t *testing.T) { t.Parallel() diff --git a/docs/reference/cli/server_fix-oidc-links.md b/docs/reference/cli/server_fix-oidc-links.md index ef5250948e..a79b701b0d 100644 --- a/docs/reference/cli/server_fix-oidc-links.md +++ b/docs/reference/cli/server_fix-oidc-links.md @@ -55,3 +55,12 @@ The OIDC issuer URL. The canonical issuer is resolved via OIDC discovery. | Environment | $CODER_FIX_OIDC_LINKS_DRY_RUN | Print analysis only, do not modify the database. + +### --force-reset-all + +| | | +|-------------|----------------------------------------------------| +| Type | bool | +| Environment | $CODER_FIX_OIDC_LINKS_FORCE_RESET_ALL | + +Reset all OIDC linked IDs, not just those with a mismatched issuer. Mutually exclusive with --issuer-url. diff --git a/enterprise/cli/testdata/coder_server_fix-oidc-links_--help.golden b/enterprise/cli/testdata/coder_server_fix-oidc-links_--help.golden index 201a71435e..496963074f 100644 --- a/enterprise/cli/testdata/coder_server_fix-oidc-links_--help.golden +++ b/enterprise/cli/testdata/coder_server_fix-oidc-links_--help.golden @@ -13,6 +13,10 @@ OPTIONS: -n, --dry-run bool, $CODER_FIX_OIDC_LINKS_DRY_RUN Print analysis only, do not modify the database. + --force-reset-all bool, $CODER_FIX_OIDC_LINKS_FORCE_RESET_ALL + Reset all OIDC linked IDs, not just those with a mismatched issuer. + Mutually exclusive with --issuer-url. + --issuer-url string, $CODER_OIDC_ISSUER_URL The OIDC issuer URL. The canonical issuer is resolved via OIDC discovery.