clone/vikunja
1
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-02-01 22:47:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(auth): add ForceUserInfo option to OpenID provider (#797)

Browse Source 
Problem:

When using Casdoor as an OpenID provider, there's an inconsistency between the user information in the JWT token and the UserInfo endpoint. The token contains the user's unique ID in the `name` field, while the UserInfo endpoint correctly returns the user's display name.

Solution:

This PR adds a new `ForceUserInfo` option to the OpenID provider configuration. When enabled, it forces the use of the UserInfo endpoint to retrieve user information instead of relying on claims from the ID token.

Impact:

- Default behavior remains unchanged (backward compatible)
- New option allows administrators to force using UserInfo endpoint data
- Particularly useful for providers like Casdoor that don't fully comply with OIDC standards

Related:

I've opened an issue in the Casdoor repository (https://github.com/casdoor/casdoor/issues/3806) to discuss the root cause. However, changing Casdoor's token structure might cause significant compatibility issues for existing integrations, so it's unclear if this can be fixed at the provider level. This PR provides a workaround in Vikunja that doesn't affect existing functionality.
This commit is contained in:
Weijie Zhao
2025-05-20 16:06:34 +08:00
committed by GitHub
parent 80b1f6cab3
commit 00c4148f05
4 changed files with 177 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
config-raw.json
View File

@@ -709,6 +709,11 @@
"key": "emailfallback",
"default_value": "false",
"comment": "This option allows to look for a local account where the OIDC user's email match the Vikunja local email. Allowed value is either `true` or `false`. That option can be combined with `usernamefallback`.\nUse with caution, this can allow the 3rd party provider to connect to *any* local account and therefore potential account hijaking."
},
{
"key": "forceuserinfo",
"default_value": "false",
"comment": "This option forces the use of the OpenID Connect UserInfo endpoint to retrieve user information instead of relying on claims from the ID token. When set to `true`, user data (email, name, username) will always be obtained from the UserInfo endpoint even if the information is available in the token claims. This is useful for providers that don't include complete user information in their tokens or when you need the most up-to-date user data. Allowed value is either `true` or `false`."
}
]
}

53
pkg/modules/auth/openid/openid.go
View File

