Skip to content

Commit

Permalink
Refactor: Move login out of models (go-gitea#16199)
Browse files Browse the repository at this point in the history 
`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 go-gitea#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 <[email protected]>
  • Loading branch information
@zeripath @AbdulrhmnGhanem
zeripath authored and AbdulrhmnGhanem committed Aug 10, 2021
1 parent eaeba5b commit 855c92e
Show file tree
Hide file tree
Showing 77 changed files with 3,785 additions and 2,933 deletions.
18 changes: 9 additions & 9 deletions cmd/admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
"text/tabwriter"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/auth/oauth2"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/log"
pwd "code.gitea.io/gitea/modules/password"
repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/services/auth/source/oauth2"

"github.com/urfave/cli"
)
Expand Down Expand Up @@ -597,7 +597,7 @@ func runRegenerateKeys(_ *cli.Context) error {
return models.RewriteAllPublicKeys()
}

func parseOAuth2Config(c *cli.Context) *models.OAuth2Config {
func parseOAuth2Config(c *cli.Context) *oauth2.Source {
var customURLMapping *oauth2.CustomURLMapping
if c.IsSet("use-custom-urls") {
customURLMapping = &oauth2.CustomURLMapping{
Expand All @@ -609,7 +609,7 @@ func parseOAuth2Config(c *cli.Context) *models.OAuth2Config {
} else {
customURLMapping = nil
}
return &models.OAuth2Config{
return &oauth2.Source{
Provider: c.String("provider"),
ClientID: c.String("key"),
ClientSecret: c.String("secret"),
Expand All @@ -625,10 +625,10 @@ func runAddOauth(c *cli.Context) error {
}

return models.CreateLoginSource(&models.LoginSource{
Type: models.LoginOAuth2,
Name: c.String("name"),
IsActived: true,
Cfg: parseOAuth2Config(c),
Type: models.LoginOAuth2,
Name: c.String("name"),
IsActive: true,
Cfg: parseOAuth2Config(c),
})
}

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

oAuth2Config := source.OAuth2()
oAuth2Config := source.Cfg.(*oauth2.Source)

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

Expand Down
76 changes: 36 additions & 40 deletions cmd/admin_auth_ldap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"strings"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/auth/ldap"
"code.gitea.io/gitea/services/auth/source/ldap"

"github.com/urfave/cli"
)
Expand Down Expand Up @@ -172,86 +172,86 @@ func parseLoginSource(c *cli.Context, loginSource *models.LoginSource) {
loginSource.Name = c.String("name")
}
if c.IsSet("not-active") {
loginSource.IsActived = !c.Bool("not-active")
loginSource.IsActive = !c.Bool("not-active")
}
if c.IsSet("synchronize-users") {
loginSource.IsSyncEnabled = c.Bool("synchronize-users")
}
}

// parseLdapConfig assigns values on config according to command line flags.
func parseLdapConfig(c *cli.Context, config *models.LDAPConfig) error {
func parseLdapConfig(c *cli.Context, config *ldap.Source) error {
if c.IsSet("name") {
config.Source.Name = c.String("name")
config.Name = c.String("name")
}
if c.IsSet("host") {
config.Source.Host = c.String("host")
config.Host = c.String("host")
}
if c.IsSet("port") {
config.Source.Port = c.Int("port")
config.Port = c.Int("port")
}
if c.IsSet("security-protocol") {
p, ok := findLdapSecurityProtocolByName(c.String("security-protocol"))
if !ok {
return fmt.Errorf("Unknown security protocol name: %s", c.String("security-protocol"))
}
config.Source.SecurityProtocol = p
config.SecurityProtocol = p
}
if c.IsSet("skip-tls-verify") {
config.Source.SkipVerify = c.Bool("skip-tls-verify")
config.SkipVerify = c.Bool("skip-tls-verify")
}
if c.IsSet("bind-dn") {
config.Source.BindDN = c.String("bind-dn")
config.BindDN = c.String("bind-dn")
}
if c.IsSet("user-dn") {
config.Source.UserDN = c.String("user-dn")
config.UserDN = c.String("user-dn")
}
if c.IsSet("bind-password") {
config.Source.BindPassword = c.String("bind-password")
config.BindPassword = c.String("bind-password")
}
if c.IsSet("user-search-base") {
config.Source.UserBase = c.String("user-search-base")
config.UserBase = c.String("user-search-base")
}
if c.IsSet("username-attribute") {
config.Source.AttributeUsername = c.String("username-attribute")
config.AttributeUsername = c.String("username-attribute")
}
if c.IsSet("firstname-attribute") {
config.Source.AttributeName = c.String("firstname-attribute")
config.AttributeName = c.String("firstname-attribute")
}
if c.IsSet("surname-attribute") {
config.Source.AttributeSurname = c.String("surname-attribute")
config.AttributeSurname = c.String("surname-attribute")
}
if c.IsSet("email-attribute") {
config.Source.AttributeMail = c.String("email-attribute")
config.AttributeMail = c.String("email-attribute")
}
if c.IsSet("attributes-in-bind") {
config.Source.AttributesInBind = c.Bool("attributes-in-bind")
config.AttributesInBind = c.Bool("attributes-in-bind")
}
if c.IsSet("public-ssh-key-attribute") {
config.Source.AttributeSSHPublicKey = c.String("public-ssh-key-attribute")
config.AttributeSSHPublicKey = c.String("public-ssh-key-attribute")
}
if c.IsSet("page-size") {
config.Source.SearchPageSize = uint32(c.Uint("page-size"))
config.SearchPageSize = uint32(c.Uint("page-size"))
}
if c.IsSet("user-filter") {
config.Source.Filter = c.String("user-filter")
config.Filter = c.String("user-filter")
}
if c.IsSet("admin-filter") {
config.Source.AdminFilter = c.String("admin-filter")
config.AdminFilter = c.String("admin-filter")
}
if c.IsSet("restricted-filter") {
config.Source.RestrictedFilter = c.String("restricted-filter")
config.RestrictedFilter = c.String("restricted-filter")
}
if c.IsSet("allow-deactivate-all") {
config.Source.AllowDeactivateAll = c.Bool("allow-deactivate-all")
config.AllowDeactivateAll = c.Bool("allow-deactivate-all")
}
return nil
}

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

loginSource := &models.LoginSource{
Type: models.LoginLDAP,
IsActived: true, // active by default
Cfg: &models.LDAPConfig{
Source: &ldap.Source{
Enabled: true, // always true
},
Type: models.LoginLDAP,
IsActive: true, // active by default
Cfg: &ldap.Source{
Enabled: true, // always true
},
}

parseLoginSource(c, loginSource)
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
return err
}

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

parseLoginSource(c, loginSource)
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
return err
}

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

loginSource := &models.LoginSource{
Type: models.LoginDLDAP,
IsActived: true, // active by default
Cfg: &models.LDAPConfig{
Source: &ldap.Source{
Enabled: true, // always true
},
Type: models.LoginDLDAP,
IsActive: true, // active by default
Cfg: &ldap.Source{
Enabled: true, // always true
},
}

parseLoginSource(c, loginSource)
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
return err
}

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

parseLoginSource(c, loginSource)
if err := parseLdapConfig(c, loginSource.LDAP()); err != nil {
if err := parseLdapConfig(c, loginSource.Cfg.(*ldap.Source)); err != nil {
return err
}

Expand Down
Loading

0 comments on commit 855c92e

Please sign in to comment.