From fec9bd18dd8e1d4b144a64535c9f5d247937743c Mon Sep 17 00:00:00 2001 From: velor2012 <38395332+velor2012@users.noreply.github.com> Date: Thu, 8 Oct 2026 08:20:15 +0800 Subject: [PATCH] =?UTF-8?q?fix(oauth):=20=E6=94=AF=E6=8C=81=20Casdoor=20?= =?UTF-8?q?=E8=AE=A4=E8=AF=81=EF=BC=8C=E4=BF=AE=E5=A4=8D=E7=AC=AC=E4=B8=89?= =?UTF-8?q?=E6=96=B9=E5=9B=9E=E8=B0=83=E7=9A=84=E7=94=A8=E6=88=B7id?= =?UTF-8?q?=E4=B8=BAstring=E6=97=B6=E4=BC=9A=E5=AF=BC=E8=87=B4=E6=9C=8D?= =?UTF-8?q?=E5=8A=A1=E5=99=A8=E5=86=85=E9=83=A8=E9=94=99=E8=AF=AF=E7=9A=84?= =?UTF-8?q?=E9=97=AE=E9=A2=98=20(#46)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(oauth): support Casdoor string user IDs * test(oauth): verify legacy IDs and auth source isolation --------- Co-authored-by: Noru Wyrms --- docs/changelog/index.md | 1 + docs/guide/sso.md | 8 ++ internal/apps/oauth/oauth_test.go | 135 ++++++++++++++++++++++++++++-- internal/model/users.go | 17 +--- internal/model/users_test.go | 31 +++++++ internal/repository/user.go | 7 +- 6 files changed, 173 insertions(+), 26 deletions(-) create mode 100644 internal/model/users_test.go diff --git a/docs/changelog/index.md b/docs/changelog/index.md index b748bad1..f30a1399 100644 --- a/docs/changelog/index.md +++ b/docs/changelog/index.md @@ -13,6 +13,7 @@ sidebar: false ### 🛠 修复 - 默认信任 Cloudflare 官方 IPv4/IPv6 网段，并允许管理员追加或覆盖可信代理 CIDR；显式 `[]` 可关闭信任。客户端地址恢复后统一用于访问日志、WAF、`X-Real-IP` 和 `X-Forwarded-For` 追加项，旧版自定义 OpenResty 主模板也会自动补入相关指令。 +- 修复 Casdoor 等认证源返回字符串 `id` 时 OIDC 登录回调失败的问题；自动注册独立生成本地用户 ID，避免第三方数字 ID 与已有用户或其他认证源冲突，已有用户及绑定不受影响。 ## [v3.5.7] - 2026-10-07 diff --git a/docs/guide/sso.md b/docs/guide/sso.md index 1250580d..36ead7e6 100644 --- a/docs/guide/sso.md +++ b/docs/guide/sso.md @@ -60,6 +60,8 @@ https://openflare.example.com/login 如果希望只允许已有用户使用 SSO，可以关闭用户注册。未绑定的第三方账号会进入绑定已有账号流程。 +第三方身份通过「认证源 + `sub`」关联本地用户；自动注册时独立生成本地用户 ID，不使用第三方的 `id` 或数字 `sub` 作为本地用户主键。升级不会修改已有用户 ID 或账号绑定。 + ## 修改认证源 修改认证源时，Client Secret 输入框留空表示保留已有密钥；填写新值则会覆盖保存。 @@ -68,6 +70,12 @@ https://openflare.example.com/login ## 常见问题 +### Casdoor 授权后提示「服务器内部错误，请稍后重试」 + +旧版本将 ID Token 中非标准的 `id` 字段按整数解析，而 Casdoor 返回的 `id` 是字符串（通常为 UUID），可能导致授权成功后回调失败。请升级到包含此修复的版本，并从登录页重新发起授权，不要刷新或重复使用旧的回调链接。 + +如果升级后仍失败，请在浏览器开发者工具的 Network 中检查 `POST /api/v1/oauth/callback` 的状态码和响应 `error_msg`，以区分令牌兑换、ID Token 校验或数据库错误。排查时不要分享授权码、Token、Client Secret 或完整用户信息。 + ### 返回 `invalid_scope` 说明第三方平台不允许当前配置的 Scope。OIDC 默认 Scope 是 `openid profile email`。请到认证源编辑页调整 Scope，或在第三方平台放行对应 Scope。 diff --git a/internal/apps/oauth/oauth_test.go b/internal/apps/oauth/oauth_test.go index d36183ff..6fd23e63 100644 --- a/internal/apps/oauth/oauth_test.go +++ b/internal/apps/oauth/oauth_test.go @@ -262,7 +262,7 @@ func oidcDiscoveryResponse() *http.Response { } type mockClaims struct { - ID uint64 `json:"id"` + ID any `json:"id"` Issuer string `json:"iss"` Subject string `json:"sub"` Audience string `json:"aud"` @@ -275,12 +275,16 @@ type mockClaims struct { Active bool `json:"active"` } +// generateMockIDToken signs test claims with numeric IDs or Casdoor-style string IDs. func generateMockIDToken(issuer, sub, aud, nonce, username, email, name string) string { signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: testRSAPrivateKey}, (&jose.SignerOptions{}).WithType("JWT")) if err != nil { panic(err) } - id, _ := strconv.ParseUint(sub, 10, 64) + var id any = sub + if numericID, parseErr := strconv.ParseUint(sub, 10, 64); parseErr == nil { + id = numericID + } claims := mockClaims{ ID: id, Issuer: issuer, @@ -705,6 +709,7 @@ func TestLogout(t *testing.T) { } } +// TestCallbackLoginAndUserInfo covers string provider IDs, repeat logins, and local ID collisions. func TestCallbackLoginAndUserInfo(t *testing.T) { initializeTestConfig() dbConn := setupTestDB(t) @@ -719,7 +724,8 @@ func TestCallbackLoginAndUserInfo(t *testing.T) { var state string // 1. Mock the outgoing HTTP client for token exchange and user info fetching - httpMock := newMockOIDCClient(testIssuerURL, testClientID, &state, "88888", "test_oauth_user", "oauth@linux.do", "Oauth Test User") + const externalSubject = "a27dfc56-07ae-4c9d-9e5c-99103bd8805f" + httpMock := newMockOIDCClient(testIssuerURL, testClientID, &state, externalSubject, "test_oauth_user", "oauth@linux.do", "Oauth Test User") router := setupTestRouter(dbConn, mockRedis, httpMock) // Get Login URL first to initialize the session and generate the state @@ -766,15 +772,25 @@ func TestCallbackLoginAndUserInfo(t *testing.T) { t.Errorf("expected logged_in status, got %s", callbackResp.Data.Status) } - if callbackResp.Data.User.Username != "test_oauth_user" || callbackResp.Data.User.ID != 88888 { + if callbackResp.Data.User == nil { + t.Fatal("Callback(Casdoor string ID) returned no user, want logged-in user") + } + if callbackResp.Data.User.Username != "test_oauth_user" || callbackResp.Data.User.ID == 0 { t.Errorf("unexpected user returned: %+v", callbackResp.Data.User) } // Verify user is created in database var user model.User - if err := dbConn.First(&user, "id = ?", 88888).Error; err != nil { + if err := dbConn.First(&user, "id = ?", callbackResp.Data.User.ID).Error; err != nil { t.Fatalf("user was not created in DB: %v", err) } + account, err := repository.FindExternalAccount(context.Background(), 100, externalSubject) + if err != nil { + t.Fatalf("FindExternalAccount(%q) error = %v, want nil", externalSubject, err) + } + if account.UserID != user.ID { + t.Errorf("FindExternalAccount(%q).UserID = %d, want %d", externalSubject, account.UserID, user.ID) + } // Extract session cookie cookies := w.Result().Cookies() @@ -795,8 +811,40 @@ func TestCallbackLoginAndUserInfo(t *testing.T) { t.Fatalf("failed to fetch user info, status %d", w2.Code) } + // The same external subject must reuse its binding on subsequent logins. + wRepeatLogin := performRequest(router, http.MethodGet, "/api/v1/oauth/login?source="+testSourceName, nil, nil, []*http.Cookie{sessionCookie}) + if wRepeatLogin.Code != http.StatusOK { + t.Fatalf("GetLoginURL(existing user) status = %d, want 200", wRepeatLogin.Code) + } + if err := json.Unmarshal(wRepeatLogin.Body.Bytes(), &loginUrlResp); err != nil { + t.Fatal(err) + } + parsedURL, err = url.Parse(loginUrlResp.Data.AuthorizeURL) + if err != nil { + t.Fatal(err) + } + state = parsedURL.Query().Get("state") + wRepeat := performRequest(router, http.MethodPost, "/api/v1/oauth/callback", []byte(fmt.Sprintf(`{"state":"%s","code":"repeat_auth_code"}`, state)), map[string]string{ + "Content-Type": "application/json", + }, wRepeatLogin.Result().Cookies()) + if wRepeat.Code != http.StatusOK { + t.Fatalf("Callback(existing Casdoor user) status = %d, want 200; body: %s", wRepeat.Code, wRepeat.Body.String()) + } + var repeatResp struct { + Data OAuthCallbackResult `json:"data"` + } + if err := json.Unmarshal(wRepeat.Body.Bytes(), &repeatResp); err != nil { + t.Fatal(err) + } + if repeatResp.Data.User == nil || repeatResp.Data.User.ID != user.ID { + t.Errorf("Callback(existing Casdoor user) user = %+v, want ID %d", repeatResp.Data.User, user.ID) + } + // 5. Test Callback (Login flow - existing user, username collision check) var state2 string + if err := dbConn.Create(&model.User{ID: 99999, Username: "local_user", IsActive: true}).Error; err != nil { + t.Fatalf("failed to create local user with overlapping external ID: %v", err) + } // Callback with same username but different external ID (99999) httpMock2 := newMockOIDCClient(testIssuerURL, testClientID, &state2, "99999", "test_oauth_user", "another@linux.do", "Another User") @@ -833,6 +881,9 @@ func TestCallbackLoginAndUserInfo(t *testing.T) { Data OAuthCallbackResult `json:"data"` } _ = json.Unmarshal(w3.Body.Bytes(), &collisionResp) + if collisionResp.Data.User == nil || collisionResp.Data.User.ID == 0 || collisionResp.Data.User.ID == 99999 { + t.Fatalf("Callback(numeric provider ID) user = %+v, want independent local ID", collisionResp.Data.User) + } if collisionResp.Data.User.Username != "test_oauth_user-1" { t.Errorf("expected collision renamed username, got %s", collisionResp.Data.User.Username) @@ -891,6 +942,80 @@ func TestCallbackLoginAndUserInfo(t *testing.T) { }) } +// TestCallbackIdentityIsScopedToAuthSource verifies legacy IDs survive login and +// identical subjects from different authentication sources remain separate users. +func TestCallbackIdentityIsScopedToAuthSource(t *testing.T) { + initializeTestConfig() + dbConn := setupTestDB(t) + mockRedis := newMockRedisClient() + seedTestAuthSource(t, dbConn) + require.NoError(t, dbConn.Create(&model.SystemConfig{ + Key: model.ConfigKeyRegistrationEnabled, Value: "true", Type: "system", + }).Error) + require.NoError(t, dbConn.Create(&model.User{ + ID: 88888, Username: "legacy_user", IsActive: true, + }).Error) + require.NoError(t, dbConn.Create(&model.ExternalAccount{ + ID: 1, AuthSourceID: 100, UserID: 88888, ExternalID: "88888", + }).Error) + require.NoError(t, dbConn.Create(&model.AuthSource{ + ID: 101, Name: "second-source", Type: model.AuthSourceTypeOIDC, + IsActive: true, ClientID: testClientID, ClientSecret: testClientSecret, + OpenIDDiscoveryURL: testIssuerURL, + }).Error) + + for _, tc := range []struct { + name string + sourceID uint64 + legacy bool + }{ + {name: testSourceName, sourceID: 100, legacy: true}, + {name: "second-source", sourceID: 101}, + } { + t.Run(tc.name, func(t *testing.T) { + var state string + httpMock := newMockOIDCClient(testIssuerURL, testClientID, &state, "88888", "provider_user", "provider@example.com", "Provider User") + router := setupTestRouter(dbConn, mockRedis, httpMock) + login := performRequest(router, http.MethodGet, "/api/v1/oauth/login?source="+tc.name, nil, nil, nil) + require.Equal(t, http.StatusOK, login.Code, login.Body.String()) + var loginResp struct { + Data OAuthAuthorizeResponse `json:"data"` + } + require.NoError(t, json.Unmarshal(login.Body.Bytes(), &loginResp)) + authorizeURL, err := url.Parse(loginResp.Data.AuthorizeURL) + require.NoError(t, err) + state = authorizeURL.Query().Get("state") + require.NotEmpty(t, state) + + callback := performRequest(router, http.MethodPost, "/api/v1/oauth/callback", + []byte(fmt.Sprintf(`{"state":"%s","code":"test_code"}`, state)), + map[string]string{"Content-Type": "application/json"}, login.Result().Cookies()) + require.Equal(t, http.StatusOK, callback.Code, callback.Body.String()) + var callbackResp struct { + Data OAuthCallbackResult `json:"data"` + } + require.NoError(t, json.Unmarshal(callback.Body.Bytes(), &callbackResp)) + require.Equal(t, "logged_in", callbackResp.Data.Status) + require.NotNil(t, callbackResp.Data.User) + if tc.legacy { + assert.Equal(t, uint64(88888), callbackResp.Data.User.ID) + assert.Equal(t, "legacy_user", callbackResp.Data.User.Username) + } else { + assert.NotZero(t, callbackResp.Data.User.ID) + assert.NotEqual(t, uint64(88888), callbackResp.Data.User.ID) + } + account, err := repository.FindExternalAccount(context.Background(), tc.sourceID, "88888") + require.NoError(t, err) + assert.Equal(t, callbackResp.Data.User.ID, account.UserID) + }) + } + var users, accounts int64 + require.NoError(t, dbConn.Model(&model.User{}).Count(&users).Error) + require.NoError(t, dbConn.Model(&model.ExternalAccount{}).Count(&accounts).Error) + assert.Equal(t, int64(2), users) + assert.Equal(t, int64(2), accounts) +} + func TestCallbackBind(t *testing.T) { initializeTestConfig() dbConn := setupTestDB(t) diff --git a/internal/model/users.go b/internal/model/users.go index 9c61bf2e..dce9b405 100644 --- a/internal/model/users.go +++ b/internal/model/users.go @@ -6,7 +6,6 @@ package model import ( "errors" - "strconv" "strings" "time" @@ -15,8 +14,8 @@ import ( ) // OAuthUserInfo 用户信息结构（同时支持 OIDC ID Token claims 和 UserEndpoint 响应） +// 第三方身份使用 Sub；忽略非标准 id 字段，本地用户 ID 独立生成。 type OAuthUserInfo struct { - ID uint64 `json:"id"` Sub string `json:"sub"` Username string `json:"username"` PreferredUsername string `json:"preferred_username"` @@ -26,20 +25,6 @@ type OAuthUserInfo struct { AvatarURL string `json:"avatar_url"` } -// GetID 获取用户 ID -func (u *OAuthUserInfo) GetID() uint64 { - if u.ID != 0 { - return u.ID - } - // 从 sub 解析（OIDC 格式） - if u.Sub != "" { - if id, err := strconv.ParseUint(u.Sub, 10, 64); err == nil { - return id - } - } - return 0 -} - // User 用户表实体 type User struct { ID uint64 `json:"id,string" gorm:"primaryKey;not null"` diff --git a/internal/model/users_test.go b/internal/model/users_test.go new file mode 100644 index 00000000..35265084 --- /dev/null +++ b/internal/model/users_test.go @@ -0,0 +1,31 @@ +// Copyright 2026 Arctel.net +// SPDX-License-Identifier: Apache-2.0 + +package model + +import ( + "encoding/json" + "testing" +) + +// TestOAuthUserInfoIgnoresProviderID verifies nonstandard IDs cannot affect OIDC identity parsing. +func TestOAuthUserInfoIgnoresProviderID(t *testing.T) { + for _, providerID := range []string{`"a27dfc56-07ae-4c9d-9e5c-99103bd8805f"`, `"88888"`, `88888`, `null`} { + t.Run(providerID, func(t *testing.T) { + payload := `{"id":` + providerID + `,"sub":"external-subject","preferred_username":"oidc-user","email":"oidc@example.com","name":"OIDC User"}` + var info OAuthUserInfo + if err := json.Unmarshal([]byte(payload), &info); err != nil { + t.Fatalf("json.Unmarshal(OAuthUserInfo, id=%s) error = %v, want nil", providerID, err) + } + want := OAuthUserInfo{ + Sub: "external-subject", + PreferredUsername: "oidc-user", + Email: "oidc@example.com", + Name: "OIDC User", + } + if info != want { + t.Errorf("json.Unmarshal(OAuthUserInfo, id=%s) = %+v, want %+v", providerID, info, want) + } + }) + } +} diff --git a/internal/repository/user.go b/internal/repository/user.go index 6569acab..ddd840d6 100644 --- a/internal/repository/user.go +++ b/internal/repository/user.go @@ -199,11 +199,11 @@ func UpdateUser(ctx context.Context, user *model.User) error { } // CreateUserFromOAuth creates a user from OAuth profile data and fills userOut. +// The local ID is generated independently of the provider's identity claims. func CreateUserFromOAuth(ctx context.Context, userOut *model.User, oauthInfo *model.OAuthUserInfo) error { now := time.Now() - userID := oauthInfo.GetID() newUser := model.User{ - ID: userID, + ID: idgen.NextUint64ID(), Username: oauthInfo.Username, Nickname: oauthInfo.Name, Email: oauthInfo.Email, @@ -212,9 +212,6 @@ func CreateUserFromOAuth(ctx context.Context, userOut *model.User, oauthInfo *mo LastLoginAt: now, IsAdmin: false, } - if newUser.ID == 0 { - newUser.ID = idgen.NextUint64ID() - } if err := db.DB(ctx).Create(&newUser).Error; err != nil { return err }