@@ -56,6 +56,7 @@ type Provider struct {
Scope string `json:"scope"`
EmailFallback bool `json:"email_fallback"`
UsernameFallback bool `json:"username_fallback"`
ForceUserInfo bool `json:"force_user_info"`
ClientSecret string `json:"-"`
openIDProvider *oidc.Provider
Oauth2Config *oauth2.Config `json:"-"`
@@ -288,6 +289,32 @@ func getOrCreateUser(s *xorm.Session, cl *claims, provider *Provider, idToken *o
return
}
// mergeClaims combines claims from token and userinfo based on the ForceUserInfo setting
// cl represents the claims from the token, cl2 represents the claims from userinfo
func mergeClaims(cl *claims, cl2 *claims, forceUserInfo bool) error {
if (forceUserInfo && cl2.Email != "") || cl.Email == "" {
cl.Email = cl2.Email
}
if (forceUserInfo && cl2.Name != "") || cl.Name == "" {
cl.Name = cl2.Name
}
if (forceUserInfo && cl2.PreferredUsername != "") || cl.PreferredUsername == "" {
cl.PreferredUsername = cl2.PreferredUsername
}
if cl.PreferredUsername == "" && cl2.Nickname != "" {
cl.PreferredUsername = cl2.Nickname
}
if cl.Email == "" {
return &user.ErrNoOpenIDEmailProvided{}
}
return nil
}
func getClaims(provider *Provider, oauth2Token *oauth2.Token, idToken *oidc.IDToken) (*claims, error) {
cl := &claims{}
@@ -297,7 +324,7 @@ func getClaims(provider *Provider, oauth2Token *oauth2.Token, idToken *oidc.IDTo
return nil, err
}
if cl.Email == "" || cl.Name == "" || cl.PreferredUsername == "" {
if provider.ForceUserInfo || cl.Email == "" || cl.Name == "" || cl.PreferredUsername == "" {
info, err := provider.openIDProvider.UserInfo(context.Background(), provider.Oauth2Config.TokenSource(context.Background(), oauth2Token))
if err != nil {
log.Errorf("Error getting userinfo for provider %s: %v", provider.Name, err)
@@ -311,25 +338,13 @@ func getClaims(provider *Provider, oauth2Token *oauth2.Token, idToken *oidc.IDTo
return nil, err
}
if cl.Email == "" {
cl.Email = cl2.Email
}
err = mergeClaims(cl, cl2, provider.ForceUserInfo)
if err != nil {
if user.IsErrNoEmailProvided(err) {
log.Errorf("Claim does not contain an email address for provider %s", provider.Name)
}
if cl.Name == "" {
cl.Name = cl2.Name
}
if cl.PreferredUsername == "" {
cl.PreferredUsername = cl2.PreferredUsername
}
if cl.PreferredUsername == "" && cl2.Nickname != "" {
cl.PreferredUsername = cl2.Nickname
}
if cl.Email == "" {
log.Errorf("Claim does not contain an email address for provider %s", provider.Name)
return nil, &user.ErrNoOpenIDEmailProvided{}
return nil, err
}
}
return cl, nil

124
pkg/modules/auth/openid/openid_test.go
View File

@@ -292,3 +292,127 @@ func TestGetOrCreateUser(t *testing.T) {
assert.Equal(t, 11, int(u.ID), "user id 11 expected")
})
}
// TestMergeClaims tests the mergeClaims function with different configurations including forceUserInfo
func TestMergeClaims(t *testing.T) {
t.Run("ForceUserInfo enabled - should use userinfo values", func(t *testing.T) {
// Setup token claims
tokenClaims := &claims{
Email: "token-email@example.com",
Name: "Token Name",
PreferredUsername: "token_username",
}
// Setup userinfo claims
userinfoClaims := &claims{
Email: "userinfo-email@example.com",
Name: "UserInfo Name",
PreferredUsername: "userinfo_username",
}
// Test with ForceUserInfo enabled
err := mergeClaims(tokenClaims, userinfoClaims, true)
require.NoError(t, err)
// Verify userinfo data was used
assert.Equal(t, "userinfo-email@example.com", tokenClaims.Email)
assert.Equal(t, "UserInfo Name", tokenClaims.Name)
assert.Equal(t, "userinfo_username", tokenClaims.PreferredUsername)
})
t.Run("ForceUserInfo disabled - should use token values if present", func(t *testing.T) {
// Setup token claims with all values
tokenClaims := &claims{
Email: "token-email@example.com",
Name: "Token Name",
PreferredUsername: "token_username",
}
// Setup userinfo claims
userinfoClaims := &claims{
Email: "userinfo-email@example.com",
Name: "UserInfo Name",
PreferredUsername: "userinfo_username",
}
// Test with ForceUserInfo disabled
err := mergeClaims(tokenClaims, userinfoClaims, false)
require.NoError(t, err)
// Verify token data was preserved
assert.Equal(t, "token-email@example.com", tokenClaims.Email)
assert.Equal(t, "Token Name", tokenClaims.Name)
assert.Equal(t, "token_username", tokenClaims.PreferredUsername)
})
t.Run("Missing values - should use userinfo when token is missing values", func(t *testing.T) {
// Setup token claims with missing values
tokenClaims := &claims{
Email: "token-email@example.com",
// Missing Name and PreferredUsername
}
// Setup userinfo claims
userinfoClaims := &claims{
Email: "userinfo-email@example.com",
Name: "UserInfo Name",
PreferredUsername: "userinfo_username",
}
// Test with ForceUserInfo disabled, but missing values in token
err := mergeClaims(tokenClaims, userinfoClaims, false)
require.NoError(t, err)
// Verify token email was kept, but missing fields were filled from userinfo
assert.Equal(t, "token-email@example.com", tokenClaims.Email)
assert.Equal(t, "UserInfo Name", tokenClaims.Name)
assert.Equal(t, "userinfo_username", tokenClaims.PreferredUsername)
})
t.Run("Use nickname when preferred_username is missing", func(t *testing.T) {
// Setup token claims with missing preferred_username
tokenClaims := &claims{
Email: "token-email@example.com",
Name: "Token Name",
// Missing PreferredUsername
}
// Setup userinfo claims with nickname but no preferred_username
userinfoClaims := &claims{
Email: "userinfo-email@example.com",
Name: "UserInfo Name",
Nickname: "userinfo_nickname",
// Missing PreferredUsername to test fallback to nickname
}
// Test with ForceUserInfo disabled
err := mergeClaims(tokenClaims, userinfoClaims, false)
require.NoError(t, err)
// Verify nickname was used for preferred_username
assert.Equal(t, "userinfo_nickname", tokenClaims.PreferredUsername)
})
t.Run("Error when email is missing", func(t *testing.T) {
// Setup token claims with missing email
tokenClaims := &claims{
// Missing Email
Name: "Token Name",
PreferredUsername: "token_username",
}
// Setup userinfo claims also with missing email
userinfoClaims := &claims{
// Missing Email
Name: "UserInfo Name",
PreferredUsername: "userinfo_username",
}
// Test with ForceUserInfo disabled
err := mergeClaims(tokenClaims, userinfoClaims, false)
// Verify error is returned for missing email
require.Error(t, err)
assert.IsType(t, &user.ErrNoOpenIDEmailProvided{}, err)
})
}

14
pkg/modules/auth/openid/providers.go
View File

@@ -125,6 +125,7 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid
"scope",
"emailfallback",
"usernamefallback",
"forceuserinfo",
},
requiredKeys...,
)
@@ -180,6 +181,18 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid
usernameFallback = usernameFallbackTypedValue
}
}
var forceUserInfo = false
forceUserInfoValue, exists := pi["forceuserinfo"]
if exists {
forceUserInfoTypedValue, ok := forceUserInfoValue.(bool)
if ok {
forceUserInfo = forceUserInfoTypedValue
} else {
log.Errorf("forceuserinfo is not a boolean for provider %s, value: %v", key, forceUserInfoValue)
}
}
provider = &Provider{
Name: name,
Key: key,
@@ -190,6 +203,7 @@ func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provid
Scope: scope,
EmailFallback: emailFallback,
UsernameFallback: usernameFallback,
ForceUserInfo: forceUserInfo,
}
cl, is := pi["clientid"].(int)