From 71cad7aa13ce3e3a568c21b6dfe7d26fb144341f Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 27 Jan 2025 14:55:47 +0100 Subject: [PATCH] chore(auth): refactor creating users in openid and ldap --- pkg/config/config.go | 8 +++++- pkg/modules/auth/auth.go | 27 +++++++++++++++++++ pkg/modules/auth/ldap/ldap.go | 43 +++++++++---------------------- pkg/modules/auth/openid/openid.go | 27 +------------------ 4 files changed, 47 insertions(+), 58 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 4801c3231..59e69ff05 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -86,7 +86,10 @@ const ( AuthLdapVerifyTLS Key = `auth.ldap.verifytls` AuthLdapBindDN Key = `auth.ldap.binddn` // #nosec G101 - AuthLdapBindPassword Key = `auth.ldap.bindpassword` + AuthLdapBindPassword Key = `auth.ldap.bindpassword` + AuthLdapAttributeUsername Key = `auth.ldap.attribute.username` + AuthLdapAttributeEmail Key = `auth.ldap.attribute.email` + AuthLdapAttributeDisplayname Key = `auth.ldap.attribute.displayname` LegalImprintURL Key = `legal.imprinturl` LegalPrivacyURL Key = `legal.privacyurl` @@ -348,6 +351,9 @@ func InitDefaultConfig() { AuthLdapPort.setDefault(389) AuthLdapUseTLS.setDefault(true) AuthLdapVerifyTLS.setDefault(true) + AuthLdapAttributeUsername.setDefault("uid") + AuthLdapAttributeEmail.setDefault("mail") + AuthLdapAttributeDisplayname.setDefault("displayName") // Database DatabaseType.setDefault("sqlite") diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index f78702387..5b0deff6e 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -27,8 +27,10 @@ import ( "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/web" + petname "github.com/dustinkirkland/golang-petname" "github.com/golang-jwt/jwt/v5" "github.com/labstack/echo/v4" + "xorm.io/xorm" ) // These are all valid auth types @@ -127,3 +129,28 @@ func GetAuthFromClaims(c echo.Context) (a web.Auth, err error) { } return nil, echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."}) } + +func CreateUserWithRandomUsername(s *xorm.Session, uu *user.User) (u *user.User, err error) { + // Check if we actually have a preferred username and generate a random one right away if we don't + for { + if uu.Username == "" { + uu.Username = petname.Generate(3, "-") + } + + u, err = user.CreateUser(s, uu) + if err == nil { + break + } + + if !user.IsErrUsernameExists(err) { + return nil, err + } + + // If their preferred username is already taken, generate a new one + uu.Username = petname.Generate(3, "-") + } + + // And create their project + err = models.CreateNewProjectForUser(s, u) + return +} diff --git a/pkg/modules/auth/ldap/ldap.go b/pkg/modules/auth/ldap/ldap.go index 1209f1815..d7c933ac1 100644 --- a/pkg/modules/auth/ldap/ldap.go +++ b/pkg/modules/auth/ldap/ldap.go @@ -20,15 +20,13 @@ import ( "fmt" "strings" - petname "github.com/dustinkirkland/golang-petname" - "xorm.io/xorm" - "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/log" - "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" "github.com/go-ldap/ldap/v3" + "xorm.io/xorm" ) func ConnectAndBindToLDAPDirectory() (l *ldap.Conn, err error) { @@ -76,7 +74,12 @@ func AuthenticateUserInLDAP(s *xorm.Session, username, password string) (u *user config.AuthLdapBaseDN.GetString(), ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, fmt.Sprintf("(&(objectClass=inetOrgPerson)(uid=%s))", username), - []string{"dn", "uid", "mail", "displayName"}, + []string{ + "dn", + config.AuthLdapAttributeUsername.GetString(), + config.AuthLdapAttributeEmail.GetString(), + config.AuthLdapAttributeDisplayname.GetString(), + }, nil, ) @@ -102,9 +105,9 @@ func AuthenticateUserInLDAP(s *xorm.Session, username, password string) (u *user } func getOrCreateLdapUser(s *xorm.Session, entry *ldap.Entry) (u *user.User, err error) { - username := entry.GetAttributeValue("uid") - email := entry.GetAttributeValue("mail") - name := entry.GetAttributeValue("displayName") + username := entry.GetAttributeValue(config.AuthLdapAttributeUsername.GetString()) + email := entry.GetAttributeValue(config.AuthLdapAttributeEmail.GetString()) + name := entry.GetAttributeValue(config.AuthLdapAttributeDisplayname.GetString()) u, err = user.GetUserWithEmail(s, &user.User{ Issuer: user.IssuerLDAP, @@ -125,29 +128,7 @@ func getOrCreateLdapUser(s *xorm.Session, entry *ldap.Entry) (u *user.User, err Subject: username, } - // Check if we actually have a preferred username and generate a random one right away if we don't - if uu.Username == "" { - uu.Username = petname.Generate(3, "-") - } - - // TODO abstract this away an use in openid auth and here - u, err = user.CreateUser(s, uu) - if err != nil && !user.IsErrUsernameExists(err) { - return nil, err - } - - // If their preferred username is already taken, generate a random one - if user.IsErrUsernameExists(err) { - uu.Username = petname.Generate(3, "-") - u, err = user.CreateUser(s, uu) - if err != nil { - return nil, err - } - } - - // And create their project - err = models.CreateNewProjectForUser(s, u) - return + return auth.CreateUserWithRandomUsername(s, uu) } return diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 784600baf..a42b59478 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -417,32 +417,7 @@ func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *us Subject: subject, } - // Check if we actually have a preferred username and generate a random one right away if we don't - if uu.Username == "" { - uu.Username = petname.Generate(3, "-") - } - - u, err = user.CreateUser(s, uu) - if err != nil && !user.IsErrUsernameExists(err) { - return nil, err - } - - // If their preferred username is already taken, generate a random one - if user.IsErrUsernameExists(err) { - uu.Username = petname.Generate(3, "-") - u, err = user.CreateUser(s, uu) - if err != nil { - return nil, err - } - } - - // And create their project - err = models.CreateNewProjectForUser(s, u) - if err != nil { - return nil, err - } - - return + return auth.CreateUserWithRandomUsername(s, uu) } // If it exists, check if the email address changed and change it if not