Skip to content

Oauth username provider suffix added #17780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 125 additions & 0 deletions models/oauth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package models
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyright head missing


import (
"fmt"
"strings"

"code.gitea.io/gitea/models/db"
)

// OAuth Login Source
type OAuth struct {
ID int64 `xorm:"pk autoincr"`
Name string
}

func init() {
db.RegisterModel(new(OAuth))
}

func getOAuthByID(e db.Engine, id int64) (*OAuth, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These function is essentially deadcode as it isn't used other than GetOAuthByID which on itself isn't used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General note, a lot of dead code functions are in these files...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already have the table, why should we create a new one?

o := new(OAuth)
has, err := e.ID(id).Get(o)
if err != nil {
return nil, err
} else if !has {
return nil, ErrUserNotExist{id, "", 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ErrUserNotExist Wrong error to give here.

}
return o, nil
}

// GetOAuthByID returns the oauth object by given ID if exists.
func GetOAuthByID(id int64) (*OAuth, error) {
return getOAuthByID(db.GetEngine(db.DefaultContext), id)
}

// GetOAuthByName returns oauth by given name.
func GetOAuthByName(name string) (*OAuth, error) {
return getOAuthByName(db.GetEngine(db.DefaultContext), name)
}

func getOAuthByName(e db.Engine, name string) (*OAuth, error) {
if len(name) == 0 {
return nil, ErrUserNotExist{0, name, 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto wrong error.

}
o := &OAuth{Name: strings.ToLower(name)}
has, err := e.Get(o)
if err != nil {
return nil, err
} else if !has {
return nil, ErrUserNotExist{0, name, 0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto wrong error.

}
return o, nil
}

// CreateOAuth creates record of a new oauth.
func CreateOAuth(o *OAuth) (err error) {
sess := db.NewSession(db.DefaultContext)
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
}

if _, err = sess.Insert(o); err != nil {
return err
}

return sess.Commit()
}

func validateOAuth(o *OAuth) error {

return nil
}

func updateOAuth(e db.Engine, o *OAuth) error {
if err := validateOAuth(o); err != nil {
return err
}

_, err := e.ID(o.ID).AllCols().Update(o)
return err
}

// UpdateOAuth updates oauth's information.
func UpdateOAuth(o *OAuth) error {
return updateOAuth(db.GetEngine(db.DefaultContext), o)
}

// UpdateOAuthCols update user according special columns
func UpdateOAuthCols(o *OAuth, cols ...string) error {
return updateOAuthCols(db.GetEngine(db.DefaultContext), o, cols...)
}

func updateOAuthCols(e db.Engine, o *OAuth, cols ...string) error {
if err := validateOAuth(o); err != nil {
return err
}

_, err := e.ID(o.ID).Cols(cols...).Update(o)
return err
}

func deleteOAuth(e db.Engine, o *OAuth) error {
if _, err := e.ID(o.ID).Delete(new(OAuth)); err != nil {
return fmt.Errorf("Delete: %v", err)
}

return nil
}

// DeleteOAuth deletes the record of oauth
func DeleteOAuth(o *OAuth) (err error) {
sess := db.NewSession(db.DefaultContext)
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
}

if err = deleteOAuth(sess, o); err != nil {
// Note: don't wrapper error here.
return err
}

return sess.Commit()
}
3 changes: 3 additions & 0 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ type User struct {
DiffViewStyle string `xorm:"NOT NULL DEFAULT ''"`
Theme string `xorm:"NOT NULL DEFAULT ''"`
KeepActivityPrivate bool `xorm:"NOT NULL DEFAULT false"`

// OAuth
OAuthProvider int64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OAuthProvider int64
OAuthProviderID int64

You will also need a migration for this - as existing users won't have any.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an external_account table, I think it's already ready.

}

func init() {
Expand Down
28 changes: 21 additions & 7 deletions routers/web/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,14 +649,28 @@ func SignInOAuthCallback(ctx *context.Context) {
ctx.ServerError("CreateUser", err)
return
}

oauthProviderName := strings.ToLower(provider)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this actually?

This adds the name of the oauth into a new database table, link within the user to the ID of that oauth. But that doesn't solve the issue of adding a suffix.

Also, we already have this, look at the LoginSource field ;) which should give you the same way of getting the Oauth's name.


oauth, err := models.GetOAuthByName(oauthProviderName)

if err != nil {
oauth = &models.OAuth{
Name: oauthProviderName,
}

models.CreateOAuth(oauth)
}

u = &models.User{
Name: getUserName(&gothUser),
FullName: gothUser.Name,
Email: gothUser.Email,
IsActive: !setting.OAuth2Client.RegisterEmailConfirm,
LoginType: login.OAuth2,
LoginSource: loginSource.ID,
LoginName: gothUser.UserID,
Name: getUserName(&gothUser),
FullName: gothUser.Name,
Email: gothUser.Email,
IsActive: !setting.OAuth2Client.RegisterEmailConfirm,
LoginType: login.OAuth2,
LoginSource: loginSource.ID,
LoginName: gothUser.UserID,
OAuthProvider: oauth.ID,
}

if !createAndHandleCreatedUser(ctx, base.TplName(""), nil, u, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) {
Expand Down