feat: implement disabling oidc issuer checks (#13991)

* use DANGEROUS prefix and drop a warning log

cli/server.go

@@ -106,7 +106,7 @@ import (
 	"github.com/coder/coder/v2/tailnet"
 )
 
-func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*coderd.OIDCConfig, error) {
+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!")
 	}
@@ -114,6 +114,12 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
 		return nil, xerrors.Errorf("OIDC issuer URL must be set!")
 	}
 
+	// Skipping issuer checks is not recommended.
+	if vals.OIDC.SkipIssuerChecks {
+		logger.Warn(ctx, "issuer checks with OIDC is disabled. This is not recommended as it can compromise the security of the authentication")
+		ctx = oidc.InsecureIssuerURLContext(ctx, vals.OIDC.IssuerURL.String())
+	}
+
 	oidcProvider, err := oidc.NewProvider(
 		ctx, vals.OIDC.IssuerURL.String(),
 	)
@@ -167,6 +173,9 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
 		Provider:     oidcProvider,
 		Verifier: oidcProvider.Verifier(&oidc.Config{
 			ClientID: vals.OIDC.ClientID.String(),
+			// Enabling this skips checking the "iss" claim in the token
+			// matches the issuer URL. This is not recommended.
+			SkipIssuerCheck: vals.OIDC.SkipIssuerChecks.Value(),
 		}),
 		EmailDomain:         vals.OIDC.EmailDomain,
 		AllowSignups:        vals.OIDC.AllowSignups.Value(),
@@ -657,7 +666,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				// Missing:
 				//	- Userinfo
 				//	- Verify
-				oc, err := createOIDCConfig(ctx, vals)
+				oc, err := createOIDCConfig(ctx, options.Logger, vals)
 				if err != nil {
 					return xerrors.Errorf("create oidc config: %w", err)
 				}

cli/testdata/coder_server_--help.golden

@@ -513,6 +513,12 @@ OIDC OPTIONS:
           The custom text to show on the error page informing about disabled
           OIDC signups. Markdown format is supported.
 
+      --dangerous-oidc-skip-issuer-checks bool, $CODER_DANGEROUS_OIDC_SKIP_ISSUER_CHECKS
+          OIDC issuer urls must match in the request, the id_token 'iss' claim,
+          and in the well-known configuration. This flag disables that
+          requirement, and can lead to an insecure OIDC configuration. It is not
+          recommended to use this flag.
+
 PROVISIONING OPTIONS: 
 Tune the behavior of the provisioner, which is responsible for creating,
 updating, and deleting workspace resources.

cli/testdata/server-config.yaml.golden

@@ -364,6 +364,11 @@ oidc:
   # Markdown format is supported.
   # (default: <unset>, type: string)
   signupsDisabledText: ""
+  # OIDC issuer urls must match in the request, the id_token 'iss' claim, and in the
+  # well-known configuration. This flag disables that requirement, and can lead to
+  # an insecure OIDC configuration. It is not recommended to use this flag.
+  # (default: <unset>, type: bool)
+  dangerousSkipIssuerChecks: false
 # 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.

coderd/apidoc/docs.go

