Skip to content

Commit 9577cc8

Browse files
johnstcnmafredri
authored and
pull[bot]
committed
feat: add CODER_OIDC_IGNORE_EMAIL_VERIFIED config knob (#5165)
* Adds a configuration knob CODER_OIDC_IGNORE_EMAIL_VERIFIED that allows ignoring the email_verified OIDC claim * Adds warning message at startup if CODER_OIDC_IGNORE_EMAIL_VERIFIED=true * Adds warning whenever an unverified OIDC email is let through * Skips flaky test on non-linux platforms Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
1 parent aff5897 commit 9577cc8

File tree

10 files changed

+91
-26
lines changed

10 files changed

+91
-26
lines changed

‎cli/deployment/config.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,12 @@ func newConfig() *codersdk.DeploymentConfig {
231231
Flag: "oidc-scopes",
232232
Default: []string{oidc.ScopeOpenID, "profile", "email"},
233233
},
234+
IgnoreEmailVerified: &codersdk.DeploymentConfigField[bool]{
235+
Name: "OIDC Ignore Email Verified",
236+
Usage: "Ignore the email_verified claim from the upstream provider.",
237+
Flag: "oidc-ignore-email-verified",
238+
Default: false,
239+
},
234240
},
235241

236242
Telemetry: &codersdk.TelemetryConfig{

‎cli/deployment/config_test.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,23 +122,37 @@ func TestConfig(t *testing.T) {
122122
require.Equal(t, config.Trace.Enable.Value, true)
123123
require.Equal(t, config.Trace.HoneycombAPIKey.Value, "my-honeycomb-key")
124124
},
125+
}, {
126+
Name: "OIDC_Defaults",
127+
Env: map[string]string{},
128+
Valid: func(config *codersdk.DeploymentConfig) {
129+
require.Empty(t, config.OIDC.IssuerURL.Value)
130+
require.Empty(t, config.OIDC.EmailDomain.Value)
131+
require.Empty(t, config.OIDC.ClientID.Value)
132+
require.Empty(t, config.OIDC.ClientSecret.Value)
133+
require.True(t, config.OIDC.AllowSignups.Value)
134+
require.ElementsMatch(t, config.OIDC.Scopes.Value, []string{"openid", "email", "profile"})
135+
require.False(t, config.OIDC.IgnoreEmailVerified.Value)
136+
},
125137
}, {
126138
Name: "OIDC",
127139
Env: map[string]string{
128-
"CODER_OIDC_ISSUER_URL": "https://accounts.google.com",
129-
"CODER_OIDC_EMAIL_DOMAIN": "coder.com",
130-
"CODER_OIDC_CLIENT_ID": "client",
131-
"CODER_OIDC_CLIENT_SECRET": "secret",
132-
"CODER_OIDC_ALLOW_SIGNUPS": "false",
133-
"CODER_OIDC_SCOPES": "something,here",
140+
"CODER_OIDC_ISSUER_URL": "https://accounts.google.com",
141+
"CODER_OIDC_EMAIL_DOMAIN": "coder.com",
142+
"CODER_OIDC_CLIENT_ID": "client",
143+
"CODER_OIDC_CLIENT_SECRET": "secret",
144+
"CODER_OIDC_ALLOW_SIGNUPS": "false",
145+
"CODER_OIDC_SCOPES": "something,here",
146+
"CODER_OIDC_IGNORE_EMAIL_VERIFIED": "true",
134147
},
135148
Valid: func(config *codersdk.DeploymentConfig) {
136149
require.Equal(t, config.OIDC.IssuerURL.Value, "https://accounts.google.com")
137150
require.Equal(t, config.OIDC.EmailDomain.Value, "coder.com")
138151
require.Equal(t, config.OIDC.ClientID.Value, "client")
139152
require.Equal(t, config.OIDC.ClientSecret.Value, "secret")
140-
require.Equal(t, config.OIDC.AllowSignups.Value, false)
153+
require.False(t, config.OIDC.AllowSignups.Value)
141154
require.Equal(t, config.OIDC.Scopes.Value, []string{"something", "here"})
155+
require.True(t, config.OIDC.IgnoreEmailVerified.Value)
142156
},
143157
}, {
144158
Name: "GitHub",

‎cli/server.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,10 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
398398
return xerrors.Errorf("configure oidc client certificates: %w", err)
399399
}
400400

