Skip to content

Commit

Permalink
Fail earlier if user is not allowed to log in
Browse files Browse the repository at this point in the history
We want the user to perform the OIDC authentication for auditing and
logging purposes, but we don't need them to choose a local password
before we tell them that they are not allowed to log in.
  • Loading branch information
adombeck committed Jan 28, 2025
1 parent a0f3051 commit 7296ada
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 16 deletions.
19 changes: 6 additions & 13 deletions internal/broker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,11 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, session *session, au
return AuthDenied, errorMessageForDisplay(err, "could not fetch user info")
}

if !b.userNameIsAllowed(authInfo.UserInfo.Name) {
log.Warningf(context.Background(), "User %q is not in the list of allowed users", authInfo.UserInfo.Name)
return AuthDenied, errorMessage{Message: "permission denied"}
}

session.authInfo["auth_info"] = authInfo
return AuthNext, nil

Expand Down Expand Up @@ -656,19 +661,7 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, session *session, au

// userNameIsAllowed checks whether the user's username is allowed to access the machine.
func (b *Broker) userNameIsAllowed(userName string) bool {
normalizedUsername := b.provider.NormalizeUsername(userName)
// The user is allowed to log in if:
// - ALL users are allowed
// - the user's name is in the list of allowed_users
// - the user is the owner of the machine and OWNER is in the allowed_users list
if b.cfg.userConfig.allUsersAllowed {
return true
}
if _, ok := b.cfg.userConfig.allowedUsers[normalizedUsername]; ok {
return true
}

return b.cfg.isOwnerAllowed(normalizedUsername)
return b.cfg.userNameIsAllowed(b.provider.NormalizeUsername(userName))
}

func (b *Broker) startAuthenticate(sessionID string) (context.Context, error) {
Expand Down
27 changes: 24 additions & 3 deletions internal/broker/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,32 @@ func parseConfigFile(cfgPath string, p provider) (userConfig, error) {
return cfg, nil
}

func (uc *userConfig) isOwnerAllowed(userName string) bool {
func (uc *userConfig) userNameIsAllowed(userName string) bool {
uc.ownerMutex.RLock()
defer uc.ownerMutex.RUnlock()

return uc.ownerAllowed && uc.owner == userName
// The user is allowed to log in if:
// - ALL users are allowed
// - the user's name is in the list of allowed_users
// - OWNER is in the allowed_users list and the user is the owner of the machine
// - The user will be registered as the owner
if uc.allUsersAllowed {
return true
}
if _, ok := uc.allowedUsers[userName]; ok {
return true
}
if uc.ownerAllowed && uc.owner == userName {
return true
}

return uc.shouldRegisterOwner()
}

// shouldRegisterOwner returns true if the first user to log in should be registered as the owner.
// Only call this with the ownerMutex locked.
func (uc *userConfig) shouldRegisterOwner() bool {
return uc.ownerAllowed && uc.firstUserBecomesOwner && uc.owner == ""
}

func (uc *userConfig) registerOwner(cfgPath, userName string) error {
Expand All @@ -200,7 +221,7 @@ func (uc *userConfig) registerOwner(cfgPath, userName string) error {
uc.ownerMutex.Lock()
defer uc.ownerMutex.Unlock()

if shouldRegister := uc.ownerAllowed && uc.firstUserBecomesOwner && uc.owner == ""; !shouldRegister {
if !uc.shouldRegisterOwner() {
return nil
}

Expand Down

0 comments on commit 7296ada

Please sign in to comment.