@@ -10533,6 +10533,9 @@ const docTemplate = `{
                 "signups_disabled_text": {
                     "type": "string"
                 },
+                "skip_issuer_checks": {
+                    "type": "boolean"
+                },
                 "user_role_field": {
                     "type": "string"
                 },

coderd/apidoc/swagger.json

@@ -9480,6 +9480,9 @@
         "signups_disabled_text": {
           "type": "string"
         },
+        "skip_issuer_checks": {
+          "type": "boolean"
+        },
         "user_role_field": {
           "type": "string"
         },

coderd/coderdtest/oidctest/idp.go

@@ -97,6 +97,9 @@ type FakeIDP struct {
 	deviceCode *syncmap.Map[string, deviceFlow]
 
 	// hooks
+	// hookWellKnown allows mutating the returned .well-known/configuration JSON.
+	// Using this can break the IDP configuration, so be careful.
+	hookWellKnown func(r *http.Request, j *ProviderJSON) error
 	// hookValidRedirectURL can be used to reject a redirect url from the
 	// IDP -> Application. Almost all IDPs have the concept of
 	// "Authorized Redirect URLs". This can be used to emulate that.
@@ -151,6 +154,12 @@ func WithMiddlewares(mws ...func(http.Handler) http.Handler) func(*FakeIDP) {
 	}
 }
 
+func WithHookWellKnown(hook func(r *http.Request, j *ProviderJSON) error) func(*FakeIDP) {
+	return func(f *FakeIDP) {
+		f.hookWellKnown = hook
+	}
+}
+
 // WithRefresh is called when a refresh token is used. The email is
 // the email of the user that is being refreshed assuming the claims are correct.
 func WithRefresh(hook func(email string) error) func(*FakeIDP) {
@@ -753,7 +762,16 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler {
 	mux.Get("/.well-known/openid-configuration", func(rw http.ResponseWriter, r *http.Request) {
 		f.logger.Info(r.Context(), "http OIDC config", slogRequestFields(r)...)
 
-		_ = json.NewEncoder(rw).Encode(f.provider)
+		cpy := f.provider
+		if f.hookWellKnown != nil {
+			err := f.hookWellKnown(r, &cpy)
+			if err != nil {
+				http.Error(rw, err.Error(), http.StatusInternalServerError)
+				return
+			}
+		}
+
+		_ = json.NewEncoder(rw).Encode(cpy)
 	})
 
 	// Authorize is called when the user is redirected to the IDP to login.
@@ -1371,8 +1389,11 @@ func (f *FakeIDP) AppCredentials() (clientID string, clientSecret string) {
 	return f.clientID, f.clientSecret
 }
 
-// OIDCConfig returns the OIDC config to use for Coderd.
-func (f *FakeIDP) OIDCConfig(t testing.TB, scopes []string, opts ...func(cfg *coderd.OIDCConfig)) *coderd.OIDCConfig {
+func (f *FakeIDP) PublicKey() crypto.PublicKey {
+	return f.key.Public()
+}
+
+func (f *FakeIDP) OauthConfig(t testing.TB, scopes []string) *oauth2.Config {
 	t.Helper()
 
 	if len(scopes) == 0 {
@@ -1391,22 +1412,50 @@ func (f *FakeIDP) OIDCConfig(t testing.TB, scopes []string, opts ...func(cfg *co
 		RedirectURL: "https://redirect.com",
 		Scopes:      scopes,
 	}
+	f.cfg = oauthCfg
 
-	ctx := oidc.ClientContext(context.Background(), f.HTTPClient(nil))
+	return oauthCfg
+}
+
+func (f *FakeIDP) OIDCConfigSkipIssuerChecks(t testing.TB, scopes []string, opts ...func(cfg *coderd.OIDCConfig)) *coderd.OIDCConfig {
+	ctx := oidc.InsecureIssuerURLContext(context.Background(), f.issuer)
+
+	return f.internalOIDCConfig(ctx, t, scopes, func(config *oidc.Config) {
+		config.SkipIssuerCheck = true
+	}, opts...)
+}
+
+func (f *FakeIDP) OIDCConfig(t testing.TB, scopes []string, opts ...func(cfg *coderd.OIDCConfig)) *coderd.OIDCConfig {
+	return f.internalOIDCConfig(context.Background(), t, scopes, nil, opts...)
+}
+
+// OIDCConfig returns the OIDC config to use for Coderd.
+func (f *FakeIDP) internalOIDCConfig(ctx context.Context, t testing.TB, scopes []string, verifierOpt func(config *oidc.Config), opts ...func(cfg *coderd.OIDCConfig)) *coderd.OIDCConfig {
+	t.Helper()
+
+	oauthCfg := f.OauthConfig(t, scopes)
+
+	ctx = oidc.ClientContext(ctx, f.HTTPClient(nil))
 	p, err := oidc.NewProvider(ctx, f.provider.Issuer)
 	require.NoError(t, err, "failed to create OIDC provider")
+
+	verifierConfig := &oidc.Config{
+		ClientID: oauthCfg.ClientID,
+		SupportedSigningAlgs: []string{
+			"RS256",
+		},
+		// Todo: add support for Now()
+	}
+	if verifierOpt != nil {
+		verifierOpt(verifierConfig)
+	}
+
 	cfg := &coderd.OIDCConfig{
 		OAuth2Config: oauthCfg,
 		Provider:     p,
 		Verifier: oidc.NewVerifier(f.provider.Issuer, &oidc.StaticKeySet{
 			PublicKeys: []crypto.PublicKey{f.key.Public()},
-		}, &oidc.Config{
-			ClientID: oauthCfg.ClientID,
-			SupportedSigningAlgs: []string{
-				"RS256",
-			},
-			// Todo: add support for Now()
-		}),
+		}, verifierConfig),
 		UsernameField: "preferred_username",
 		EmailField:    "email",
 		AuthURLParams: map[string]string{"access_type": "offline"},
@@ -1419,13 +1468,12 @@ func (f *FakeIDP) OIDCConfig(t testing.TB, scopes []string, opts ...func(cfg *co
 		opt(cfg)
 	}
 
-	f.cfg = oauthCfg
 	return cfg
 }
 
 func (f *FakeIDP) getClaims(m *syncmap.Map[string, jwt.MapClaims], key string) (jwt.MapClaims, bool) {
 	v, ok := m.Load(key)
-	if !ok {
+	if !ok || v == nil {
 		if f.defaultIDClaims != nil {
 			return f.defaultIDClaims, true
 		}

coderd/coderdtest/oidctest/idp_test.go

@@ -2,19 +2,22 @@ package oidctest_test
 
 import (
 	"context"
+	"crypto"
 	"net/http"
-	"net/http/httptest"
 	"testing"
 	"time"
 
 	"github.com/golang-jwt/jwt/v4"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/xerrors"
 
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
 
+	"github.com/coder/coder/v2/coderd"
 	"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
+	"github.com/coder/coder/v2/testutil"
 )
 
 // TestFakeIDPBasicFlow tests the basic flow of the fake IDP.
@@ -27,12 +30,6 @@ func TestFakeIDPBasicFlow(t *testing.T) {
 		oidctest.WithLogging(t, nil),
 	)
 
-	var handler http.Handler
-	srv := httptest.NewServer(http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		handler.ServeHTTP(w, r)
-	})))
-	defer srv.Close()
-
 	cfg := fake.OIDCConfig(t, nil)
 	cli := fake.HTTPClient(nil)
 	ctx := oidc.ClientContext(context.Background(), cli)
@@ -71,3 +68,84 @@ func TestFakeIDPBasicFlow(t *testing.T) {
 	require.NoError(t, err, "failed to refresh token")
 	require.NotEmpty(t, refreshed.AccessToken, "access token is empty on refresh")
 }
+
+// TestIDPIssuerMismatch emulates a situation where the IDP issuer url does
+// not match the one in the well-known config and claims.
+// This can happen in some edge cases and in some azure configurations.
+//
+// This test just makes sure a fake IDP can set up this scenario.
+func TestIDPIssuerMismatch(t *testing.T) {
+	t.Parallel()
+
+	const proxyURL = "https://proxy.com"
+	const primaryURL = "https://primary.com"
+
+	fake := oidctest.NewFakeIDP(t,
+		oidctest.WithIssuer(proxyURL),
+		oidctest.WithDefaultIDClaims(jwt.MapClaims{
+			"iss": primaryURL,
+		}),
+		oidctest.WithHookWellKnown(func(r *http.Request, j *oidctest.ProviderJSON) error {
+			// host should be proxy.com, but we return the primaryURL
+			if r.Host != "proxy.com" {
+				return xerrors.Errorf("unexpected host: %s", r.Host)
+			}
+			j.Issuer = primaryURL
+			return nil
+		}),
+		oidctest.WithLogging(t, nil),
+	)
+
+	ctx := testutil.Context(t, testutil.WaitMedium)
+	// Do not use real network requests
+	cli := fake.HTTPClient(nil)
+	ctx = oidc.ClientContext(ctx, cli)
+
+	// Allow the issuer mismatch
+	verifierContext := oidc.InsecureIssuerURLContext(ctx, "this field does not matter")
+	p, err := oidc.NewProvider(verifierContext, "https://proxy.com")
+	require.NoError(t, err, "failed to create OIDC provider")
+
+	oauthConfig := fake.OauthConfig(t, nil)
+	cfg := &coderd.OIDCConfig{
+		OAuth2Config: oauthConfig,
+		Provider:     p,
+		Verifier: oidc.NewVerifier(fake.WellknownConfig().Issuer, &oidc.StaticKeySet{
+			PublicKeys: []crypto.PublicKey{fake.PublicKey()},
+		}, &oidc.Config{
+			SkipIssuerCheck: true,
+			ClientID:        oauthConfig.ClientID,
+			SupportedSigningAlgs: []string{
+				"RS256",
+			},
+		}),
+		UsernameField: "preferred_username",
+		EmailField:    "email",
+		AuthURLParams: map[string]string{"access_type": "offline"},
+	}
+
+	const expectedState = "random-state"
+	var token *oauth2.Token
+
+	fake.SetCoderdCallbackHandler(func(w http.ResponseWriter, r *http.Request) {
+		// Emulate OIDC flow
+		code := r.URL.Query().Get("code")
+		state := r.URL.Query().Get("state")
+		assert.Equal(t, expectedState, state, "state mismatch")
+
+		oauthToken, err := cfg.Exchange(ctx, code)
+		if assert.NoError(t, err, "failed to exchange code") {
+			assert.NotEmpty(t, oauthToken.AccessToken, "access token is empty")
+			assert.NotEmpty(t, oauthToken.RefreshToken, "refresh token is empty")
+		}
+		token = oauthToken
+	})
+
+	//nolint:bodyclose
+	resp := fake.OIDCCallback(t, expectedState, nil) // Use default claims
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+
+	idToken, err := cfg.Verifier.Verify(ctx, token.Extra("id_token").(string))
+	require.NoError(t, err)
+	require.Equal(t, primaryURL, idToken.Issuer)
+}

coderd/userauth_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/cookiejar"
 	"net/url"
@@ -884,6 +885,7 @@ func TestUserOIDC(t *testing.T) {
 		EmailDomain         []string
 		AssertUser          func(t testing.TB, u codersdk.User)
 		StatusCode          int
+		AssertResponse      func(t testing.TB, resp *http.Response)
 		IgnoreEmailVerified bool
 		IgnoreUserInfo      bool
 	}{
@@ -1224,6 +1226,21 @@ func TestUserOIDC(t *testing.T) {
 			AllowSignups: true,
 			StatusCode:   http.StatusOK,
 		},
+		{
+			Name: "IssuerMismatch",
+			IDTokenClaims: jwt.MapClaims{
+				"iss":            "https://mismatch.com",
+				"email":          "user@domain.tld",
+				"email_verified": true,
+			},
+			AllowSignups: true,
+			StatusCode:   http.StatusBadRequest,
+			AssertResponse: func(t testing.TB, resp *http.Response) {
+				data, err := io.ReadAll(resp.Body)
+				require.NoError(t, err)
+				require.Contains(t, string(data), "id token issued by a different provider")
+			},
+		},
 	} {
 		tc := tc
 		t.Run(tc.Name, func(t *testing.T) {
@@ -1255,6 +1272,9 @@ func TestUserOIDC(t *testing.T) {
 			client, resp := fake.AttemptLogin(t, owner, tc.IDTokenClaims)
 			numLogs++ // add an audit log for login
 			require.Equal(t, tc.StatusCode, resp.StatusCode)
+			if tc.AssertResponse != nil {
+				tc.AssertResponse(t, resp)
+			}
 
 			ctx := testutil.Context(t, testutil.WaitLong)
 
@@ -1532,6 +1552,51 @@ func TestUserLogout(t *testing.T) {
 	}
 }
 
+// TestOIDCSkipIssuer verifies coderd can run without checking the issuer url
+// in the OIDC exchange. This means the CODER_OIDC_ISSUER_URL does not need
+// to match the id_token `iss` field, or the value returned in the well-known
+// config.
+//
+// So this test has:
+// - OIDC at http://localhost:<port>
+// - well-known config with issuer https://primary.com
+// - JWT with issuer https://secondary.com
+//
+// Without this security check disabled, all three above would have to match.
+func TestOIDCSkipIssuer(t *testing.T) {
+	t.Parallel()
+	const primaryURLString = "https://primary.com"
+	const secondaryURLString = "https://secondary.com"
+	primaryURL := must(url.Parse(primaryURLString))
+
+	fake := oidctest.NewFakeIDP(t,
+		oidctest.WithServing(),
+		oidctest.WithDefaultIDClaims(jwt.MapClaims{}),
+		oidctest.WithHookWellKnown(func(r *http.Request, j *oidctest.ProviderJSON) error {
+			assert.NotEqual(t, r.URL.Host, primaryURL.Host, "request went to wrong host")
+			j.Issuer = primaryURLString
+			return nil
+		}),
+	)
+
+	owner := coderdtest.New(t, &coderdtest.Options{
+		OIDCConfig: fake.OIDCConfigSkipIssuerChecks(t, nil, func(cfg *coderd.OIDCConfig) {
+			cfg.AllowSignups = true
+		}),
+	})
+
+	// User can login and use their token.
+	ctx := testutil.Context(t, testutil.WaitShort)
+	//nolint:bodyclose
+	userClient, _ := fake.Login(t, owner, jwt.MapClaims{
+		"iss":   secondaryURLString,
+		"email": "alice@coder.com",
+	})
+	found, err := userClient.User(ctx, "me")
+	require.NoError(t, err)
+	require.Equal(t, found.LoginType, codersdk.LoginTypeOIDC)
+}
+
 func oauth2Callback(t *testing.T, client *codersdk.Client) *http.Response {
 	client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
 		return http.ErrUseLastResponse

codersdk/deployment.go

@@ -523,6 +523,7 @@ type OIDCConfig struct {
 	SignInText          serpent.String                      `json:"sign_in_text" typescript:",notnull"`
 	IconURL             serpent.URL                         `json:"icon_url" typescript:",notnull"`
 	SignupsDisabledText serpent.String                      `json:"signups_disabled_text" typescript:",notnull"`
+	SkipIssuerChecks    serpent.Bool                        `json:"skip_issuer_checks" typescript:",notnull"`
 }
 
 type TelemetryConfig struct {
@@ -1644,6 +1645,16 @@ when required by your organization's security policy.`,
 			Group:       &deploymentGroupOIDC,
 			YAML:        "signupsDisabledText",
 		},
