Allow headless auth when local auth is disabled #41933
Conversation
Joerger
force-pushed
the
joerger/allow-headless-with-local-auth-disabled
branch
from
3ad0e75
to
419e6bb
Compare
Joerger
force-pushed
the
joerger/allow-headless-with-local-auth-disabled
branch
from
419e6bb
to
8448fab
Compare
lib/auth/methods.go
Outdated
| // Only grab user lock on successful headless auth to perform normal login | ||
| // checks, like whether the user is locked. Failed headless login attempts | ||
| // won't lock out the user. | ||
| if err := a.WithUserLock(ctx, req.Username, func() error { return nil }); err != nil { | ||
| log.Debugf("WithUserLock for user %q failed during headless authentication: %v", req.Username, err) | ||
| return nil, trace.Wrap(authenticateHeadlessError) | ||
| } |
There was a problem hiding this comment.
What does it mean to "grab user lock" in this context? "grab user lock" next to "perform normal login checks, like whether the user is locked" short-circuits my brain. 😶
I think the old comment has some useful context too, like the fact that failed headless login attempts usually fail due to timed-out or canceled attempts. It took me a minute to understand what's going on and both the new comment and the old one were crucial to figuring that out. It might be a good idea to keep that part of the old comment as well.
There was a problem hiding this comment.
It means checking the user's failed login attempts to check whether the user is locked by failed logins, and updating the user's login attempts upon auth failure.
We actually don't need this for headless auth as it's more similar to SSO login than local login - it usually fails from timeout / cancellation. Even if the user denies a headless request we don't want to lock out the user, so it can just be removed. Updated the comment as well.
Joerger
force-pushed
the
joerger/allow-headless-with-local-auth-disabled
branch
from
0343383
to
e995b7f
Compare
|
@fheinecke friendly ping to review |
Joerger
force-pushed
the
joerger/allow-headless-with-local-auth-disabled
branch
from
e995b7f
to
6531ede
Compare
public-teleport-github-review-bot
bot
removed the request for review
from fheinecke
|
@Joerger |
* Restore headless auth for sso user.
Joerger
force-pushed
the
joerger/allow-headless-with-local-auth-disabled
branch
from
2dcb20f
to
b142d29
Compare
changelog: Fix Headless auth for sso users, including when local auth is disabled.
Note: In addition to the issue below, this fixes a regression in headless auth for sso users caused by #41196.
Closes #37063
Closes https://github.com/gravitational/customer-sensitive-requests/issues/260