[bug] Fix OIDC users requiring second approval (#371)

* tidy up NewSignup
* pre-approve users created via OIDC
This commit is contained in:
tobi 2022-01-31 16:03:47 +01:00 committed by GitHub
parent 5be8a7a7ea
commit 18e7537393
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 13 deletions

View file

@ -30,8 +30,6 @@ import (
"github.com/gin-contrib/sessions" "github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin" "github.com/gin-gonic/gin"
"github.com/google/uuid" "github.com/google/uuid"
"github.com/spf13/viper"
"github.com/superseriousbusiness/gotosocial/internal/config"
"github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/oidc" "github.com/superseriousbusiness/gotosocial/internal/oidc"
@ -206,19 +204,27 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i
} }
} }
// we still need to set *a* password even if it's not a password the user will end up using, so set something random // 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. // 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 // 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() password := uuid.NewString() + uuid.NewString()
// Since this user is created via oidc, which has been set up by the admin, we can assume that the account is already
// implicitly approved, and that the email address has already been verified: otherwise, we end up in situations where
// the admin first approves the user in OIDC, and then has to approve them again in GoToSocial, which doesn't make sense.
//
// In other words, if a user logs in via OIDC, they should be able to use their account straight away.
//
// See: https://github.com/superseriousbusiness/gotosocial/issues/357
requireApproval := false
emailVerified := true
// create the user! this will also create an account and store it in the database so we don't need to do that here // create the user! this will also create an account and store it in the database so we don't need to do that here
requireApproval := viper.GetBool(config.Keys.AccountsApprovalRequired) user, err = m.db.NewSignup(ctx, username, "", requireApproval, claims.Email, password, ip, "", appID, emailVerified, admin)
user, err = m.db.NewSignup(ctx, username, "", requireApproval, claims.Email, password, ip, "", appID, claims.EmailVerified, admin)
if err != nil { if err != nil {
return nil, fmt.Errorf("error creating user: %s", err) return nil, fmt.Errorf("error creating user: %s", err)
} }
return user, nil return user, nil
} }

View file

@ -94,13 +94,13 @@ func (a *adminDB) NewSignup(ctx context.Context, username string, reason string,
// if something went wrong while creating a user, we might already have an account, so check here first... // if something went wrong while creating a user, we might already have an account, so check here first...
acct := &gtsmodel.Account{} acct := &gtsmodel.Account{}
err = a.conn.NewSelect(). q := a.conn.NewSelect().
Model(acct). Model(acct).
Where("username = ?", username). Where("username = ?", username).
WhereGroup(" AND ", whereEmptyOrNull("domain")). WhereGroup(" AND ", whereEmptyOrNull("domain"))
Scan(ctx)
if err != nil { if err := q.Scan(ctx); err != nil {
// we just don't have an account yet so create one // we just don't have an account yet so create one before we proceed
accountURIs := uris.GenerateURIsForAccount(username) accountURIs := uris.GenerateURIsForAccount(username)
accountID, err := id.NewRandomULID() accountID, err := id.NewRandomULID()
if err != nil { if err != nil {
@ -125,6 +125,7 @@ func (a *adminDB) NewSignup(ctx context.Context, username string, reason string,
FollowingURI: accountURIs.FollowingURI, FollowingURI: accountURIs.FollowingURI,
FeaturedCollectionURI: accountURIs.CollectionURI, FeaturedCollectionURI: accountURIs.CollectionURI,
} }
if _, err = a.conn. if _, err = a.conn.
NewInsert(). NewInsert().
Model(acct). Model(acct).
@ -158,6 +159,7 @@ func (a *adminDB) NewSignup(ctx context.Context, username string, reason string,
if emailVerified { if emailVerified {
u.ConfirmedAt = time.Now() u.ConfirmedAt = time.Now()
u.Email = email u.Email = email
u.UnconfirmedEmail = ""
} }
if admin { if admin {