lunny/gitea

Archived

Fork 1

You've already forked gitea

Code Issues 1.7k Pull Requests 154 Projects Releases 128 Wiki Activity

Refactor: Move login out of models #16199

Merged

lunny merged 63 commits from move-login-out-of-models into main

2021-07-24 10:16:34 +00:00

Conversation 0 Commits 63 Files Changed 77 +3803 -2951

zeripath commented

2021-06-18 22:20:58 +00:00

Contributor

Copy Link

models does far too much. In particular it handles all UserSignin.

It shouldn't be responsible for calling LDAP, SMTP or PAM for signing in.

Therefore we should move this code out of models.

This code has to depend on models - therefore it belongs in services.

There is a package in services called auth and clearly this functionality belongs in there.

Plan:

Change auth.Auth to auth.Method - as they represent methods of authentication.
Move models.UserSignIn into auth
Move models.ExternalUserLogin
Move most of the LoginVia* methods to auth or subpackages
Move Resynchronize functionality to auth
- Involved some restructuring of models/ssh_key.go to reduce the size of this massive file and simplify its files.
Move the rest of the LDAP functionality in to the ldap subpackage
Re-factor the login sources to express an interfaces auth.Source?
- I've done this through some smaller interfaces Authenticator and Synchronizable - which would allow us to extend things in future
Now LDAP is out of models - need to think about modules/auth/ldap and I think all of that functionality might just be moveable
Similarly a lot Oauth2 functionality need not be in models too and should be moved to services/auth/source/oauth2
- modules/auth/oauth2/oauth2.go uses xorm... This is naughty - probably need to move this into models.
- models/oauth2.go - mostly should be in modules/auth/oauth2 or services/auth/source/oauth2
More simplifications of login_source.go may need to be done
Allow wiring in of notify registration - this can now easily be done - but I think we should do it in another PR - see #16178
More refactors...?
- OpenID should probably become an auth Method but I think that can be left for another PR
- Methods should also probably be cleaned up - again another PR I think.
- SSPI still needs more refactors.

`models` does far too much. In particular it handles all `UserSignin`. It shouldn't be responsible for calling LDAP, SMTP or PAM for signing in. Therefore we should move this code out of `models`. This code has to depend on `models` - therefore it belongs in `services`. There is a package in `services` called `auth` and clearly this functionality belongs in there. Plan: - [x] Change `auth.Auth` to `auth.Method` - as they represent methods of authentication. - [x] Move `models.UserSignIn` into `auth` - [x] Move `models.ExternalUserLogin` - [x] Move most of the `LoginVia*` methods to `auth` or subpackages - [x] Move Resynchronize functionality to `auth` - Involved some restructuring of `models/ssh_key.go` to reduce the size of this massive file and simplify its files. - [x] Move the rest of the LDAP functionality in to the ldap subpackage - [x] Re-factor the login sources to express an interfaces `auth.Source`? - I've done this through some smaller interfaces Authenticator and Synchronizable - which would allow us to extend things in future - [x] Now LDAP is out of models - need to think about modules/auth/ldap and I think all of that functionality might just be moveable - [x] Similarly a lot Oauth2 functionality need not be in models too and should be moved to services/auth/source/oauth2 - [x] modules/auth/oauth2/oauth2.go uses xorm... This is naughty - probably need to move this into models. - [x] models/oauth2.go - mostly should be in modules/auth/oauth2 or services/auth/source/oauth2 - [x] More simplifications of login_source.go may need to be done - Allow wiring in of notify registration - *this can now easily be done - but I think we should do it in another PR* - see #16178 - More refactors...? - OpenID should probably become an auth Method but I think that can be left for another PR - Methods should also probably be cleaned up - again another PR I think. - SSPI still needs more refactors.

👍 5 ❤️ 4

lunny reviewed 2021-07-14 01:26:56 +00:00