401+
if cfg.OIDC.IgnoreEmailVerified.Value {
402+
logger.Warn(ctx, "coder will not check email_verified for OIDC logins")
403+
}
404+
401405
oidcProvider, err := oidc.NewProvider(ctx, cfg.OIDC.IssuerURL.Value)
402406
if err != nil {
403407
return xerrors.Errorf("configure oidc provider: %w", err)

‎cli/testdata/coder_server_--help.golden

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ Flags:
9898
--oidc-email-domain string Email domain that clients logging in with
9999
OIDC must match.
100100
Consumes $CODER_OIDC_EMAIL_DOMAIN
101+
--oidc-ignore-email-verified Ignore the email_verified claim from the
102+
upstream provider.
103+
Consumes $CODER_OIDC_IGNORE_EMAIL_VERIFIED
101104
--oidc-issuer-url string Issuer URL to use for Login with OIDC.
102105
Consumes $CODER_OIDC_ISSUER_URL
103106
--oidc-scopes strings Scopes to grant when authenticating with

‎coderd/userauth.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ type OIDCConfig struct {
195195
// EmailDomain is the domain to enforce when a user authenticates.
196196
EmailDomain string
197197
AllowSignups bool
198+
// IgnoreEmailVerified allows ignoring the email_verified claim
199+
// from an upstream OIDC provider. See #5065 for context.
200+
IgnoreEmailVerified bool
198201
}
199202

200203
func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
@@ -264,10 +267,13 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
264267
if ok {
265268
verified, ok := verifiedRaw.(bool)
266269
if ok && !verified {
267-
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
268-
Message: fmt.Sprintf("Verify the %q email address on your OIDC provider to authenticate!", email),
269-
})
270-
return
270+
if !api.OIDCConfig.IgnoreEmailVerified {
271+
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
272+
Message: fmt.Sprintf("Verify the %q email address on your OIDC provider to authenticate!", email),
273+
})
274+
return
275+
}
276+
api.Logger.Warn(ctx, "allowing unverified oidc email %q")
271277
}
272278
}
273279
// The username is a required property in Coder. We make a best-effort