+		{
+			Name: "Skip OIDC issuer checks (not recommended)",
+			Description: "OIDC issuer urls must match in the request, the id_token 'iss' claim, and in the well-known configuration. " +
+				"This flag disables that requirement, and can lead to an insecure OIDC configuration. It is not recommended to use this flag.",
+			Flag:  "dangerous-oidc-skip-issuer-checks",
+			Env:   "CODER_DANGEROUS_OIDC_SKIP_ISSUER_CHECKS",
+			Value: &c.OIDC.SkipIssuerChecks,
+			Group: &deploymentGroupOIDC,
+			YAML:  "dangerousSkipIssuerChecks",
+		},
 		// Telemetry settings
 		{
 			Name:        "Telemetry Enable",

docs/api/general.md

@@ -347,6 +347,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \
       "scopes": ["string"],
       "sign_in_text": "string",
       "signups_disabled_text": "string",
+      "skip_issuer_checks": true,
       "user_role_field": "string",
       "user_role_mapping": {},
       "user_roles_default": ["string"],

docs/api/schemas.md

@@ -1804,6 +1804,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
       "scopes": ["string"],
       "sign_in_text": "string",
       "signups_disabled_text": "string",
+      "skip_issuer_checks": true,
       "user_role_field": "string",
       "user_role_mapping": {},
       "user_roles_default": ["string"],
@@ -2226,6 +2227,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
     "scopes": ["string"],
     "sign_in_text": "string",
     "signups_disabled_text": "string",
+    "skip_issuer_checks": true,
     "user_role_field": "string",
     "user_role_mapping": {},
     "user_roles_default": ["string"],
@@ -3523,6 +3525,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
   "scopes": ["string"],
   "sign_in_text": "string",
   "signups_disabled_text": "string",
+  "skip_issuer_checks": true,
   "user_role_field": "string",
   "user_role_mapping": {},
   "user_roles_default": ["string"],
@@ -3555,6 +3558,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
 | `scopes`                | array of string                  | false    |              |                                                                                  |
 | `sign_in_text`          | string                           | false    |              |                                                                                  |
 | `signups_disabled_text` | string                           | false    |              |                                                                                  |
+| `skip_issuer_checks`    | boolean                          | false    |              |                                                                                  |
 | `user_role_field`       | string                           | false    |              |                                                                                  |
 | `user_role_mapping`     | object                           | false    |              |                                                                                  |
 | `user_roles_default`    | array of string                  | false    |              |                                                                                  |

docs/cli/server.md

@@ -673,6 +673,16 @@ URL pointing to the icon to use on the OpenID Connect login button.
 
 The custom text to show on the error page informing about disabled OIDC signups. Markdown format is supported.
 
+### --dangerous-oidc-skip-issuer-checks
+
+|             |                                                       |
+| ----------- | ----------------------------------------------------- |
+| Type        | <code>bool</code>                                     |
+| Environment | <code>$CODER_DANGEROUS_OIDC_SKIP_ISSUER_CHECKS</code> |
+| YAML        | <code>oidc.dangerousSkipIssuerChecks</code>           |
+
+OIDC issuer urls must match in the request, the id_token 'iss' claim, and in the well-known configuration. This flag disables that requirement, and can lead to an insecure OIDC configuration. It is not recommended to use this flag.
+
 ### --telemetry
 
 |             |                                      |

enterprise/cli/testdata/coder_server_--help.golden

@@ -514,6 +514,12 @@ OIDC OPTIONS:
           The custom text to show on the error page informing about disabled
           OIDC signups. Markdown format is supported.
 
+      --dangerous-oidc-skip-issuer-checks bool, $CODER_DANGEROUS_OIDC_SKIP_ISSUER_CHECKS
+          OIDC issuer urls must match in the request, the id_token 'iss' claim,
+          and in the well-known configuration. This flag disables that
+          requirement, and can lead to an insecure OIDC configuration. It is not
+          recommended to use this flag.
+
 PROVISIONING OPTIONS: 
 Tune the behavior of the provisioner, which is responsible for creating,
 updating, and deleting workspace resources.

site/src/api/typesGenerated.ts

@@ -850,6 +850,7 @@ export interface OIDCConfig {
   readonly sign_in_text: string;
   readonly icon_url: string;
   readonly signups_disabled_text: string;
+  readonly skip_issuer_checks: boolean;
 }
 
 // From codersdk/organizations.go