Harden OIDC migration and make optional

This commit hardens the migration part of the OIDC from
the old username based approach to the new sub based approach
and makes it possible for the operator to opt out entirely.

Fixes #1990

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
Kristoffer Dalby 2024-10-04 12:24:35 +02:00 committed by Juan Font
parent 64bb56352f
commit 78214699ad
3 changed files with 27 additions and 7 deletions

View file

@ -443,7 +443,9 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
// This check is for legacy, if the user cannot be found by the OIDC identifier
// look it up by username. This should only be needed once.
if user == nil {
// This branch will presist for a number of versions after the OIDC migration and
// then be removed following a deprecation.
if a.cfg.MapLegacyUsers && user == nil {
user, err = a.db.GetUserByName(claims.Username)
if err != nil && !errors.Is(err, db.ErrUserNotFound) {
return nil, fmt.Errorf("creating or updating user: %w", err)
@ -453,6 +455,15 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
if user == nil {
user = &types.User{}
}
// If the user exists, but it already has a provider identifier (OIDC sub), create a new user.
// This is to prevent users that have already been migrated to the new OIDC format
// to be updated with the new OIDC identifier inexplicitly which might be the cause of an
// account takeover.
if user.ProviderIdentifier != "" {
log.Info().Str("username", claims.Username).Str("sub", claims.Sub).Msg("user found by username, but has provider identifier, creating new user.")
user = &types.User{}
}
}
user.FromClaim(claims)