‎coderd/userauth_test.go

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -479,13 +479,14 @@ func TestUserOIDC(t *testing.T) {
479479
t.Parallel()
480480

481481
for _, tc := range []struct {
482-
Name string
483-
Claims jwt.MapClaims
484-
AllowSignups bool
485-
EmailDomain string
486-
Username string
487-
AvatarURL string
488-
StatusCode int
482+
Name string
483+
Claims jwt.MapClaims
484+
AllowSignups bool
485+
EmailDomain string
486+
Username string
487+
AvatarURL string
488+
StatusCode int
489+
IgnoreEmailVerified bool
489490
}{{
490491
Name: "EmailOnly",
491492
Claims: jwt.MapClaims{
@@ -502,6 +503,24 @@ func TestUserOIDC(t *testing.T) {
502503
},
503504
AllowSignups: true,
504505
StatusCode: http.StatusForbidden,
506+
}, {
507+
Name: "EmailNotAString",
508+
Claims: jwt.MapClaims{
509+
"email": 3.14159,
510+
"email_verified": false,
511+
},
512+
AllowSignups: true,
513+
StatusCode: http.StatusBadRequest,
514+
}, {
515+
Name: "EmailNotVerifiedIgnored",
516+
Claims: jwt.MapClaims{
517+
"email": "kyle@kwc.io",
518+
"email_verified": false,
519+
},
520+
AllowSignups: true,
521+
StatusCode: http.StatusTemporaryRedirect,
522+
Username: "kyle",
523+
IgnoreEmailVerified: true,
505524
}, {
506525
Name: "NotInRequiredEmailDomain",
507526
Claims: jwt.MapClaims{
@@ -593,6 +612,7 @@ func TestUserOIDC(t *testing.T) {
593612
config := conf.OIDCConfig()
594613
config.AllowSignups = tc.AllowSignups
595614
config.EmailDomain = tc.EmailDomain
615+
config.IgnoreEmailVerified = tc.IgnoreEmailVerified
596616

597617
client := coderdtest.New(t, &coderdtest.Options{
598618
OIDCConfig: config,

‎codersdk/deploymentconfig.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,13 @@ type OAuth2GithubConfig struct {
8787
}
8888

8989
type OIDCConfig struct {
90-
AllowSignups *DeploymentConfigField[bool] `json:"allow_signups" typescript:",notnull"`
91-
ClientID *DeploymentConfigField[string] `json:"client_id" typescript:",notnull"`
92-
ClientSecret *DeploymentConfigField[string] `json:"client_secret" typescript:",notnull"`
93-
EmailDomain *DeploymentConfigField[string] `json:"email_domain" typescript:",notnull"`
94-
IssuerURL *DeploymentConfigField[string] `json:"issuer_url" typescript:",notnull"`
95-
Scopes *DeploymentConfigField[[]string] `json:"scopes" typescript:",notnull"`
90+
AllowSignups *DeploymentConfigField[bool] `json:"allow_signups" typescript:",notnull"`
91+
ClientID *DeploymentConfigField[string] `json:"client_id" typescript:",notnull"`
92+
ClientSecret *DeploymentConfigField[string] `json:"client_secret" typescript:",notnull"`
93+
EmailDomain *DeploymentConfigField[string] `json:"email_domain" typescript:",notnull"`
94+
IssuerURL *DeploymentConfigField[string] `json:"issuer_url" typescript:",notnull"`
95+
Scopes *DeploymentConfigField[[]string] `json:"scopes" typescript:",notnull"`
96+
IgnoreEmailVerified *DeploymentConfigField[bool] `json:"ignore_email_verified" typescript:",notnull"`
9697
}
9798

9899
type TelemetryConfig struct {

‎docs/admin/auth.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,21 @@ Once complete, run `sudo service coder restart` to reboot Coder.
7676
> When a new user is created, the `preferred_username` claim becomes the username. If this claim is empty, the email address will be stripped of the domain, and become the username (e.g. `example@coder.com` becomes `example`).
7777
7878
If your OpenID Connect provider requires client TLS certificates for authentication, you can configure them like so:
79+
7980
```console
8081
CODER_TLS_CLIENT_CERT_FILE=/path/to/cert.pem
8182
CODER_TLS_CLIENT_KEY_FILE=/path/to/key.pem
8283
```
8384

85+
Coder requires all OIDC email addresses to be verified by default. If the `email_verified` claim is present in the token response from the identity provider, Coder will validate that its value is `true`.
86+
If needed, you can disable this behavior with the following setting:
87+
88+
```console
89+
CODER_OIDC_IGNORE_EMAIL_VERIFIED=true
90+
```
91+
92+
> **Note:** This will cause Coder to implicitly treat all OIDC emails as "verified".
93+
8494
## SCIM (enterprise)
8595

8696
Coder supports user provisioning and deprovisioning via SCIM 2.0 with header

‎loadtest/reconnectingpty/run_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import (
2323

2424
func Test_Runner(t *testing.T) {
2525
t.Parallel()
26-
if runtime.GOOS == "windows" {
27-
t.Skip("PTY is flakey on Windows")
26+
if runtime.GOOS != "linux" {
27+
t.Skip("PTY is flakey on non-Linux platforms")
2828
}
2929

3030
t.Run("OK", func(t *testing.T) {

‎site/src/api/typesGenerated.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ export interface OIDCConfig {
441441
readonly email_domain: DeploymentConfigField<string>
442442
readonly issuer_url: DeploymentConfigField<string>
443443
readonly scopes: DeploymentConfigField<string[]>
444+
readonly ignore_email_verified: DeploymentConfigField<boolean>
444445
}
445446

446447
// From codersdk/organizations.go

0 commit comments

Comments
 (0)