Oauth username provider suffix added #17780
Conversation
Codecov Report
@@ Coverage Diff @@
## main #17780 +/- ##
=======================================
Coverage ? 45.54%
=======================================
Files ? 808
Lines ? 89969
Branches ? 0
=======================================
Hits ? 40975
Misses ? 42449
Partials ? 6545
Continue to review full report at Codecov.
|
GiteaBot
added
the
lgtm/need 2
|
|
we create the provider db table and the namespace field representing the provider in the user model → If some providers are modified, we have only to update the provider table. Or if deleted, we fetch all users with that provider and delete them. This strategy seems good. |
|
I have implemented the above comment. I'm sorry that I have made a trivial mistake and bother you. |
| if err != nil { | ||
| return nil, err | ||
| } else if !has { | ||
| return nil, ErrUserNotExist{id, "", 0} |
There was a problem hiding this comment.
ErrUserNotExist Wrong error to give here.
| db.RegisterModel(new(OAuth)) | ||
| } | ||
|
|
||
| func getOAuthByID(e db.Engine, id int64) (*OAuth, error) { |
There was a problem hiding this comment.
These function is essentially deadcode as it isn't used other than GetOAuthByID which on itself isn't used.
There was a problem hiding this comment.
General note, a lot of dead code functions are in these files...
There was a problem hiding this comment.
I think we already have the table, why should we create a new one?
|
|
||
| func getOAuthByName(e db.Engine, name string) (*OAuth, error) { | ||
| if len(name) == 0 { | ||
| return nil, ErrUserNotExist{0, name, 0} |
| if err != nil { | ||
| return nil, err | ||
| } else if !has { | ||
| return nil, ErrUserNotExist{0, name, 0} |
| @@ -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 | |||
There was a problem hiding this comment.
| OAuthProvider int64 | |
| OAuthProviderID int64 |
You will also need a migration for this - as existing users won't have any.
There was a problem hiding this comment.
We have an external_account table, I think it's already ready.
| @@ -649,14 +649,28 @@ func SignInOAuthCallback(ctx *context.Context) { | |||
| ctx.ServerError("CreateUser", err) | |||
| return | |||
| } | |||
|
|
|||
| oauthProviderName := strings.ToLower(provider) | |||
There was a problem hiding this comment.
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.
| @@ -0,0 +1,125 @@ | |||
| package models | |||
| @@ -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 | |||
There was a problem hiding this comment.
We have an external_account table, I think it's already ready.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
This is in progress for a while now, I'm closing it to avoid stale pull requests. Please reopen when it's ready for review. |
I implement the issue #7014.
In preparation for introducing the second login source, usernames should be namespace-wise. The suffix "-{provider}" will be added automatically.