--- a/cmd/admin_auth_oauth.go +++ b/cmd/admin_auth_oauth.go @@ -94,6 +94,10 @@ Name: "required-claim-value", Value: "", Usage: "Claim value that has to be set to allow users to login with this source", + }, + &cli.StringSliceFlag{ + Name: "required-claim-values", + Usage: "Require any one exact array value from current OpenID Connect UserInfo (cannot combine with required-claim-value)", }, &cli.StringFlag{ Name: "group-claim-name", @@ -160,6 +164,7 @@ Scopes: c.StringSlice("scopes"), RequiredClaimName: c.String("required-claim-name"), RequiredClaimValue: c.String("required-claim-value"), + RequiredClaimValues: c.StringSlice("required-claim-values"), GroupClaimName: c.String("group-claim-name"), AdminGroup: c.String("admin-group"), RestrictedGroup: c.String("restricted-group"), @@ -177,6 +182,12 @@ } config := parseOAuth2Config(c) + if err := oauth2.ValidateRequiredClaimValues(config.RequiredClaimName, config.RequiredClaimValue, config.RequiredClaimValues); err != nil { + return err + } + if config.RequiredClaimValues != nil && config.Provider != "openidConnect" { + return errors.New("required-claim-values requires openidConnect") + } if config.Provider == "openidConnect" { discoveryURL, err := url.Parse(config.OpenIDConnectAutoDiscoveryURL) if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") { @@ -246,6 +257,10 @@ oAuth2Config.RequiredClaimValue = c.String("required-claim-value") } + if c.IsSet("required-claim-values") { + oAuth2Config.RequiredClaimValues = c.StringSlice("required-claim-values") + } + if c.IsSet("group-claim-name") { oAuth2Config.GroupClaimName = c.String("group-claim-name") } @@ -293,6 +308,12 @@ } oAuth2Config.CustomURLMapping = customURLMapping + if err := oauth2.ValidateRequiredClaimValues(oAuth2Config.RequiredClaimName, oAuth2Config.RequiredClaimValue, oAuth2Config.RequiredClaimValues); err != nil { + return err + } + if oAuth2Config.RequiredClaimValues != nil && oAuth2Config.Provider != "openidConnect" { + return errors.New("required-claim-values requires openidConnect") + } source.Cfg = oAuth2Config return auth_model.UpdateSource(ctx, source) --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -322,6 +322,13 @@ ctx.Data["Source"] = source ctx.Data["HasTLS"] = source.HasTLS() + // The legacy form has no representation for the opt-in current-UserInfo gate. + // Refuse rather than silently dropping it during source reconstruction. + if current, ok := source.Cfg.(*oauth2.Source); ok && current.RequiredClaimValues != nil { + ctx.RenderWithErr("This OAuth source uses required-claim-values; update it through the reviewed CLI.", tplAuthEdit, form) + return + } + if ctx.HasError() { ctx.HTML(http.StatusOK, tplAuthEdit) return --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1427,7 +1427,11 @@ return goth.User{}, err } - if oauth2Source.RequiredClaimName != "" { + if oauth2Source.RequiredClaimValues != nil { + if oauth2Source.Provider != "openidConnect" || !oauth2.MatchRequiredClaimValues(gothUser.RawData, oauth2Source.RequiredClaimName, oauth2Source.RequiredClaimValue, oauth2Source.RequiredClaimValues) { + return goth.User{}, user_model.ErrUserProhibitLogin{Name: gothUser.UserID} + } + } else if oauth2Source.RequiredClaimName != "" { claimInterface, has := gothUser.RawData[oauth2Source.RequiredClaimName] if !has { return goth.User{}, user_model.ErrUserProhibitLogin{Name: gothUser.UserID} --- a/services/auth/source/oauth2/providers_openid.go +++ b/services/auth/source/oauth2/providers_openid.go @@ -4,6 +4,7 @@ package oauth2 import ( + "errors" "html/template" "forgejo.org/modules/log" @@ -43,6 +44,22 @@ if err != nil { log.Warn("Failed to create OpenID Connect Provider with name '%s' with url '%s': %v", providerName, source.OpenIDConnectAutoDiscoveryURL, err) } + if err != nil { + return nil, err + } + if source.RequiredClaimValues != nil { + if err := ValidateRequiredClaimValues(source.RequiredClaimName, source.RequiredClaimValue, source.RequiredClaimValues); err != nil { + return nil, err + } + if provider.OpenIDConfig == nil || provider.SkipUserInfoRequest { + return nil, errors.New("required-claim-values needs current UserInfo") + } + guarded, err := RequireCurrentUserInfoClaim(provider.Client(), provider.OpenIDConfig.UserInfoEndpoint, source.RequiredClaimName) + if err != nil { + return nil, err + } + provider.HTTPClient = guarded + } return provider, err } --- /dev/null +++ b/services/auth/source/oauth2/required_claim_values.go @@ -0,0 +1,202 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package oauth2 + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "net/http" + "net/url" + "strings" + "unicode" + "unicode/utf8" +) + +const maxClaimValues = 1024 +const maxUserInfoBytes = 1 << 20 + +// ValidateRequiredClaimValues validates only the opt-in list extension. A nil +// list retains every legacy single-value behavior. An explicitly empty list +// fails closed rather than turning a configured gate into claim-presence-only. +func ValidateRequiredClaimValues(name, legacy string, values []string) error { + if values == nil { + return nil + } + if !validClaimLiteral(name) || legacy != "" || len(values) == 0 || len(values) > 64 { + return errors.New("invalid or conflicting required-claim-values configuration") + } + seen := map[string]bool{} + for _, value := range values { + if !validClaimLiteral(value) || seen[value] { + return errors.New("invalid required-claim-values literal") + } + seen[value] = true + } + return nil +} + +func validClaimLiteral(value string) bool { + return value != "" && len(value) <= 256 && utf8.ValidString(value) && + value == strings.TrimSpace(value) && !strings.ContainsAny(value, "\x00\r\n,") && + !strings.ContainsFunc(value, func(r rune) bool { return unicode.IsControl(r) || r == utf8.RuneError }) +} + +// strictClaimValues accepts only a well-formed claim array. No scalar parsing, +// coercion, regex or prefix matching is introduced into the new gate. +func strictClaimValues(raw any) ([]string, bool) { + var values []string + switch v := raw.(type) { + case []string: + values = v + case []any: + values = make([]string, len(v)) + for i, value := range v { + s, ok := value.(string) + if !ok { + return nil, false + } + values[i] = s + } + default: + return nil, false + } + if len(values) > maxClaimValues { + return nil, false + } + for _, value := range values { + if !validClaimLiteral(value) { + return nil, false + } + } + return values, true +} + +// MatchRequiredClaimValues evaluates the new consumer gate against current +// authenticated provider data; it does not mutate or manufacture issuer claims. +func MatchRequiredClaimValues(claims map[string]any, name, legacy string, required []string) bool { + if required == nil || ValidateRequiredClaimValues(name, legacy, required) != nil { + return false + } + raw, present := claims[name] + if !present { + return false + } + values, ok := strictClaimValues(raw) + if !ok { + return false + } + for _, value := range values { + for _, wanted := range required { + if value == wanted { + return true + } + } + } + return false +} + +// RequireCurrentUserInfoClaim preserves the existing HTTP client settings and +// adds a strict response guard for this opt-in source only. A current UserInfo +// array must overwrite any stale ID-token claim. Missing/invalid data and HTTP +// failures cannot fall back to the ID-token map in Goth's merging code. +func RequireCurrentUserInfoClaim(client *http.Client, endpoint, claim string) (*http.Client, error) { + u, err := url.Parse(endpoint) + if err != nil || u.Scheme != "https" || u.Host == "" || u.User != nil || u.Fragment != "" || !validClaimLiteral(claim) { + return nil, errors.New("required-claim-values needs an HTTPS UserInfo endpoint") + } + if client == nil { + client = http.DefaultClient + } + result := *client + base := result.Transport + if base == nil { + base = http.DefaultTransport + } + result.Transport = ¤tClaimTransport{base: base, endpoint: endpoint, claim: claim} + // Redirects could evade endpoint matching or send a bearer to another origin. + // They are unsupported for the new guarded provider; legacy clients are intact. + result.CheckRedirect = func(*http.Request, []*http.Request) error { + return errors.New("guarded OAuth redirects are not supported") + } + return &result, nil +} + +type currentClaimTransport struct { + base http.RoundTripper + endpoint, claim string +} + +func (t *currentClaimTransport) RoundTrip(req *http.Request) (*http.Response, error) { + response, err := t.base.RoundTrip(req) + if req.URL.String() != t.endpoint { + return response, err + } + if err != nil { + if response != nil && response.Body != nil { + response.Body.Close() + } + return nil, errors.New("current UserInfo lookup failed") + } + if response == nil || response.Body == nil { + return nil, errors.New("current UserInfo response missing") + } + defer response.Body.Close() + if req.Method != http.MethodGet || response.StatusCode != http.StatusOK { + return nil, errors.New("current UserInfo refused") + } + body, err := io.ReadAll(io.LimitReader(response.Body, maxUserInfoBytes+1)) + if err != nil || len(body) > maxUserInfoBytes || !utf8.Valid(body) { + return nil, errors.New("current UserInfo response invalid") + } + // Reject duplicate top-level keys before Unmarshal can hide a conflicting + // groups or subject claim. json.RawMessage permits unrelated normal claims. + decoder := json.NewDecoder(bytes.NewReader(body)) + token, err := decoder.Token() + if err != nil || token != json.Delim('{') { + return nil, errors.New("current UserInfo object required") + } + claims := map[string]json.RawMessage{} + for decoder.More() { + keyToken, err := decoder.Token() + if err != nil { + return nil, errors.New("current UserInfo key invalid") + } + key, ok := keyToken.(string) + if !ok { + return nil, errors.New("current UserInfo key invalid") + } + if _, duplicate := claims[key]; duplicate { + return nil, errors.New("current UserInfo duplicate claim") + } + var value json.RawMessage + if decoder.Decode(&value) != nil { + return nil, errors.New("current UserInfo value invalid") + } + claims[key] = value + } + if token, err = decoder.Token(); err != nil || token != json.Delim('}') { + return nil, errors.New("current UserInfo object invalid") + } + var trailing any + if decoder.Decode(&trailing) != io.EOF { + return nil, errors.New("current UserInfo trailing data") + } + raw, present := claims[t.claim] + if !present { + return nil, errors.New("current UserInfo required claim missing") + } + var value any + if json.Unmarshal(raw, &value) != nil { + return nil, errors.New("current UserInfo required claim invalid") + } + if _, ok := strictClaimValues(value); !ok { + return nil, errors.New("current UserInfo required claim array invalid") + } + // Return the original public response bytes to Goth. Subject equality and + // audience/issuer checks remain in its existing authenticated provider path. + response.Body = io.NopCloser(bytes.NewReader(body)) + return response, nil +} --- /dev/null +++ b/services/auth/source/oauth2/required_claim_values_test.go @@ -0,0 +1,144 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package oauth2 + +import ( + "errors" + "io" + "net/http" + "reflect" + "strings" + "testing" +) + +var caseValues = []string{"forge-admins", "forge-coulomb-owners", "forge-coulomb-readers"} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func mockUserInfo(body string, status int) *http.Client { + return &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: status, Header: http.Header{}, Body: io.NopCloser(strings.NewReader(body)), Request: r}, nil + })} +} + +func TestRequiredClaimExactAnyOfAndNoMutation(t *testing.T) { + for _, group := range caseValues { + groups := []any{group, "unrelated"} + claims := map[string]any{"groups": groups, "other": "preserved"} + if !MatchRequiredClaimValues(claims, "groups", "", caseValues) { + t.Fatal("individual allowed group rejected") + } + if !reflect.DeepEqual(claims, map[string]any{"groups": groups, "other": "preserved"}) { + t.Fatal("claims mutated") + } + } + if !MatchRequiredClaimValues(map[string]any{"groups": append([]string(nil), caseValues...)}, "groups", "", caseValues) { + t.Fatal("multiple allowed groups rejected") + } + for _, raw := range []any{nil, true, 1, "forge-admins", "forge-admins,forge-coulomb-owners", []any{"forge-admins", 1}, []any{"forge-admins", nil}, []string{}, []any{}, []string{"forge-admins-lookalike"}, []string{" Forge-admins"}, []string{"forge-admins\n"}, []string{"Forge-admins"}, []string{"forge-admins,other"}, []string{"forge-admins", ""}} { + if MatchRequiredClaimValues(map[string]any{"groups": raw}, "groups", "", caseValues) { + t.Fatal("malformed/nonmatching claim accepted") + } + } + if MatchRequiredClaimValues(map[string]any{}, "groups", "", caseValues) { + t.Fatal("absent claim accepted") + } + if MatchRequiredClaimValues(map[string]any{"unrelated": []string{"forge-admins"}}, "groups", "", caseValues) { + t.Fatal("unrelated claim used") + } +} + +func TestRequiredClaimConfigurationRefusalsAndLegacyIsolation(t *testing.T) { + if ValidateRequiredClaimValues("", "old arbitrary literal", nil) != nil { + t.Fatal("legacy config changed") + } + for _, cfg := range []struct { + name, legacy string + values []string + }{ + {"groups", "", []string{}}, {"groups", "", []string{""}}, {"groups", "old", caseValues}, {"", "", caseValues}, + {"groups", "", []string{"a", "a"}}, {"groups", "", []string{"a,b"}}, {"groups", "", []string{" a"}}, + } { + if ValidateRequiredClaimValues(cfg.name, cfg.legacy, cfg.values) == nil { + t.Fatal("invalid config accepted") + } + } + if ValidateRequiredClaimValues("groups", "", caseValues) != nil { + t.Fatal("case config invalid") + } +} + +func TestCurrentUserInfoGuardRejectsMissingMalformedAndUnavailable(t *testing.T) { + for _, body := range []string{ + `{}`, `{"sub":"alice"}`, `{"groups":null}`, `{"groups":true}`, `{"groups":"forge-admins"}`, + `{"groups":["forge-admins",null]}`, `{"groups":["forge-admins",1]}`, + `{"groups":["forge-admins"],"groups":[]}`, `{"groups":[]} {}`, `[]`, `{"groups":[]`, + `{"groups":["forge-admins",""]}`, strings.Repeat(" ", maxUserInfoBytes+1), + } { + client, err := RequireCurrentUserInfoClaim(mockUserInfo(body, 200), "https://issuer.example/userinfo", "groups") + if err != nil { + t.Fatal(err) + } + response, err := client.Get("https://issuer.example/userinfo") + if err == nil { + response.Body.Close() + t.Fatal("invalid current claim did not fail closed") + } + } + client, _ := RequireCurrentUserInfoClaim(mockUserInfo(`{"groups":[]}`, 503), "https://issuer.example/userinfo", "groups") + if response, err := client.Get("https://issuer.example/userinfo"); err == nil { + response.Body.Close() + t.Fatal("non200 accepted") + } + broken := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { return nil, errors.New("private backend error") })} + client, _ = RequireCurrentUserInfoClaim(broken, "https://issuer.example/userinfo", "groups") + if _, err := client.Get("https://issuer.example/userinfo"); err == nil || strings.Contains(err.Error(), "private backend") { + t.Fatal("lookup failure not contained") + } +} + +func TestCurrentUserInfoEmptyArrayOverwritesAndGroupRemoval(t *testing.T) { + for _, body := range []string{`{"sub":"alice","groups":[]}`, `{"sub":"alice","groups":["forge-coulomb-readers"]}`} { + client, err := RequireCurrentUserInfoClaim(mockUserInfo(body, 200), "https://issuer.example/userinfo", "groups") + if err != nil { + t.Fatal(err) + } + response, err := client.Get("https://issuer.example/userinfo") + if err != nil { + t.Fatal(err) + } + b, _ := io.ReadAll(response.Body) + response.Body.Close() + if string(b) != body { + t.Fatal("response changed") + } + } +} + +func TestCurrentUserInfoGuardIsOptInAndRedirectsRefuse(t *testing.T) { + original := mockUserInfo(`{"unrelated":true}`, 200) + guarded, err := RequireCurrentUserInfoClaim(original, "https://issuer.example/userinfo", "groups") + if err != nil { + t.Fatal(err) + } + response, err := original.Get("https://issuer.example/userinfo") + if err != nil { + t.Fatal("original client mutated") + } + response.Body.Close() + response, err = guarded.Get("https://issuer.example/unrelated") + if err != nil { + t.Fatal("unrelated endpoint changed") + } + response.Body.Close() + if guarded.CheckRedirect == nil || guarded.CheckRedirect(nil, nil) == nil { + t.Fatal("redirect can bypass guard") + } + for _, endpoint := range []string{"", "http://issuer.example/userinfo", "https://user:pass@issuer.example/userinfo", "https://issuer.example/userinfo#fragment"} { + if _, err := RequireCurrentUserInfoClaim(original, endpoint, "groups"); err == nil { + t.Fatal("invalid endpoint accepted") + } + } +} --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -22,6 +22,7 @@ Scopes []string AttributeSSHPublicKey string RequiredClaimName string + RequiredClaimValues []string `json:",omitempty"` RequiredClaimValue string GroupClaimName string AdminGroup string