Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions controller/custom_oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type CustomOAuthProviderResponse struct {
UsernameField string `json:"username_field"`
DisplayNameField string `json:"display_name_field"`
EmailField string `json:"email_field"`
GroupField string `json:"group_field"`
WellKnown string `json:"well_known"`
AuthStyle int `json:"auth_style"`
AccessPolicy string `json:"access_policy"`
Expand Down Expand Up @@ -62,6 +63,7 @@ func toCustomOAuthProviderResponse(p *model.CustomOAuthProvider) *CustomOAuthPro
UsernameField: p.UsernameField,
DisplayNameField: p.DisplayNameField,
EmailField: p.EmailField,
GroupField: p.GroupField,
WellKnown: p.WellKnown,
AuthStyle: p.AuthStyle,
AccessPolicy: p.AccessPolicy,
Expand Down Expand Up @@ -127,6 +129,7 @@ type CreateCustomOAuthProviderRequest struct {
UsernameField string `json:"username_field"`
DisplayNameField string `json:"display_name_field"`
EmailField string `json:"email_field"`
GroupField string `json:"group_field"`
WellKnown string `json:"well_known"`
AuthStyle int `json:"auth_style"`
AccessPolicy string `json:"access_policy"`
Expand Down Expand Up @@ -245,6 +248,7 @@ func CreateCustomOAuthProvider(c *gin.Context) {
UsernameField: req.UsernameField,
DisplayNameField: req.DisplayNameField,
EmailField: req.EmailField,
GroupField: req.GroupField,
WellKnown: req.WellKnown,
AuthStyle: req.AuthStyle,
AccessPolicy: req.AccessPolicy,
Expand Down Expand Up @@ -282,6 +286,7 @@ type UpdateCustomOAuthProviderRequest struct {
UsernameField string `json:"username_field"`
DisplayNameField string `json:"display_name_field"`
EmailField string `json:"email_field"`
GroupField string `json:"group_field"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

GroupField cannot be cleared once set due to non-pointer type.

Unlike other optional fields (WellKnown, AccessPolicy, etc.) which use *string to distinguish "not provided" from "set to empty", GroupField is a plain string. The condition if req.GroupField != "" prevents clearing an existing value.

🔧 Proposed fix to allow clearing GroupField
 type UpdateCustomOAuthProviderRequest struct {
 	...
-	GroupField            string  `json:"group_field"`
+	GroupField            *string `json:"group_field"`
 	...
 }

And update the handler:

-	if req.GroupField != "" {
-		provider.GroupField = req.GroupField
+	if req.GroupField != nil {
+		provider.GroupField = *req.GroupField
 	}

Also applies to: 376-378

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controller/custom_oauth.go` at line 289, Change the GroupField field from
string to *string in the request/struct where GroupField is defined (to match
WellKnown, AccessPolicy patterns) so you can distinguish "not provided" vs "set
to empty"; then update any handlers and assignment logic that reference
GroupField (e.g., where you currently check if req.GroupField != "" and where
you copy to the model) to use nil checks (if req.GroupField != nil) and allow
setting the target to *req.GroupField (including empty string) to support
clearing the existing value — also update the corresponding JSON unmarshalling
usage and any code paths around GroupField at the other occurrence noted so they
use the pointer semantics.

WellKnown *string `json:"well_known"` // Optional: if nil, keep existing
AuthStyle *int `json:"auth_style"` // Optional: if nil, keep existing
AccessPolicy *string `json:"access_policy"` // Optional: if nil, keep existing
Expand Down Expand Up @@ -368,6 +373,9 @@ func UpdateCustomOAuthProvider(c *gin.Context) {
if req.EmailField != "" {
provider.EmailField = req.EmailField
}
if req.GroupField != "" {
provider.GroupField = req.GroupField
}
if req.WellKnown != nil {
provider.WellKnown = *req.WellKnown
}
Expand Down
35 changes: 35 additions & 0 deletions controller/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/QuantumNous/new-api/i18n"
"github.com/QuantumNous/new-api/model"
"github.com/QuantumNous/new-api/oauth"
"github.com/QuantumNous/new-api/setting/ratio_setting"
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"gorm.io/gorm"
Expand Down Expand Up @@ -209,6 +210,29 @@ func findOrCreateOAuthUser(c *gin.Context, provider oauth.Provider, oauthUser *o
if user.Id == 0 {
return nil, &OAuthUserDeletedError{}
}

// Update user's group if OAuth provides a different group and it's in available groups
if oauthUser.Group != "" && oauthUser.Group != user.Group {
common.SysLog(fmt.Sprintf("[OAuth] User %d current group: '%s', OAuth group: '%s'", user.Id, user.Group, oauthUser.Group))
// Check if group exists in group ratio settings
if ratio_setting.ContainsGroupRatio(oauthUser.Group) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how groups are typically validated elsewhere in the codebase
rg -n --type go -C3 'ContainsGroupRatio|groupRatioMap' --glob '!*_test.go'

Repository: QuantumNous/new-api

Length of output: 4535


🏁 Script executed:

#!/bin/bash
# Find group configuration files and what groups are defined
find . -type f -name "*.go" | xargs grep -l "defaultGroup" | head -10

Repository: QuantumNous/new-api

Length of output: 101


🏁 Script executed:

#!/bin/bash
# Check what groups are defined in the ratio setting
rg -n "defaultGroupRatio|defaultGroupSpecialUsableGroup" --type go -A 10 | head -80

Repository: QuantumNous/new-api

Length of output: 3317


🏁 Script executed:

#!/bin/bash
# Check if there's any OAuth-specific group configuration
rg -n "oauth.*group|OAuth.*[Gg]roup" --type go | grep -i config

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Look for other group validation patterns in the codebase
rg -n "user\.Group\s*=|Group.*=" --type go controller/ | head -20

Repository: QuantumNous/new-api

Length of output: 1408


🏁 Script executed:

#!/bin/bash
# Check the oauth.go file context around lines 215-295 to understand the group assignment flow
sed -n '210,300p' controller/oauth.go

Repository: QuantumNous/new-api

Length of output: 3629


🏁 Script executed:

#!/bin/bash
# Check if there's OAuth provider configuration for allowed groups
rg -n "oauth.*config|oauthUser|GetUserInfoFromOAuth" --type go -B 2 -A 5 | head -100

Repository: QuantumNous/new-api

Length of output: 6025


🏁 Script executed:

#!/bin/bash
# Look for OAuthUser struct definition to understand what Group field represents
rg -n "type.*OAuthUser|oauthUser.*Group" --type go -B 2 -A 8 | head -80

Repository: QuantumNous/new-api

Length of output: 4314


🏁 Script executed:

#!/bin/bash
# Check the full OAuthUser struct definition to see if there's any group-related field
rg -n "type OAuthUser" --type go -A 15 oauth/

Repository: QuantumNous/new-api

Length of output: 944


🏁 Script executed:

#!/bin/bash
# Look for where OAuthUser.Group is populated from OAuth provider
rg -n "\.Group\s*=" --type go oauth/ | head -20

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Check if there's any OAuth provider configuration related to allowed groups
rg -n "config.*[Gg]roup|allowedGroup|groupMapping" --type go -i | head -30

Repository: QuantumNous/new-api

Length of output: 537


🏁 Script executed:

#!/bin/bash
# Check the GenericOAuthProvider config to understand GroupField better
rg -n "type.*Provider|GroupField" --type go oauth/ -B 2 -A 2 | head -50

Repository: QuantumNous/new-api

Length of output: 1828


🏁 Script executed:

#!/bin/bash
# Look for GenericOAuthProvider struct and its config
rg -n "type GenericOAuthProvider|type.*Config" --type go oauth/generic.go -A 20 | head -80

Repository: QuantumNous/new-api

Length of output: 582


🏁 Script executed:

#!/bin/bash
# Check what GroupField is used for in the generic provider
sed -n '240,280p' oauth/generic.go

Repository: QuantumNous/new-api

Length of output: 1670


Replace ratio_setting.ContainsGroupRatio() with dedicated OAuth group validation.

Using ContainsGroupRatio() to validate OAuth-provided groups is semantically incorrect—it checks pricing tier configuration rather than authorization policy. This couples billing configuration to user assignment logic, creating maintenance risks:

  • Adding a pricing tier implicitly makes it OAuth-assignable
  • Removing a tier silently breaks existing OAuth group assignments
  • The validation doesn't reflect its actual purpose

Introduce a dedicated "OAuth allowed groups" configuration (separate from group_ratio_setting) to explicitly control which groups can be auto-assigned via OAuth, independent of pricing tiers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controller/oauth.go` at line 218, The code incorrectly uses
ratio_setting.ContainsGroupRatio(oauthUser.Group) to decide OAuth group
assignment; replace this with a new dedicated OAuth group validation that reads
from a separate configuration (e.g., oauth_allowed_groups or
OAuthGroupAllowlist) rather than group_ratio_setting. Add a function like
OAuthGroupAllowed(group string) bool (or a method on a new OAuthConfig struct)
and call that where oauthUser.Group is validated, ensure the new config is
loaded/validated at startup, and remove dependence on
ratio_setting.ContainsGroupRatio so pricing tiers do not control OAuth
assignment.

user.Group = oauthUser.Group
if err := user.Update(false); err != nil {
common.SysError(fmt.Sprintf("[OAuth] Failed to update user %d group to '%s': %s", user.Id, oauthUser.Group, err.Error()))
} else {
common.SysLog(fmt.Sprintf("[OAuth] Updated user %d group to '%s' from OAuth provider", user.Id, oauthUser.Group))
}
} else {
common.SysLog(fmt.Sprintf("[OAuth] OAuth group '%s' not in group ratio settings for user %d, keeping current group '%s'", oauthUser.Group, user.Id, user.Group))
}
} else {
if oauthUser.Group == "" {
common.SysLog(fmt.Sprintf("[OAuth] User %d OAuth group is empty, skipping group update", user.Id))
} else if oauthUser.Group == user.Group {
common.SysLog(fmt.Sprintf("[OAuth] User %d group '%s' already matches OAuth group, no update needed", user.Id, user.Group))
}
}
Comment on lines +213 to +234

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Race condition: user.Update(false) can overwrite concurrent quota changes.

The Update(false) method (see model/user.go:494-510) performs a full-row update using the entire user struct. Between FillUserByProviderID (line 205) and Update (line 220), concurrent requests may have modified UsedQuota or RequestCount. The update will blindly overwrite those changes.

Consider using a targeted column update instead:

🔧 Proposed fix using targeted update
 		if ratio_setting.ContainsGroupRatio(oauthUser.Group) {
-			user.Group = oauthUser.Group
-			if err := user.Update(false); err != nil {
+			if err := model.DB.Model(user).Update("group", oauthUser.Group).Error; err != nil {
 				common.SysError(fmt.Sprintf("[OAuth] Failed to update user %d group to '%s': %s", user.Id, oauthUser.Group, err.Error()))
 			} else {
+				user.Group = oauthUser.Group // Update local copy after successful DB write
 				common.SysLog(fmt.Sprintf("[OAuth] Updated user %d group to '%s' from OAuth provider", user.Id, oauthUser.Group))
 			}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controller/oauth.go` around lines 213 - 234, The current flow uses
FillUserByProviderID then calls user.Update(false), which does a full-row
overwrite and can clobber concurrent changes to UsedQuota and RequestCount;
instead change the update to a targeted column update that only modifies the
Group field (or add a new method like UpdateGroup / UpdateColumns that issues an
UPDATE users SET group=... WHERE id=...), and invoke that from the OAuth path
where you currently call user.Update(false) (keep the
ratio_setting.ContainsGroupRatio check and logging, but replace the full-row
Update with a column-specific update to avoid overwriting
UsedQuota/RequestCount).


return user, nil
}

Expand Down Expand Up @@ -262,6 +286,17 @@ func findOrCreateOAuthUser(c *gin.Context, provider oauth.Provider, oauthUser *o
user.Role = common.RoleCommonUser
user.Status = common.UserStatusEnabled

// Auto-assign group from OAuth provider if configured
if oauthUser.Group != "" {
// Check if the group from OAuth is in the platform's group ratio settings
if ratio_setting.ContainsGroupRatio(oauthUser.Group) {
user.Group = oauthUser.Group
common.SysLog(fmt.Sprintf("[OAuth] Auto-assigned group '%s' to new user from OAuth provider (matched group ratio settings)", oauthUser.Group))
} else {
common.SysLog(fmt.Sprintf("[OAuth] Group '%s' from OAuth provider not found in group ratio settings, using default 'default'", oauthUser.Group))
}
}

// Handle affiliate code
affCode := session.Get("aff")
inviterId := 0
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ require (
github.com/icza/bitio v1.1.0 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
github.com/jackc/pgx/v5 v5.7.1 // indirect
github.com/jackc/pgx/v5 v5.9.1 // indirect
github.com/jackc/puddle/v2 v2.2.2 // indirect
github.com/jfreymuth/vorbis v1.0.2 // indirect
github.com/jinzhu/inflection v1.0.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI
github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg=
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo=
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
github.com/jackc/pgx/v5 v5.7.1 h1:x7SYsPBYDkHDksogeSmZZ5xzThcTgRz++I5E+ePFUcs=
github.com/jackc/pgx/v5 v5.7.1/go.mod h1:e7O26IywZZ+naJtWWos6i6fvWK+29etgITqrqHLfoZA=
github.com/jackc/pgx/v5 v5.9.1 h1:uwrxJXBnx76nyISkhr33kQLlUqjv7et7b9FjCen/tdc=
github.com/jackc/pgx/v5 v5.9.1/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4=
github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo=
github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
github.com/jfreymuth/oggvorbis v1.0.5 h1:u+Ck+R0eLSRhgq8WTmffYnrVtSztJcYrl588DM4e3kQ=
Expand Down
1 change: 1 addition & 0 deletions model/custom_oauth_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type CustomOAuthProvider struct {
UsernameField string `json:"username_field" gorm:"type:varchar(128);default:'preferred_username'"` // Username field path
DisplayNameField string `json:"display_name_field" gorm:"type:varchar(128);default:'name'"` // Display name field path
EmailField string `json:"email_field" gorm:"type:varchar(128);default:'email'"` // Email field path
GroupField string `json:"group_field" gorm:"type:varchar(128);default:''"` // Group field path for auto-assigning user group, e.g., "groups", "roles", "data.group"

// Advanced options
WellKnown string `json:"well_known" gorm:"type:varchar(512)"` // OIDC discovery endpoint (optional)
Expand Down
15 changes: 13 additions & 2 deletions oauth/generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,16 @@ func (p *GenericOAuthProvider) GetUserInfo(ctx context.Context, token *OAuthToke
username := gjson.Get(bodyStr, p.config.UsernameField).String()
displayName := gjson.Get(bodyStr, p.config.DisplayNameField).String()
email := gjson.Get(bodyStr, p.config.EmailField).String()
group := ""
if p.config.GroupField != "" {
groupResult := gjson.Get(bodyStr, p.config.GroupField)
// If result is an array, take the first element
if groupResult.IsArray() && len(groupResult.Array()) > 0 {
group = groupResult.Array()[0].String()
} else {
group = groupResult.String()
}
}

// If user ID field returns a number, convert it
if userId == "" {
Expand All @@ -260,8 +270,8 @@ func (p *GenericOAuthProvider) GetUserInfo(ctx context.Context, token *OAuthToke
return nil, NewOAuthError(i18n.MsgOAuthUserInfoEmpty, map[string]any{"Provider": p.config.Name})
}

logger.LogDebug(ctx, "[OAuth-Generic-%s] GetUserInfo success: id=%s, username=%s, name=%s, email=%s",
p.config.Slug, userId, username, displayName, email)
common.SysLog(fmt.Sprintf("[OAuth-Generic-%s] GetUserInfo success: id=%s, username=%s, name=%s, email=%s, group=%s",
p.config.Slug, userId, username, displayName, email, group))

policyRaw := strings.TrimSpace(p.config.AccessPolicy)
if policyRaw != "" {
Expand All @@ -284,6 +294,7 @@ func (p *GenericOAuthProvider) GetUserInfo(ctx context.Context, token *OAuthToke
Username: username,
DisplayName: displayName,
Email: email,
Group: group,
Extra: map[string]any{
"provider": p.config.Slug,
},
Expand Down
2 changes: 2 additions & 0 deletions oauth/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ type OAuthUser struct {
DisplayName string
// Email is the email from the OAuth provider
Email string
// Group is the group/role from the OAuth provider (optional, used for auto-assigning user group)
Group string
// Extra contains any additional provider-specific data
Extra map[string]any
}
Expand Down
11 changes: 11 additions & 0 deletions web/src/components/settings/CustomOAuthSetting.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,17 @@ const CustomOAuthSetting = ({ serverAddress }) => {
</Col>
</Row>

<Row gutter={16}>
<Col span={12}>
<Form.Input
field="group_field"
label={t('用户组字段(可选)')}
placeholder={t('例如:groups、roles、data.group')}
extraText={t('用于从用户信息中提取组名称并自动设置用户分组,支持 JSONPath 语法;如果返回数组则取第一个值')}
/>
</Col>
</Row>

<Collapse
keepDOM
activeKey={advancedActiveKeys}
Expand Down