mirror of
https://github.com/coder/coder.git
synced 2026-06-06 06:28:20 +00:00
Garrett Delfosse
d5b0e93c6c
## Problem The OIDC callback checks `email_verified` via a Go type assertion (`verifiedRaw.(bool)`). When an IdP returns the claim as a string (`"false"`), a number, or omits it entirely, the assertion fails silently and the email is implicitly treated as verified. Several real IdPs (SAML-to-OIDC bridges, certain Azure AD B2C configurations) emit string-typed booleans, making this reachable in practice. ## Fix Add `coerceEmailVerified()` to handle `bool`, `string` (`"true"`/`"false"`/`"1"`/`"0"` via `strconv.ParseBool`), `float64`, `json.Number`, and `int`/`int64` variants. Rewrite the check to be fail-closed: an absent claim, an unrecognized type, or any non-truthy value is treated as unverified and rejected. The existing `IgnoreEmailVerified` config option remains as an escape hatch. Fixes https://linear.app/codercom/issue/PLAT-228 > Generated with [Coder Agents](https://coder.com) by @f0ssel <details><summary>Implementation plan</summary> ### Production code (`coderd/userauth.go`) - Added `encoding/json` import - Added `coerceEmailVerified(v interface{}) (verified bool, ok bool)` helper near EOF - Replaced the type-assertion block (lines ~1342-1363) with fail-closed logic that uses `coerceEmailVerified` ### Unit tests (`coderd/userauth_internal_test.go`, new file) - Table-driven test covering: `bool`, `string` (`"true"`, `"false"`, `"1"`, `"0"`, `"TRUE"`, `"t"`, `"f"`, `"invalid"`, `""`), `json.Number`, `float64`, `int`, `int64`, `nil`, `[]string{}`, `map[string]string{}` ### Integration tests (`coderd/userauth_test.go`, `coderd/users_test.go`) - Added 3 new test cases: `EmailVerifiedMissingIgnored` (200), `EmailVerifiedAsStringTrue` (200), `EmailVerifiedAsStringFalse` (403) - Updated existing test cases that omitted `email_verified` and expected success to include `"email_verified": true` ### FakeIDP (`coderd/coderdtest/oidctest/idp.go`) - `encodeClaims` now defaults `email_verified` to `true` (like `exp`, `aud`, `iss`) so tests that don't care about the verification flow are unaffected </details>
66 lines
2.3 KiB
Go
66 lines
2.3 KiB
Go
package coderd
|
|
|
|
import (
|
|
"encoding/json"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
func TestCoerceEmailVerified(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
input interface{}
|
|
wantBool bool
|
|
wantOK bool
|
|
}{
|
|
// Native booleans
|
|
{name: "BoolTrue", input: true, wantBool: true, wantOK: true},
|
|
{name: "BoolFalse", input: false, wantBool: false, wantOK: true},
|
|
|
|
// Strings
|
|
{name: "StringTrue", input: "true", wantBool: true, wantOK: true},
|
|
{name: "StringFalse", input: "false", wantBool: false, wantOK: true},
|
|
{name: "StringOne", input: "1", wantBool: true, wantOK: true},
|
|
{name: "StringZero", input: "0", wantBool: false, wantOK: true},
|
|
{name: "StringTRUE", input: "TRUE", wantBool: true, wantOK: true},
|
|
{name: "StringFALSE", input: "FALSE", wantBool: false, wantOK: true},
|
|
{name: "StringT", input: "t", wantBool: true, wantOK: true},
|
|
{name: "StringF", input: "f", wantBool: false, wantOK: true},
|
|
{name: "StringInvalid", input: "invalid", wantBool: false, wantOK: false},
|
|
{name: "StringEmpty", input: "", wantBool: false, wantOK: false},
|
|
|
|
// json.Number (when decoder uses UseNumber)
|
|
{name: "JSONNumberOne", input: json.Number("1"), wantBool: true, wantOK: true},
|
|
{name: "JSONNumberZero", input: json.Number("0"), wantBool: false, wantOK: true},
|
|
{name: "JSONNumberInvalid", input: json.Number("abc"), wantBool: false, wantOK: false},
|
|
|
|
// float64 (default JSON numeric type)
|
|
{name: "Float64One", input: float64(1), wantBool: true, wantOK: true},
|
|
{name: "Float64Zero", input: float64(0), wantBool: false, wantOK: true},
|
|
|
|
// Integer types
|
|
{name: "IntOne", input: int(1), wantBool: true, wantOK: true},
|
|
{name: "IntZero", input: int(0), wantBool: false, wantOK: true},
|
|
{name: "Int64One", input: int64(1), wantBool: true, wantOK: true},
|
|
{name: "Int64Zero", input: int64(0), wantBool: false, wantOK: true},
|
|
|
|
// Nil and unsupported types
|
|
{name: "Nil", input: nil, wantBool: false, wantOK: false},
|
|
{name: "Slice", input: []string{}, wantBool: false, wantOK: false},
|
|
{name: "Map", input: map[string]string{}, wantBool: false, wantOK: false},
|
|
}
|
|
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
gotBool, gotOK := coerceEmailVerified(tc.input)
|
|
assert.Equal(t, tc.wantBool, gotBool, "bool value mismatch")
|
|
assert.Equal(t, tc.wantOK, gotOK, "ok value mismatch")
|
|
})
|
|
}
|
|
}
|