Fix prohibit login check on authorization (#6106)

* fix bug prohibit login not applied on dashboard * fix tests * fix bug user status leak * fix typo * return after render
go-gitea · Feb 19, 2019 · f5fa22a · f5fa22a
1 parent 538a26d
commit f5fa22a
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 9 deletions.
diff --git a/integrations/release_test.go b/integrations/release_test.go
@@ -112,7 +112,7 @@ func TestCreateReleasePaging(t *testing.T) {
 
 	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.12", i18n.Tr("en", "repo.release.draft"), 10)
 
-	// Check that user3 does not see draft and still see 10 latest releases
-	session2 := loginUser(t, "user3")
+	// Check that user4 does not see draft and still see 10 latest releases
+	session2 := loginUser(t, "user4")
 	checkLatestReleaseAndCount(t, session2, "/user2/repo1", "v0.0.11", i18n.Tr("en", "repo.release.stable"), 10)
 }
diff --git a/integrations/repo_test.go b/integrations/repo_test.go
@@ -104,7 +104,7 @@ func TestViewRepoWithSymlinks(t *testing.T) {
 func TestViewAsRepoAdmin(t *testing.T) {
 	for user, expectedNoDescription := range map[string]bool{
 		"user2": true,
-		"user3": false,
+		"user4": false,
 	} {
 		prepareTestEnv(t)
 

diff --git a/models/error.go b/models/error.go
@@ -90,6 +90,38 @@ func (err ErrUserNotExist) Error() string {
 	return fmt.Sprintf("user does not exist [uid: %d, name: %s, keyid: %d]", err.UID, err.Name, err.KeyID)
 }
 
+// ErrUserProhibitLogin represents a "ErrUserProhibitLogin" kind of error.
+type ErrUserProhibitLogin struct {
+	UID  int64
+	Name string
+}
+
+// IsErrUserProhibitLogin checks if an error is a ErrUserProhibitLogin
+func IsErrUserProhibitLogin(err error) bool {
+	_, ok := err.(ErrUserProhibitLogin)
+	return ok
+}
+
+func (err ErrUserProhibitLogin) Error() string {
+	return fmt.Sprintf("user is not allowed login [uid: %d, name: %s]", err.UID, err.Name)
+}
+
+// ErrUserInactive represents a "ErrUserInactive" kind of error.
+type ErrUserInactive struct {
+	UID  int64
+	Name string
+}
+
+// IsErrUserInactive checks if an error is a ErrUserInactive
+func IsErrUserInactive(err error) bool {
+	_, ok := err.(ErrUserInactive)
+	return ok
+}
+
+func (err ErrUserInactive) Error() string {
+	return fmt.Sprintf("user is inactive [uid: %d, name: %s]", err.UID, err.Name)
+}
+
 // ErrEmailAlreadyUsed represents a "EmailAlreadyUsed" kind of error.
 type ErrEmailAlreadyUsed struct {
 	Email string

diff --git a/models/login_source.go b/models/login_source.go
@@ -600,16 +600,29 @@ func ExternalUserLogin(user *User, login, password string, source *LoginSource,
 		return nil, ErrLoginSourceNotActived
 	}
 
+	var err error
 	switch source.Type {
 	case LoginLDAP, LoginDLDAP:
-		return LoginViaLDAP(user, login, password, source, autoRegister)
+		user, err = LoginViaLDAP(user, login, password, source, autoRegister)
 	case LoginSMTP:
-		return LoginViaSMTP(user, login, password, source.ID, source.Cfg.(*SMTPConfig), autoRegister)
+		user, err = LoginViaSMTP(user, login, password, source.ID, source.Cfg.(*SMTPConfig), autoRegister)
 	case LoginPAM:
-		return LoginViaPAM(user, login, password, source.ID, source.Cfg.(*PAMConfig), autoRegister)
+		user, err = LoginViaPAM(user, login, password, source.ID, source.Cfg.(*PAMConfig), autoRegister)
+	default:
+		return nil, ErrUnsupportedLoginType
+	}
+
+	if err != nil {
+		return nil, err
+	}
+
+	if !user.IsActive {
+		return nil, ErrUserInactive{user.ID, user.Name}
+	} else if user.ProhibitLogin {
+		return nil, ErrUserProhibitLogin{user.ID, user.Name}
 	}
 
-	return nil, ErrUnsupportedLoginType
+	return user, nil
 }
 
 // UserSignIn validates user name and password.
@@ -645,6 +658,12 @@ func UserSignIn(username, password string) (*User, error) {
 		switch user.LoginType {
 		case LoginNoType, LoginPlain, LoginOAuth2:
 			if user.IsPasswordSet() && user.ValidatePassword(password) {
+				if !user.IsActive {
+					return nil, ErrUserInactive{user.ID, user.Name}
+				} else if user.ProhibitLogin {
+					return nil, ErrUserProhibitLogin{user.ID, user.Name}
+				}
+
 				return user, nil
 			}
 

diff --git a/modules/context/auth.go b/modules/context/auth.go
@@ -8,6 +8,7 @@ import (
 	"net/url"
 
 	"code.gitea.io/gitea/modules/auth"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"github.com/go-macaron/csrf"
 	macaron "gopkg.in/macaron.v1"
@@ -32,8 +33,12 @@ func Toggle(options *ToggleOptions) macaron.Handler {
 
 		// Check prohibit login users.
 		if ctx.IsSigned {
-
-			if ctx.User.ProhibitLogin {
+			if !ctx.User.IsActive && setting.Service.RegisterEmailConfirm {
+				ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
+				ctx.HTML(200, "user/auth/activate")
+				return
+			} else if !ctx.User.IsActive || ctx.User.ProhibitLogin {
+				log.Info("Failed authentication attempt for %s from %s", ctx.User.Name, ctx.RemoteAddr())
 				ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
 				ctx.HTML(200, "user/auth/prohibit_login")
 				return

diff --git a/routers/home.go b/routers/home.go
@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/search"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
@@ -39,6 +40,10 @@ func Home(ctx *context.Context) {
 		if !ctx.User.IsActive && setting.Service.RegisterEmailConfirm {
 			ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
 			ctx.HTML(200, user.TplActivate)
+		} else if !ctx.User.IsActive || ctx.User.ProhibitLogin {
+			log.Info("Failed authentication attempt for %s from %s", ctx.User.Name, ctx.RemoteAddr())
+			ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
+			ctx.HTML(200, "user/auth/prohibit_login")
 		} else {
 			user.Dashboard(ctx)
 		}

diff --git a/routers/user/auth.go b/routers/user/auth.go
@@ -161,6 +161,19 @@ func SignInPost(ctx *context.Context, form auth.SignInForm) {
 		} else if models.IsErrEmailAlreadyUsed(err) {
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSignIn, &form)
 			log.Info("Failed authentication attempt for %s from %s", form.UserName, ctx.RemoteAddr())
+		} else if models.IsErrUserProhibitLogin(err) {
+			log.Info("Failed authentication attempt for %s from %s", form.UserName, ctx.RemoteAddr())
+			ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
+			ctx.HTML(200, "user/auth/prohibit_login")
+		} else if models.IsErrUserInactive(err) {
+			if setting.Service.RegisterEmailConfirm {
+				ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
+				ctx.HTML(200, TplActivate)
+			} else {
+				log.Info("Failed authentication attempt for %s from %s", form.UserName, ctx.RemoteAddr())
+				ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
+				ctx.HTML(200, "user/auth/prohibit_login")
+			}
 		} else {
 			ctx.ServerError("UserSignIn", err)
 		}