auth: coding style and glitches fixes for GitHub login source (#5340)

2 files changed, 26 insertions, 16 deletions

diff --git a/models/login_source.go b/models/login_source.go
index dd31adc1..7c6c5f49 100644
--- a/models/login_source.go
+++ b/models/login_source.go
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
+// FIXME: Put this file into its own package and separate into different files based on login sources.
 package models
 
 import (
@@ -62,7 +63,7 @@ var (
 	_ core.Conversion = &LDAPConfig{}
 	_ core.Conversion = &SMTPConfig{}
 	_ core.Conversion = &PAMConfig{}
-	_ core.Conversion = &GITHUBConfig{}
+	_ core.Conversion = &GitHubConfig{}
 )
 
 type LDAPConfig struct {
@@ -110,15 +111,15 @@ func (cfg *PAMConfig) ToDB() ([]byte, error) {
 	return jsoniter.Marshal(cfg)
 }
 
-type GITHUBConfig struct {
-	ApiEndpoint string // Github service (e.g. https://github.com/api/v1/)
+type GitHubConfig struct {
+	APIEndpoint string // GitHub service (e.g. https://api.github.com/)
 }
 
-func (cfg *GITHUBConfig) FromDB(bs []byte) error {
+func (cfg *GitHubConfig) FromDB(bs []byte) error {
 	return jsoniter.Unmarshal(bs, &cfg)
 }
 
-func (cfg *GITHUBConfig) ToDB() ([]byte, error) {
+func (cfg *GitHubConfig) ToDB() ([]byte, error) {
 	return jsoniter.Marshal(cfg)
 }
 
@@ -191,7 +192,7 @@ func (s *LoginSource) BeforeSet(colName string, val xorm.Cell) {
 		case LOGIN_PAM:
 			s.Cfg = new(PAMConfig)
 		case LOGIN_GITHUB:
-			s.Cfg = new(GITHUBConfig)
+			s.Cfg = new(GitHubConfig)
 		default:
 			panic("unrecognized login source type: " + com.ToStr(*val))
 		}
@@ -227,7 +228,7 @@ func (s *LoginSource) IsPAM() bool {
 	return s.Type == LOGIN_PAM
 }
 
-func (s *LoginSource) IsGITHUB() bool {
+func (s *LoginSource) IsGitHub() bool {
 	return s.Type == LOGIN_GITHUB
 }
 
@@ -271,8 +272,8 @@ func (s *LoginSource) PAM() *PAMConfig {
 	return s.Cfg.(*PAMConfig)
 }
 
-func (s *LoginSource) GITHUB() *GITHUBConfig {
-	return s.Cfg.(*GITHUBConfig)
+func (s *LoginSource) GitHub() *GitHubConfig {
+	return s.Cfg.(*GitHubConfig)
 }
 
 func CreateLoginSource(source *LoginSource) error {
@@ -516,7 +517,7 @@ func LoadAuthSources() {
 			loginSource.Cfg = &PAMConfig{}
 		case "github":
 			loginSource.Type = LOGIN_GITHUB
-			loginSource.Cfg = &GITHUBConfig{}
+			loginSource.Cfg = &GitHubConfig{}
 		default:
 			log.Fatal(2, "Failed to load authentication source: unknown type '%s'", authType)
 		}
@@ -755,10 +756,18 @@ func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMCon
 	}
 	return user, CreateUser(user)
 }
-func LoginViaGITHUB(user *User, login, password string, sourceID int64, cfg *GITHUBConfig, autoRegister bool) (*User, error) {
-	login_id, fullname, email, url, location, err := github.GITHUBAuth(cfg.ApiEndpoint, login, password)
+
+//________.__  __     ___ ___      ___.
+///  _____/|__|/  |_  /   |   \ __ _\_ |__
+///   \  ___|  \   __\/    ~    \  |  \ __ \
+//\    \_\  \  ||  |  \    Y    /  |  / \_\ \
+//\______  /__||__|   \___|_  /|____/|___  /
+//\/                 \/           \/
+
+func LoginViaGitHub(user *User, login, password string, sourceID int64, cfg *GitHubConfig, autoRegister bool) (*User, error) {
+	fullname, email, url, location, err := github.Authenticate(cfg.APIEndpoint, login, password)
 	if err != nil {
-		if strings.Contains(err.Error(), "Authentication failure") {
+		if strings.Contains(err.Error(), "401") {
 			return nil, errors.UserNotExist{0, login}
 		}
 		return nil, err
@@ -769,7 +778,7 @@ func LoginViaGITHUB(user *User, login, password string, sourceID int64, cfg *GIT
 	}
 	user = &User{
 		LowerName:   strings.ToLower(login),
-		Name:        login_id,
+		Name:        login,
 		FullName:    fullname,
 		Email:       email,
 		Website:     url,
@@ -782,6 +791,7 @@ func LoginViaGITHUB(user *User, login, password string, sourceID int64, cfg *GIT
 	}
 	return user, CreateUser(user)
 }
+
 func remoteUserLogin(user *User, login, password string, source *LoginSource, autoRegister bool) (*User, error) {
 	if !source.IsActived {
 		return nil, errors.LoginSourceNotActivated{source.ID}
@@ -795,7 +805,7 @@ func remoteUserLogin(user *User, login, password string, source *LoginSource, au
 	case LOGIN_PAM:
 		return LoginViaPAM(user, login, password, source.ID, source.Cfg.(*PAMConfig), autoRegister)
 	case LOGIN_GITHUB:
-		return LoginViaGITHUB(user, login, password, source.ID, source.Cfg.(*GITHUBConfig), autoRegister)
+		return LoginViaGitHub(user, login, password, source.ID, source.Cfg.(*GitHubConfig), autoRegister)
 	}
 
 	return nil, errors.InvalidLoginSourceType{source.Type}
diff --git a/models/models.go b/models/models.go
index 18931628..c4b60a0e 100644
--- a/models/models.go
+++ b/models/models.go
@@ -347,7 +347,7 @@ func ImportDatabase(dirPath string, verbose bool) (err error) {
 				case LOGIN_PAM:
 					bean.Cfg = new(PAMConfig)
 				case LOGIN_GITHUB:
-					bean.Cfg = new(GITHUBConfig)
+					bean.Cfg = new(GitHubConfig)
 				default:
 					return fmt.Errorf("unrecognized login source type:: %v", tp)
 				}