models/ssh_key_authorized_keys.go

						
				@ -0,0 +1,219 @@

				// Copyright 2021 The Gitea Authors. All rights reserved.

lunny commented

2021-07-14 01:26:56 +00:00

Owner

SSH related files should also be moved out of models

zeripath reviewed 2021-07-14 02:29:44 +00:00

models/ssh_key_authorized_keys.go

						
				@ -0,0 +1,219 @@

				// Copyright 2021 The Gitea Authors. All rights reserved.

zeripath commented

2021-07-14 02:29:44 +00:00

Author

Contributor

One thing at a time...

lunny reviewed 2021-07-14 03:21:18 +00:00

models/ssh_key_authorized_keys.go

						
				@ -0,0 +1,219 @@

				// Copyright 2021 The Gitea Authors. All rights reserved.

lunny commented

2021-07-14 03:21:18 +00:00

Owner

But you have split them from old file?

zeripath reviewed 2021-07-14 07:34:06 +00:00

models/ssh_key_authorized_keys.go

						
				@ -0,0 +1,219 @@

				// Copyright 2021 The Gitea Authors. All rights reserved.

zeripath commented

2021-07-14 07:34:06 +00:00

Author

Contributor

On the advice from @6543 and to prepare for that move.

I think this is already going to be difficult enough to get reviewed without adding in another refactor but if you'd like it in here I can do that.

The issue is that it'll need to be one of the first things merged in 1.16 if not the first thing as it'll be too easy conflict.

On the advice from @6543 and to prepare for that move. I think this is already going to be difficult enough to get reviewed without adding in another refactor but if you'd like it in here I can do that. The issue is that it'll need to be one of the first things merged in 1.16 if not the first thing as it'll be too easy conflict.

lunny reviewed 2021-07-14 09:05:40 +00:00

models/ssh_key_authorized_keys.go

						
				@ -0,0 +1,219 @@

				// Copyright 2021 The Gitea Authors. All rights reserved.

lunny commented

2021-07-14 09:05:40 +00:00

Owner

I think we can keep ssh related code unchanged in this PR or move them to another PR.

zeripath reviewed 2021-07-14 09:15:25 +00:00

models/ssh_key_authorized_keys.go

						
				@ -0,0 +1,219 @@

				// Copyright 2021 The Gitea Authors. All rights reserved.

zeripath commented

2021-07-14 09:15:25 +00:00

Author

Contributor

The changes that have been made are necessary for this PR.

6543 (Migrated from github.com) approved these changes 2021-07-17 19:53:47 +00:00

khmarbaise (Migrated from github.com) approved these changes 2021-07-24 09:12:39 +00:00

This repo is archived. You cannot comment on pull requests.

Reviewers

No reviewers

6543

khmarbaise

Labels

Clear labels

Pull requests that update a dependency file

frontport/done

frontport/main

good first issue

Likely to be an easy fix

Hacktoberfest

hacktoberfest-accepted

features for moderateing spam, abuse, ...

kind/upstream-related

An issue that is caused by an upstream dependency

Performance Issues affecting Big Repositories

performance/cpu

performance/memory

Performance issues affecting memory use

performance/speed

performance issues with slow downs

Issue has been reviewed and confirmed to be present or accepted to be implemented

reviewed/duplicate

reviewed/fixed

A fix for this issue is already in the attached milestone

reviewed/invalid

reviewed/not-a-bug

The reported issue is the intended behavior

status/needs-feedback

No Label

hacktoberfest-accepted

kind/upstream-related

status/needs-feedback

Milestone

Clear milestone

No items

No Milestone

1.16.0

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

2 Participants

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: lunny/gitea#16199

Reference in New Issue

Repository

lunny/gitea

Title

Body

Block a user

Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.

User to block:

Optional note:

The note is not visible to the blocked user.

No description provided.

Delete Branch "move-login-out-of-models"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?

				`@ -0,0 +1,219 @@`
				`// Copyright 2021 The Gitea Authors. All rights reserved.`