feat: deployment flag to auto handle changed oidc providers (#26419)

An opt-out flag exists as an escape hatch closes https://linear.app/codercom/issue/PLAT-343/automatically-reset-user-link-for-affected-users-when-idp-provider
2026-09-24 15:04:27 +08:00 · 2026-06-16 13:26:04 -07:00
parent b6fcb9a30a
commit 0e45ded0ed
9 changed files with 286 additions and 23 deletions
@@ -65,6 +65,7 @@ import (
 	"github.com/coder/coder/v2/cli/config"
 	"github.com/coder/coder/v2/coderd"
 	"github.com/coder/coder/v2/coderd/aibridged"
+	"github.com/coder/coder/v2/coderd/authlink"
 	"github.com/coder/coder/v2/coderd/autobuild"
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/awsiamrds"
@@ -117,6 +118,58 @@ import (
 	"github.com/coder/wgtunnel/tunnelsdk"
 )

+// oidcAuthLinks validates and can repair any broken OIDC auth links from changes in
+// OIDC providers. This function should avoid returning a fatal error as much as possible.
+// If this function fails, it should just log the error and exit.
+func oidcAuthLinks(ctx context.Context, logger slog.Logger, cli *http.Client, vals *codersdk.DeploymentValues, db database.Store) error {
+	// nolint:gocritic // Requires system privileges
+	ctx = dbauthz.AsSystemRestricted(ctx)
+	expectedIssuer, err := authlink.ResolveIssuer(ctx, cli, vals.OIDC.IssuerURL.String())
+	if err != nil {
+		// Always log if there is a failure here
+		logger.Error(ctx, "unable to resolve OIDC 'issuer'",
+			slog.F("error", err.Error()),
+			slog.F("url", vals.OIDC.IssuerURL.String()),
+		)
+		return nil
+	}
+
+	analysis, err := authlink.AnalyzeOIDCLinks(ctx, db, expectedIssuer)
+	if err != nil {
+		// Do not make this error fatal
+		logger.Error(ctx, "unable to analyze OIDC links, OIDC user links cannot be verified as linked to this issuer",
+			slog.F("error", err.Error()),
+			slog.F("url", vals.OIDC.IssuerURL.String()),
+			slog.F("issuer", expectedIssuer),
+		)
+		return nil
+	}
+
+	if !vals.OIDC.AutoRepairLinks.Value() {
+		return nil
+	}
+
+	// Repair any broken OIDC links
+	if analysis.MismatchedTotal() > 0 {
+		count, err := authlink.ResetMismatchedOIDCLinks(ctx, db, expectedIssuer)
+		if err != nil {
+			logger.Error(ctx, "unable to reset mismatched OIDC links",
+				slog.F("error", err.Error()),
+				slog.F("url", vals.OIDC.IssuerURL.String()),
+				slog.F("issuer", expectedIssuer),
+			)
+			return nil
+		}
+
+		logger.Info(ctx, "oidc users OIDC links reset",
+			slog.F("url", vals.OIDC.IssuerURL.String()),
+			slog.F("issuer", expectedIssuer),
+			slog.F("count", count),
+		)
+	}
+	return nil
+}
+
 func createOIDCConfig(ctx context.Context, logger slog.Logger, vals *codersdk.DeploymentValues) (*coderd.OIDCConfig, error) {
 	if vals.OIDC.ClientID == "" {
 		return nil, xerrors.Errorf("OIDC client ID must be set!")
@@ -720,29 +773,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				}
 			}

-			// As OIDC clients can be confidential or public,
-			// we should only check for a client id being set.
-			// The underlying library handles the case of no
-			// client secrets correctly. For more details on
-			// client types: https://oauth.net/2/client-types/
-			if vals.OIDC.ClientID != "" {
-				if vals.OIDC.IgnoreEmailVerified {
-					logger.Warn(ctx, "coder will not check email_verified for OIDC logins")
-				}
-
-				// This OIDC config is **not** being instrumented with the
-				// oauth2 instrument wrapper. If we implement the missing
-				// oidc methods, then we can instrument it.
-				// Missing:
-				//	- Userinfo
-				//	- Verify
-				oc, err := createOIDCConfig(ctx, options.Logger, vals)
-				if err != nil {
-					return xerrors.Errorf("create oidc config: %w", err)
-				}
-				options.OIDCConfig = oc
-			}
-
 			// We'll read from this channel in the select below that tracks shutdown.  If it remains
 			// nil, that case of the select will just never fire, but it's important not to have a
 			// "bare" read on this channel.
@@ -838,6 +868,35 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				return xerrors.Errorf("set deployment id: %w", err)
 			}

+			// As OIDC clients can be confidential or public,
+			// we should only check for a client id being set.
+			// The underlying library handles the case of no
+			// client secrets correctly. For more details on
+			// client types: https://oauth.net/2/client-types/
+			if vals.OIDC.ClientID != "" {
+				if vals.OIDC.IgnoreEmailVerified {
+					logger.Warn(ctx, "coder will not check email_verified for OIDC logins")
+				}
+
+				// This OIDC config is **not** being instrumented with the
+				// oauth2 instrument wrapper. If we implement the missing
+				// oidc methods, then we can instrument it.
+				// Missing:
+				//	- Userinfo
+				//	- Verify
+				oc, err := createOIDCConfig(ctx, options.Logger, vals)
+				if err != nil {
+					return xerrors.Errorf("create oidc config: %w", err)
+				}
+				options.OIDCConfig = oc
+
+				// Repair any existing broken OIDC
+				err = oidcAuthLinks(ctx, logger, httpClient, vals, options.Database)
+				if err != nil {
+					return xerrors.Errorf("oidc auth links: %w", err)
+				}
+			}
+
 			extAuthEnv, err := ReadExternalAuthProvidersFromEnv(os.Environ())
 			if err != nil {
 				return xerrors.Errorf("read external auth providers from env: %w", err)
@@ -0,0 +1,167 @@
+package cli
+
+import (
+	"database/sql"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"cdr.dev/slog/v3"
+	"cdr.dev/slog/v3/sloggers/slogtest"
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbgen"
+	"github.com/coder/coder/v2/coderd/database/dbtestutil"
+	"github.com/coder/coder/v2/codersdk"
+	"github.com/coder/coder/v2/testutil"
+	"github.com/coder/serpent"
+)
+
+func fakeOIDCIssuerDiscovery(t *testing.T, issuer string) *httptest.Server {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/.well-known/openid-configuration" {
+			http.NotFound(w, r)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(map[string]string{
+			"issuer": issuer,
+		})
+	}))
+	t.Cleanup(srv.Close)
+	return srv
+}
+
+func newDeploymentValues(t *testing.T, issuerURL string, autoRepair bool) *codersdk.DeploymentValues {
+	t.Helper()
+	vals := &codersdk.DeploymentValues{}
+	require.NoError(t, vals.OIDC.IssuerURL.Set(issuerURL))
+	vals.OIDC.AutoRepairLinks = serpent.Bool(autoRepair)
+	return vals
+}
+
+func TestOIDCAuthLinks(t *testing.T) {
+	t.Parallel()
+
+	t.Run("RepairsMismatchedLinks", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitShort)
+		logger := testutil.Logger(t)
+
+		const expectedIssuer = "https://accounts.google.com"
+		oidcSrv := fakeOIDCIssuerDiscovery(t, expectedIssuer)
+
+		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:  expectedIssuer + "||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",
+		})
+
+		vals := newDeploymentValues(t, oidcSrv.URL, true)
+		err := oidcAuthLinks(ctx, logger, oidcSrv.Client(), vals, db)
+		require.NoError(t, err)
+
+		// Mismatched link should be reset.
+		link, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
+			UserID:    mismatchedUser.ID,
+			LoginType: database.LoginTypeOIDC,
+		})
+		require.NoError(t, err)
+		require.Equal(t, "", link.LinkedID)
+
+		// Correct link should be untouched.
+		link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
+			UserID:    correctUser.ID,
+			LoginType: database.LoginTypeOIDC,
+		})
+		require.NoError(t, err)
+		require.Equal(t, expectedIssuer+"||sub-correct", link.LinkedID)
+	})
+
+	t.Run("AutoRepairDisabledSkipsReset", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitShort)
+		logger := testutil.Logger(t)
+
+		const expectedIssuer = "https://accounts.google.com"
+		oidcSrv := fakeOIDCIssuerDiscovery(t, expectedIssuer)
+
+		db, _ := dbtestutil.NewDB(t)
+
+		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",
+		})
+
+		vals := newDeploymentValues(t, oidcSrv.URL, false)
+		err := oidcAuthLinks(ctx, logger, oidcSrv.Client(), vals, db)
+		require.NoError(t, err)
+
+		// Link must not be modified when auto-repair is off.
+		link, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
+			UserID:    mismatchedUser.ID,
+			LoginType: database.LoginTypeOIDC,
+		})
+		require.NoError(t, err)
+		require.Equal(t, "https://old-issuer.example.com||sub-mismatched", link.LinkedID)
+	})
+
+	t.Run("DiscoveryFailureNonFatal", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitShort)
+		// oidcAuthLinks logs errors but does not return them.
+		logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
+
+		// Serve 500 so discovery fails.
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+		}))
+		t.Cleanup(srv.Close)
+
+		db, _ := dbtestutil.NewDB(t)
+
+		vals := newDeploymentValues(t, srv.URL, true)
+		err := oidcAuthLinks(ctx, logger, srv.Client(), vals, db)
+		require.NoError(t, err, "discovery failure must not be fatal")
+	})
+
+	t.Run("AnalyzeFailureNonFatal", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitShort)
+		// oidcAuthLinks logs errors but does not return them.
+		logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
+
+		const expectedIssuer = "https://accounts.google.com"
+		oidcSrv := fakeOIDCIssuerDiscovery(t, expectedIssuer)
+
+		// Use a closed DB so the analysis query fails.
+		connectionURL, err := dbtestutil.Open(t)
+		require.NoError(t, err)
+		sqlDB, err := sql.Open("postgres", connectionURL)
+		require.NoError(t, err)
+		db := database.New(sqlDB)
+		// Close the underlying connection so the query fails.
+		sqlDB.Close()
+
+		vals := newDeploymentValues(t, oidcSrv.URL, true)
+		err = oidcAuthLinks(ctx, logger, oidcSrv.Client(), vals, db)
+		require.NoError(t, err, "analysis failure must not be fatal")
+	})
+}
@@ -428,6 +428,12 @@ oidc:
  # setting can also break OIDC, so use with caution.
  # (default: <unset>, type: url)
  oidc-redirect-url:
+  # OIDC based users require the IdP issuer and subject in the claims to be static.
+  # If a new provider is configured, this option is required to be 'true'. It will
+  # reset any existing users to the previous provider, and match by email on their
+  # next login.
+  # (default: true, type: bool)
+  oidc-repair-links: true
 # Telemetry is critical to our ability to improve Coder. We strip all personal
 #  information before sending data to our servers. Please only disable telemetry
 #  when required by your organization's security policy.