diff --git a/internal/api/client/auth/auth.go b/internal/api/client/auth/auth.go index 047ccd8b9..bcc338ce0 100644 --- a/internal/api/client/auth/auth.go +++ b/internal/api/client/auth/auth.go @@ -48,21 +48,10 @@ const ( sessionRedirectURI = "redirect_uri" sessionForceLogin = "force_login" sessionResponseType = "response_type" - sessionCode = "code" sessionScope = "scope" sessionState = "state" ) -var sessionKeys []string = []string{ - sessionUserID, - sessionClientID, - sessionRedirectURI, - sessionForceLogin, - sessionResponseType, - sessionCode, - sessionScope, -} - // Module implements the ClientAPIModule interface for type Module struct { config *config.Config diff --git a/internal/api/client/auth/authorize.go b/internal/api/client/auth/authorize.go index 27a3cc7f9..a10408723 100644 --- a/internal/api/client/auth/authorize.go +++ b/internal/api/client/auth/authorize.go @@ -27,6 +27,7 @@ import ( "github.com/gin-contrib/sessions" "github.com/gin-gonic/gin" + "github.com/google/uuid" "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" @@ -51,14 +52,15 @@ func (m *Module) AuthorizeGETHandler(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - l.Tracef("parsed auth form: %+v", form) + l.Debugf("parsed auth form: %+v", form) if err := extractAuthForm(s, form); err != nil { l.Debugf(fmt.Sprintf("error parsing form at /oauth/authorize: %s", err)) + m.clearSession(s) c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - c.Redirect(http.StatusFound, AuthSignInPath) + c.Redirect(http.StatusSeeOther, AuthSignInPath) return } @@ -140,7 +142,7 @@ func (m *Module) AuthorizePOSTHandler(c *gin.Context) { forceLogin, ok := s.Get(sessionForceLogin).(string) if !ok { - errs = append(errs, "session missing force_login") + forceLogin = "false" } responseType, ok := s.Get(sessionResponseType).(string) @@ -211,5 +213,6 @@ func extractAuthForm(s sessions.Session, form *model.OAuthAuthorize) error { s.Set(sessionClientID, form.ClientID) s.Set(sessionRedirectURI, form.RedirectURI) s.Set(sessionScope, form.Scope) + s.Set(sessionState, uuid.NewString()) return s.Save() } diff --git a/internal/api/client/auth/callback.go b/internal/api/client/auth/callback.go index a9e22a2d9..8bf2a50b5 100644 --- a/internal/api/client/auth/callback.go +++ b/internal/api/client/auth/callback.go @@ -20,23 +20,29 @@ package auth import ( "errors" + "fmt" + "net" "net/http" + "strconv" "strings" "github.com/gin-contrib/sessions" "github.com/gin-gonic/gin" + "github.com/google/uuid" + "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/oidc" + "github.com/superseriousbusiness/gotosocial/internal/util" ) // CallbackGETHandler parses a token from an external auth provider. func (m *Module) CallbackGETHandler(c *gin.Context) { - s := sessions.Default(c) + s := sessions.Default(c) // first make sure the state set in the cookie is the same as the state returned from the external provider state := c.Query(callbackStateParam) if state == "" { - m.clearSession(s) + m.clearSession(s) c.JSON(http.StatusForbidden, gin.H{"error": "state query not found on callback"}) return } @@ -44,13 +50,13 @@ func (m *Module) CallbackGETHandler(c *gin.Context) { savedStateI := s.Get(sessionState) savedState, ok := savedStateI.(string) if !ok { - m.clearSession(s) + m.clearSession(s) c.JSON(http.StatusForbidden, gin.H{"error": "state not found in session"}) return } if state != savedState { - m.clearSession(s) + m.clearSession(s) c.JSON(http.StatusForbidden, gin.H{"error": "state mismatch"}) return } @@ -59,21 +65,37 @@ func (m *Module) CallbackGETHandler(c *gin.Context) { claims, err := m.idp.HandleCallback(c.Request.Context(), code) if err != nil { - m.clearSession(s) + m.clearSession(s) c.JSON(http.StatusForbidden, gin.H{"error": err.Error()}) return } - user, err := m.parseUserFromClaims(claims) + // We can use the client_id on the session to retrieve info about the app associated with the client_id + clientID, ok := s.Get(sessionClientID).(string) + if !ok || clientID == "" { + m.clearSession(s) + c.JSON(http.StatusInternalServerError, gin.H{"error": "no client_id found in session during callback"}) + return + } + app := >smodel.Application{ + ClientID: clientID, + } + if err := m.db.GetWhere([]db.Where{{Key: sessionClientID, Value: app.ClientID}}, app); err != nil { + m.clearSession(s) + c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("no application found for client id %s", clientID)}) + return + } + + user, err := m.parseUserFromClaims(claims, net.IP(c.ClientIP()), app.ID) if err != nil { - m.clearSession(s) + m.clearSession(s) c.JSON(http.StatusForbidden, gin.H{"error": err.Error()}) return } s.Set(sessionUserID, user.ID) if err := s.Save(); err != nil { - m.clearSession(s) + m.clearSession(s) c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } @@ -81,19 +103,117 @@ func (m *Module) CallbackGETHandler(c *gin.Context) { c.Redirect(http.StatusFound, OauthAuthorizePath) } -func (m *Module) parseUserFromClaims(claims *oidc.Claims) (*gtsmodel.User, error) { - if claims.Email == "" { - return nil, errors.New("no email returned in claims") - } - // see if we already have a user for this email address +func (m *Module) parseUserFromClaims(claims *oidc.Claims, ip net.IP, appID string) (*gtsmodel.User, error) { + if claims.Email == "" { + return nil, errors.New("no email returned in claims") + } + // see if we already have a user for this email address + user := >smodel.User{} + err := m.db.GetWhere([]db.Where{{Key: "email", Value: claims.Email}}, user) + if err == nil { + // we do! so we can just return it + return user, nil + } - if claims.Name == "" { + if _, ok := err.(db.ErrNoEntries); !ok { + // we have an actual error in the database + return nil, fmt.Errorf("error checking database for email %s: %s", claims.Email, err) + } + + // maybe we have an unconfirmed user + err = m.db.GetWhere([]db.Where{{Key: "unconfirmed_email", Value: claims.Email}}, user) + if err == nil { + // user is unconfirmed so return an error + return nil, fmt.Errorf("user with email address %s is unconfirmed", claims.Email) + } + + if _, ok := err.(db.ErrNoEntries); !ok { + // we have an actual error in the database + return nil, fmt.Errorf("error checking database for email %s: %s", claims.Email, err) + } + + // we don't have a confirmed or unconfirmed user with the claimed email address + // however, because we trust the OIDC provider, we should now create a user + account with the provided claims + + // check if the email address is available for use; if it's not there's nothing we can so + if err := m.db.IsEmailAvailable(claims.Email); err != nil { + return nil, fmt.Errorf("email %s not available: %s", claims.Email, err) + } + + // now we need a username + var username string + + // make sure claims.Name is defined since we'll be using that for the username + if claims.Name == "" { return nil, errors.New("no name returned in claims") } - username := "" - nameParts := strings.Split(claims.Name, " ") - for i, n := range nameParts { -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - } + + // check if we can just use claims.Name as-is + err = util.ValidateUsername(claims.Name) + if err == nil { + // the name we have on the claims is already a valid username + username = claims.Name + } else { + // not a valid username so we have to fiddle with it to try to make it valid + // first trim leading and trailing whitespace + trimmed := strings.TrimSpace(claims.Name) + // underscore any spaces in the middle of the name + underscored := strings.ReplaceAll(trimmed, " ", "_") + // lowercase the whole thing + lower := strings.ToLower(underscored) + // see if this is valid.... + if err := util.ValidateUsername(lower); err == nil { + // we managed to get a valid username + username = lower + } else { + return nil, fmt.Errorf("couldn't parse a valid username from claims.Name value of %s", claims.Name) + } + } + + var iString string + var found bool + // if the username isn't available we need to iterate on it until we find one that is + // we should try to do this in a predictable way so we just keep iterating i by one and trying + // the username with that number on the end + // + // note that for the first iteration, iString is still "" when the check is made, so our first choice + // is still the raw username with no integer stuck on the end + for i := 1; !found; i = i + 1 { + if err := m.db.IsUsernameAvailable(username + iString); err != nil { + if strings.Contains(err.Error(), "db error") { + // if there's an actual db error we should return + return nil, fmt.Errorf("error checking username availability: %s", err) + } + } else { + // no error so we've found a username that works + found = true + username = username + iString + continue + } + iString = strconv.Itoa(i) + } + + // check if the user is in any recognised admin groups + var admin bool + for _, g := range claims.Groups { + if strings.EqualFold(g, "admin") || strings.EqualFold(g, "admins") { + admin = true + } + } + + // we still need to set *a* password even if it's not a password the user will end up using, so set something random + // in this case, we'll just set two uuids on top of each other, which should be long + random enough to baffle any attempts to crack. + // + // if the user ever wants to log in using gts password rather than oidc flow, they'll have to request a password reset, which is fine + password := uuid.NewString() + uuid.NewString() + + // create the user! this will also create an account and store it in the database so we don't need to do that here + user, err = m.db.NewSignup(username, "", m.config.AccountsConfig.RequireApproval, claims.Email, password, ip, "", appID, claims.EmailVerified, admin) + if err != nil { + return nil, fmt.Errorf("error creating user: %s", err) + } + + return user, nil + } diff --git a/internal/api/client/auth/signin.go b/internal/api/client/auth/signin.go index 0e6895814..543505cbd 100644 --- a/internal/api/client/auth/signin.go +++ b/internal/api/client/auth/signin.go @@ -24,7 +24,6 @@ import ( "github.com/gin-contrib/sessions" "github.com/gin-gonic/gin" - "github.com/google/uuid" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "golang.org/x/crypto/bcrypt" @@ -44,13 +43,15 @@ func (m *Module) SignInGETHandler(c *gin.Context) { l.Trace("entering sign in handler") if m.idp != nil { s := sessions.Default(c) - state := uuid.NewString() - s.Set(sessionState, state) - if err := s.Save(); err != nil { + + stateI := s.Get(sessionState) + state, ok := stateI.(string) + if !ok { m.clearSession(s) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusForbidden, gin.H{"error": "state not found in session"}) return } + redirect := m.idp.AuthCodeURL(state) l.Debugf("redirecting to external idp at %s", redirect) c.Redirect(http.StatusSeeOther, redirect) diff --git a/internal/api/client/auth/util.go b/internal/api/client/auth/util.go index cbba78871..48fe4748a 100644 --- a/internal/api/client/auth/util.go +++ b/internal/api/client/auth/util.go @@ -2,17 +2,14 @@ package auth import ( "github.com/gin-contrib/sessions" - "github.com/superseriousbusiness/gotosocial/internal/router" ) func (m *Module) clearSession(s sessions.Session) { - for _, key := range sessionKeys { - s.Delete(key) - } + s.Clear() - newOptions := router.SessionOptions(m.config) - newOptions.MaxAge = -1 // instruct browser to delete cookie immediately - s.Options(newOptions) + // newOptions := router.SessionOptions(m.config) + // newOptions.MaxAge = -1 // instruct browser to delete cookie immediately + // s.Options(newOptions) if err := s.Save(); err != nil { panic(err) diff --git a/internal/cliactions/admin/account/account.go b/internal/cliactions/admin/account/account.go index 3bb1afada..123913327 100644 --- a/internal/cliactions/admin/account/account.go +++ b/internal/cliactions/admin/account/account.go @@ -64,7 +64,7 @@ var Create cliactions.GTSAction = func(ctx context.Context, c *config.Config, lo return err } - _, err = dbConn.NewSignup(username, "", false, email, password, nil, "", "") + _, err = dbConn.NewSignup(username, "", false, email, password, nil, "", "", false, false) if err != nil { return err } diff --git a/internal/db/db.go b/internal/db/db.go index bbe780e80..c764cc716 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -177,9 +177,9 @@ type DB interface { // C) something went wrong in the db IsEmailAvailable(email string) error - // NewSignup creates a new user in the database with the given parameters, with an *unconfirmed* email address. + // NewSignup creates a new user in the database with the given parameters. // By the time this function is called, it should be assumed that all the parameters have passed validation! - NewSignup(username string, reason string, requireApproval bool, email string, password string, signUpIP net.IP, locale string, appID string) (*gtsmodel.User, error) + NewSignup(username string, reason string, requireApproval bool, email string, password string, signUpIP net.IP, locale string, appID string, emailVerified bool, admin bool) (*gtsmodel.User, error) // SetHeaderOrAvatarForAccountID sets the header or avatar for the given accountID to the given media attachment. SetHeaderOrAvatarForAccountID(mediaAttachment *gtsmodel.MediaAttachment, accountID string) error diff --git a/internal/db/pg/pg.go b/internal/db/pg/pg.go index 31312b61e..d49c50114 100644 --- a/internal/db/pg/pg.go +++ b/internal/db/pg/pg.go @@ -548,38 +548,49 @@ func (ps *postgresService) IsEmailAvailable(email string) error { return nil } -func (ps *postgresService) NewSignup(username string, reason string, requireApproval bool, email string, password string, signUpIP net.IP, locale string, appID string) (*gtsmodel.User, error) { +func (ps *postgresService) NewSignup(username string, reason string, requireApproval bool, email string, password string, signUpIP net.IP, locale string, appID string, emailVerified bool, admin bool) (*gtsmodel.User, error) { key, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { ps.log.Errorf("error creating new rsa key: %s", err) return nil, err } - newAccountURIs := util.GenerateURIsForAccount(username, ps.config.Protocol, ps.config.Host) - newAccountID, err := id.NewRandomULID() + // if something went wrong while creating a user, we might already have an account, so check here first... + a := >smodel.Account{} + err = ps.conn.Model(a).Where("username = ?", username).Where("? IS NULL", pg.Ident("domain")).Select() if err != nil { - return nil, err - } + // there's been an actual error + if err != pg.ErrNoRows { + return nil, fmt.Errorf("db error checking existence of account: %s", err) + } - a := >smodel.Account{ - ID: newAccountID, - Username: username, - DisplayName: username, - Reason: reason, - URL: newAccountURIs.UserURL, - PrivateKey: key, - PublicKey: &key.PublicKey, - PublicKeyURI: newAccountURIs.PublicKeyURI, - ActorType: gtsmodel.ActivityStreamsPerson, - URI: newAccountURIs.UserURI, - InboxURI: newAccountURIs.InboxURI, - OutboxURI: newAccountURIs.OutboxURI, - FollowersURI: newAccountURIs.FollowersURI, - FollowingURI: newAccountURIs.FollowingURI, - FeaturedCollectionURI: newAccountURIs.CollectionURI, - } - if _, err = ps.conn.Model(a).Insert(); err != nil { - return nil, err + // we just don't have an account yet create one + newAccountURIs := util.GenerateURIsForAccount(username, ps.config.Protocol, ps.config.Host) + newAccountID, err := id.NewRandomULID() + if err != nil { + return nil, err + } + + a = >smodel.Account{ + ID: newAccountID, + Username: username, + DisplayName: username, + Reason: reason, + URL: newAccountURIs.UserURL, + PrivateKey: key, + PublicKey: &key.PublicKey, + PublicKeyURI: newAccountURIs.PublicKeyURI, + ActorType: gtsmodel.ActivityStreamsPerson, + URI: newAccountURIs.UserURI, + InboxURI: newAccountURIs.InboxURI, + OutboxURI: newAccountURIs.OutboxURI, + FollowersURI: newAccountURIs.FollowersURI, + FollowingURI: newAccountURIs.FollowingURI, + FeaturedCollectionURI: newAccountURIs.CollectionURI, + } + if _, err = ps.conn.Model(a).Insert(); err != nil { + return nil, err + } } pw, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) @@ -594,14 +605,25 @@ func (ps *postgresService) NewSignup(username string, reason string, requireAppr u := >smodel.User{ ID: newUserID, - AccountID: newAccountID, + AccountID: a.ID, EncryptedPassword: string(pw), - SignUpIP: signUpIP, + SignUpIP: signUpIP.To4(), Locale: locale, UnconfirmedEmail: email, CreatedByApplicationID: appID, Approved: !requireApproval, // if we don't require moderator approval, just pre-approve the user } + + if emailVerified { + u.ConfirmedAt = time.Now() + u.Email = email + } + + if admin { + u.Admin = true + u.Moderator = true + } + if _, err = ps.conn.Model(u).Insert(); err != nil { return nil, err } diff --git a/internal/processing/account/create.go b/internal/processing/account/create.go index 8b29f147f..58ac0e43c 100644 --- a/internal/processing/account/create.go +++ b/internal/processing/account/create.go @@ -45,7 +45,7 @@ func (p *processor) Create(applicationToken oauth2.TokenInfo, application *gtsmo } l.Trace("creating new username and account") - user, err := p.db.NewSignup(form.Username, util.RemoveHTML(reason), p.config.AccountsConfig.RequireApproval, form.Email, form.Password, form.IP, form.Locale, application.ID) + user, err := p.db.NewSignup(form.Username, util.RemoveHTML(reason), p.config.AccountsConfig.RequireApproval, form.Email, form.Password, form.IP, form.Locale, application.ID, false, false) if err != nil { return nil, fmt.Errorf("error creating new signup in the database: %s", err) } diff --git a/internal/router/session.go b/internal/router/session.go index 2ba4f1b5f..2b9be2f56 100644 --- a/internal/router/session.go +++ b/internal/router/session.go @@ -38,10 +38,10 @@ func SessionOptions(cfg *config.Config) sessions.Options { return sessions.Options{ Path: "/", Domain: cfg.Host, - MaxAge: 120, // 2 minutes - Secure: true, // only use cookie over https - HttpOnly: true, // exclude javascript from inspecting cookie - SameSite: http.SameSiteStrictMode, // https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cookie-same-site-00#section-4.1.1 + MaxAge: 120, // 2 minutes + Secure: true, // only use cookie over https + HttpOnly: true, // exclude javascript from inspecting cookie + SameSite: http.SameSiteDefaultMode, // https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cookie-same-site-00#section-4.1.1 } }