Skip to content

Commit 5d2e11e

Browse files
zeripathzeripath
authored
Refactor: Move login out of models (#16199)
`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.* Rename auth.Auth auth.Method * Restructure ssh_key.go - move functions from models/user.go that relate to ssh_key to ssh_key - split ssh_key.go to try create clearer function domains for allow for future refactors here. Signed-off-by: Andrew Thornton <art27@cantab.net>
1 parent f135a81 commit 5d2e11e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+3785
-2933
lines changed

‎cmd/admin.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import (
1414
"text/tabwriter"
1515

1616
"code.gitea.io/gitea/models"
17-
"code.gitea.io/gitea/modules/auth/oauth2"
1817
"code.gitea.io/gitea/modules/git"
1918
"code.gitea.io/gitea/modules/graceful"
2019
"code.gitea.io/gitea/modules/log"
2120
pwd "code.gitea.io/gitea/modules/password"
2221
repo_module "code.gitea.io/gitea/modules/repository"
2322
"code.gitea.io/gitea/modules/setting"
2423
"code.gitea.io/gitea/modules/storage"
24+
"code.gitea.io/gitea/services/auth/source/oauth2"
2525

2626
"github.com/urfave/cli"
2727
)
@@ -597,7 +597,7 @@ func runRegenerateKeys(_ *cli.Context) error {
597597
return models.RewriteAllPublicKeys()
598598
}
599599

600-
func parseOAuth2Config(c *cli.Context) *models.OAuth2Config {
600+
func parseOAuth2Config(c *cli.Context) *oauth2.Source {
601601
var customURLMapping *oauth2.CustomURLMapping
602602
if c.IsSet("use-custom-urls") {
603603
customURLMapping = &oauth2.CustomURLMapping{
@@ -609,7 +609,7 @@ func parseOAuth2Config(c *cli.Context) *models.OAuth2Config {
609609
} else {
610610
customURLMapping = nil
611611
}
612-
return &models.OAuth2Config{
612+
return &oauth2.Source{
613613
Provider: c.String("provider"),
614614
ClientID: c.String("key"),
615615
ClientSecret: c.String("secret"),
@@ -625,10 +625,10 @@ func runAddOauth(c *cli.Context) error {
625625
}
626626

627627
return models.CreateLoginSource(&models.LoginSource{
628-
Type: models.LoginOAuth2,
629-
Name: c.String("name"),
630-
IsActived: true,
631-
Cfg: parseOAuth2Config(c),
628+
Type: models.LoginOAuth2,
629+
Name: c.String("name"),
630+
IsActive: true,
631+
Cfg: parseOAuth2Config(c),
632632
})
633633
}
634634

@@ -646,7 +646,7 @@ func runUpdateOauth(c *cli.Context) error {
646646
return err
647647
}
648648

649-
oAuth2Config := source.OAuth2()
649+
oAuth2Config := source.Cfg.(*oauth2.Source)
650650

651651
if c.IsSet("name") {
652652
source.Name = c.String("name")
@@ -728,7 +728,7 @@ func runListAuth(c *cli.Context) error {
728728
w := tabwriter.NewWriter(os.Stdout, c.Int("min-width"), c.Int("tab-width"), c.Int("padding"), padChar, flags)
729729
fmt.Fprintf(w, "ID\tName\tType\tEnabled\n")
730730
for _, source := range loginSources {
731-
fmt.Fprintf(w, "%d\t%s\t%s\t%t\n", source.ID, source.Name, models.LoginNames[source.Type], source.IsActived)
731+
fmt.Fprintf(w, "%d\t%s\t%s\t%t\n", source.ID, source.Name, models.LoginNames[source.Type], source.IsActive)
732732
}
733733
w.Flush()
734734

‎cmd/admin_auth_ldap.go‎

Lines changed: 36 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"strings"
1010

1111
"code.gitea.io/gitea/models"
12-
"code.gitea.io/gitea/modules/auth/ldap"
12+
"code.gitea.io/gitea/services/auth/source/ldap"
1313

1414
"github.com/urfave/cli"
1515
)
@@ -172,86 +172,86 @@ func parseLoginSource(c *cli.Context, loginSource *models.LoginSource) {
172172
loginSource.Name = c.String("name")
173173
}
174174
if c.IsSet("not-active") {
175-
loginSource.IsActived = !c.Bool("not-active")
175+
loginSource.IsActive = !c.Bool("not-active")
176176
}
177177
if c.IsSet("synchronize-users") {
178178
loginSource.IsSyncEnabled = c.Bool("synchronize-users")
179179
}
180180
}
181181

182182
// parseLdapConfig assigns values on config according to command line flags.
183-
func parseLdapConfig(c *cli.Context, config *models.LDAPConfig) error {
183+
func parseLdapConfig(c *cli.Context, config *ldap.Source) error {
184184
if c.IsSet("name") {
185-
config.Source.Name = c.String("name")
185+
config.Name = c.String("name")
186186
}
187187
if c.IsSet("host") {
188-
config.Source.Host = c.String("host")
188+
config.Host = c.String("host")
189189
}
190190
if c.IsSet("port") {
191-
config.Source.Port = c.Int("port")
191+
config.Port = c.Int("port")
192192
}
193193
if c.IsSet("security-protocol") {
194194
p, ok := findLdapSecurityProtocolByName(c.String("security-protocol"))
195195
if !ok {
196196
return fmt.Errorf("Unknown security protocol name: %s", c.String("security-protocol"))
197197
}
198-
config.Source.SecurityProtocol = p
198+
config.SecurityProtocol = p
199199
}
200200
if c.IsSet("skip-tls-verify") {
201-
config.Source.SkipVerify = c.Bool("skip-tls-verify")
201+
config.SkipVerify = c.Bool("skip-tls-verify")
202202
}
203203
if c.IsSet("bind-dn") {
204-
config.Source.BindDN = c.String("bind-dn")
204+
config.BindDN = c.String("bind-dn")
205205
}
206206
if c.IsSet("user-dn") {
207-
config.Source.UserDN = c.String("user-dn")
207+
config.UserDN = c.String("user-dn")
208208
}
209209
if c.IsSet("bind-password") {
210-
config.Source.BindPassword = c.String("bind-password")
210+
config.BindPassword = c.String("bind-password")
211211
}
212212
if c.IsSet("user-search-base") {
213-
config.Source.UserBase = c.String("user-search-base")
213+
config.UserBase = c.String("user-search-base")
214214
}
215215
if c.IsSet("username-attribute") {
216-
config.Source.AttributeUsername = c.String("username-attribute")
216+
config.AttributeUsername = c.String("username-attribute")
217217
}
218218
if c.IsSet("firstname-attribute") {
219-
config.Source.AttributeName = c.String("firstname-attribute")
219+
config.AttributeName = c.String("firstname-attribute")
220220
}
221221
if c.IsSet("surname-attribute") {
222-
config.Source.AttributeSurname = c.String("surname-attribute")
222+
config.AttributeSurname = c.String("surname-attribute")
223223
}
224224
if c.IsSet("email-attribute") {
225-
config.Source.AttributeMail = c.String("email-attribute")
225+
config.AttributeMail = c.String("email-attribute")
226226
}
227227
if c.IsSet("attributes-in-bind") {
228-
config.Source.AttributesInBind = c.Bool("attributes-in-bind")
228+
config.AttributesInBind = c.Bool("attributes-in-bind")
229229
}
230230
if c.IsSet("public-ssh-key-attribute") {
231-
config.Source.AttributeSSHPublicKey = c.String("public-ssh-key-attribute")
231+
config.AttributeSSHPublicKey = c.String("public-ssh-key-attribute")
232232
}
233233
if c.IsSet("page-size") {
234-
config.Source.SearchPageSize = uint32(c.Uint("page-size"))
234+
config.SearchPageSize = uint32(c.Uint("page-size"))
235235
}
236236
if c.IsSet("user-filter") {
237-
config.Source.Filter = c.String("user-filter")
237+
config.Filter = c.String("user-filter")
238238
}
239239
if c.IsSet("admin-filter") {
240-
config.Source.AdminFilter = c.String("admin-filter")
240+
config.AdminFilter = c.String("admin-filter")
241241
}
242242
if c.IsSet("restricted-filter") {
243-
config.Source.RestrictedFilter = c.String("restricted-filter")
243+
config.RestrictedFilter = c.String("restricted-filter")
244244
}
245245
if c.IsSet("allow-deactivate-all") {
246-
config.Source.AllowDeactivateAll = c.Bool("allow-deactivate-all")
246+
config.AllowDeactivateAll = c.Bool("allow-deactivate-all")
247247
}
248248
return nil
249249
}
250250

251251
// findLdapSecurityProtocolByName finds security protocol by its name ignoring case.
252252
// It returns the value of the security protocol and if it was found.
253253
func findLdapSecurityProtocolByName(name string) (ldap.SecurityProtocol, bool) {
254-
for i, n := range models.SecurityProtocolNames {
254+
for i, n := range ldap.SecurityProtocolNames {
255255
if strings.EqualFold(name, n) {
256256
return i, true
257257
}
@@ -289,17 +289,15 @@ func (a *authService) addLdapBindDn(c *cli.Context) error {
289289
}
290290

291291
loginSource := &models.LoginSource{
292-
Type: models.LoginLDAP,
293-
IsActived: true, // active by default
294-
Cfg: &models.LDAPConfig{
295-
Source: &ldap.Source{
296-
Enabled: true, // always true
297-
},
292+
Type: models.LoginLDAP,
293+
IsActive: true, // active by default
294+
Cfg: &ldap.Source{
295+
Enabled: true, // always true
298296
},
299297
}
300298

301299
parseLoginSource(c, loginSource)
302-
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
300+
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
303301
return err
304302
}
305303

@@ -318,7 +316,7 @@ func (a *authService) updateLdapBindDn(c *cli.Context) error {
318316
}
319317

320318
parseLoginSource(c, loginSource)
321-
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
319+
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
322320
return err
323321
}
324322

@@ -336,17 +334,15 @@ func (a *authService) addLdapSimpleAuth(c *cli.Context) error {
336334
}
337335

338336
loginSource := &models.LoginSource{
339-
Type: models.LoginDLDAP,
340-
IsActived: true, // active by default
341-
Cfg: &models.LDAPConfig{
342-
Source: &ldap.Source{
343-
Enabled: true, // always true
344-
},
337+
Type: models.LoginDLDAP,
338+
IsActive: true, // active by default
339+
Cfg: &ldap.Source{
340+
Enabled: true, // always true
345341
},
346342
}
347343

348344
parseLoginSource(c, loginSource)
349-
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
345+
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
350346
return err
351347
}
352348

@@ -365,7 +361,7 @@ func (a *authService) updateLdapSimpleAuth(c *cli.Context) error {
365361
}
366362

367363
parseLoginSource(c, loginSource)
368-
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
364+
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
369365
return err
370366
}
371367

0 commit comments

Comments
 (0)