Skip to content

Commit a4a3d01

Browse files
fix: hibernate and unhibernate rbac issue (#6641)
* fixed user status function inauthroser * fix: correct function name for user retrieval in UserService
1 parent b64a400 commit a4a3d01

File tree

7 files changed

+57
-12
lines changed

7 files changed

+57
-12
lines changed

App.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ type App struct {
5656
server *http.Server
5757
db *pg.DB
5858
posthogClient *posthogTelemetry.PosthogClient
59+
userService user.UserService
5960
// eventProcessor.CentralEventProcessor is used to register event processors
6061
centralEventProcessor *eventProcessor.CentralEventProcessor // do not remove this.
6162
// used for local dev only
@@ -79,6 +80,7 @@ func NewApp(router *router.MuxRouter,
7980
pubSubClient *pubsub.PubSubClientServiceImpl,
8081
workflowEventProcessorImpl *in.WorkflowEventProcessorImpl,
8182
enforcerV2 *casbinv2.SyncedEnforcer,
83+
userService user.UserService,
8284
) *App {
8385
//check argo connection
8486
//todo - check argo-cd version on acd integration installation
@@ -97,6 +99,7 @@ func NewApp(router *router.MuxRouter,
9799
centralEventProcessor: centralEventProcessor,
98100
pubSubClient: pubSubClient,
99101
workflowEventProcessorImpl: workflowEventProcessorImpl,
102+
userService: userService,
100103
}
101104
return app
102105
}
@@ -112,7 +115,7 @@ func (app *App) Start() {
112115
app.MuxRouter.Init()
113116
//authEnforcer := casbin2.Create()
114117

115-
server := &http.Server{Addr: fmt.Sprintf(":%d", port), Handler: authMiddleware.Authorizer(app.sessionManager2, user.WhitelistChecker, nil)(app.MuxRouter.Router)}
118+
server := &http.Server{Addr: fmt.Sprintf(":%d", port), Handler: authMiddleware.Authorizer(app.sessionManager2, user.WhitelistChecker, app.userService.CheckUserStatusAndUpdateLoginAudit)(app.MuxRouter.Router)}
116119
app.MuxRouter.Router.Use(app.loggingMiddleware.LoggingMiddleware)
117120
app.MuxRouter.Router.Use(middleware.PrometheusMiddleware)
118121
app.MuxRouter.Router.Use(middlewares.Recovery)

cmd/external-app/externalApp.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,24 @@ type App struct {
4242
server *http.Server
4343
telemetry telemetry.TelemetryEventClient
4444
posthogClient *posthogTelemetry.PosthogClient
45+
userService user.UserService
4546
}
4647

4748
func NewApp(db *pg.DB,
4849
sessionManager *authMiddleware.SessionManager,
4950
MuxRouter *MuxRouter,
5051
telemetry telemetry.TelemetryEventClient,
5152
posthogClient *posthogTelemetry.PosthogClient,
52-
Logger *zap.SugaredLogger) *App {
53+
Logger *zap.SugaredLogger,
54+
userService user.UserService) *App {
5355
return &App{
5456
db: db,
5557
sessionManager: sessionManager,
5658
MuxRouter: MuxRouter,
5759
Logger: Logger,
5860
telemetry: telemetry,
5961
posthogClient: posthogClient,
62+
userService: userService,
6063
}
6164
}
6265
func (app *App) Start() {
@@ -73,7 +76,7 @@ func (app *App) Start() {
7376
if err != nil {
7477
app.Logger.Warnw("telemetry installation success event failed", "err", err)
7578
}
76-
server := &http.Server{Addr: fmt.Sprintf(":%d", port), Handler: authMiddleware.Authorizer(app.sessionManager, user.WhitelistChecker, nil)(app.MuxRouter.Router)}
79+
server := &http.Server{Addr: fmt.Sprintf(":%d", port), Handler: authMiddleware.Authorizer(app.sessionManager, user.WhitelistChecker, app.userService.CheckUserStatusAndUpdateLoginAudit)(app.MuxRouter.Router)}
7780
app.MuxRouter.Router.Use(middleware.PrometheusMiddleware)
7881
app.MuxRouter.Router.Use(middlewares.Recovery)
7982
app.server = server

cmd/external-app/wire_gen.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/auth/user/UserSelfRegistrationService.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,20 +126,15 @@ func (impl *UserSelfRegistrationServiceImpl) CheckAndCreateUserIfConfigured(clai
126126
emailId = sub
127127
}
128128
exists := impl.userService.UserExists(emailId)
129-
var id int32
130129
if !exists {
131130
impl.logger.Infow("self registering user, ", "email", emailId)
132131
user, err := impl.SelfRegister(emailId)
133132
if err != nil {
134133
impl.logger.Errorw("error while register user", "error", err)
135134
} else if user != nil && user.Id > 0 {
136-
id = user.Id
137135
exists = true
138136
}
139137
}
140-
if exists {
141-
impl.userService.SaveLoginAudit(emailId, "localhost", id)
142-
}
143138
impl.logger.Infow("user status", "email", emailId, "status", exists)
144139
return exists
145140
}

pkg/auth/user/UserService.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ type UserService interface {
8686
GetRoleFiltersByUserRoleGroups(userRoleGroups []userBean.UserRoleGroup) ([]userBean.RoleFilter, error)
8787
SaveLoginAudit(emailId, clientIp string, id int32)
8888
CheckIfTokenIsValid(email string, version string) error
89+
CheckUserStatusAndUpdateLoginAudit(token string) (bool, int32, string, error)
8990
}
9091

9192
type UserServiceImpl struct {
@@ -1826,3 +1827,34 @@ func (impl *UserServiceImpl) GetUserBasicDataByEmailId(emailId string) (*userBea
18261827
}
18271828
return response, nil
18281829
}
1830+
1831+
func (impl *UserServiceImpl) CheckUserStatusAndUpdateLoginAudit(token string) (bool, int32, string, error) {
1832+
emailId, version, err := impl.GetEmailAndVersionFromToken(token)
1833+
if err != nil {
1834+
impl.logger.Error("unable to fetch user by token")
1835+
err = &util.ApiError{HttpStatusCode: 401, UserMessage: "Invalid User", InternalMessage: "unable to fetch user by token"}
1836+
return false, 0, "", err
1837+
}
1838+
userId, isInactive, err := impl.getUserWithTimeoutWindowConfiguration(emailId)
1839+
if err != nil {
1840+
err = &util.ApiError{HttpStatusCode: 401, UserMessage: "Invalid User", InternalMessage: err.Error()}
1841+
impl.logger.Errorw("unable to fetch user by email, %s", token)
1842+
return isInactive, userId, "", err
1843+
}
1844+
//if user is inactive, no need to store audit log
1845+
if !isInactive {
1846+
impl.SaveLoginAudit(emailId, "localhost", userId)
1847+
}
1848+
// checking length of version, to ensure backward compatibility as earlier we did not
1849+
// have version for api-tokens
1850+
// therefore, for tokens without version we will skip the below part
1851+
if strings.HasPrefix(emailId, userBean.API_TOKEN_USER_EMAIL_PREFIX) && len(version) > 0 {
1852+
err := impl.CheckIfTokenIsValid(emailId, version)
1853+
if err != nil {
1854+
impl.logger.Errorw("token is not valid", "error", err, "token", token)
1855+
return isInactive, userId, "", err
1856+
}
1857+
}
1858+
1859+
return isInactive, userId, emailId, nil
1860+
}

pkg/auth/user/UserService_ent.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
userBean "github.com/devtron-labs/devtron/pkg/auth/user/bean"
99
userrepo "github.com/devtron-labs/devtron/pkg/auth/user/repository"
1010
"github.com/go-pg/pg"
11+
"github.com/juju/errors"
1112
"strings"
1213
)
1314

@@ -204,3 +205,14 @@ func (impl *UserServiceImpl) checkValidationAndPerformOperationsForUpdate(token
204205
}
205206
return false, isUserSuperAdmin, nil
206207
}
208+
209+
func (impl *UserServiceImpl) getUserWithTimeoutWindowConfiguration(emailId string) (int32, bool, error) {
210+
user, err := impl.userRepository.FetchActiveUserByEmail(emailId)
211+
if err != nil {
212+
impl.logger.Errorw("error while fetching user from db", "error", err)
213+
errMsg := fmt.Sprintf("failed to fetch user by email id, err: %s", err.Error())
214+
return 0, false, errors.New(errMsg)
215+
}
216+
// here false is always returned to match signature of authoriser function.
217+
return user.Id, false, nil
218+
}

wire_gen.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)