Skip to content

Headless Login: auth server changes#22726

Merged
Joerger merged 21 commits intomasterfrom
joerger/headless-authn-server
Mar 17, 2023
Merged

Headless Login: auth server changes#22726
Joerger merged 21 commits intomasterfrom
joerger/headless-authn-server

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Mar 7, 2023 •
edited
Loading

This PR adds the headless authentication auth server implementation for Headless Login.

@Joerger Joerger mentioned this pull request Mar 7, 2023
Closed
@Joerger Joerger marked this pull request as ready for review March 7, 2023 19:02
@github-actions github-actions Bot added rfd Request for Discussion size/lg labels Mar 7, 2023
@Joerger Joerger changed the base branch from joerger/headless-authn-backend to joerger/headless-authn-resource-watcher March 7, 2023 19:05
@Joerger Joerger marked this pull request as draft March 7, 2023 19:05
@Joerger Joerger force-pushed the joerger/headless-authn-server branch from 8f5f115 to e747686 Compare March 7, 2023 19:07
@Joerger Joerger force-pushed the joerger/headless-authn-resource-watcher branch 2 times, most recently from f920601 to dec617a Compare March 7, 2023 21:24
@Joerger Joerger mentioned this pull request Mar 7, 2023
Merged
@Joerger Joerger force-pushed the joerger/headless-authn-server branch from e747686 to c1981fa Compare March 7, 2023 23:42
@Joerger Joerger mentioned this pull request Mar 7, 2023
Merged
@espadolini espadolini removed their request for review March 10, 2023 17:39
@Joerger Joerger force-pushed the joerger/headless-authn-resource-watcher branch 2 times, most recently from f4de085 to f484b47 Compare March 13, 2023 17:23
@Joerger Joerger force-pushed the joerger/headless-authn-server branch from 9c13c5a to f4eb0bf Compare March 13, 2023 19:48
@Joerger Joerger marked this pull request as ready for review March 13, 2023 19:48
@Joerger Joerger requested a review from jakule March 13, 2023 19:48
@github-actions github-actions Bot requested review from Tener and rosstimothy March 13, 2023 19:49
Base automatically changed from joerger/headless-authn-resource-watcher to master March 13, 2023 23:54
@Joerger Joerger removed the request for review from capnspacehook March 14, 2023 00:11
rosstimothy
rosstimothy reviewed Mar 14, 2023
View reviewed changes
Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/auth_with_roles.go Outdated
Comment thread lib/auth/auth_with_roles_test.go Outdated
Comment thread lib/auth/clt.go Outdated
Comment on lines 826 to 833
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going to cause races? Do we want the timeouts for this use case to impact other users of the http client as well?

Copy link
Copy Markdown
Contributor Author

@Joerger Joerger Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately there isn't an easy way to do this since the timeout is only configurable at the client level, not the request level, and the proxy api server uses the same auth client for all requests. I will try handling this by adding a HTTPClient.WithClientParams(...roudtrip.ClientParams) to create a clone of the HTTP client with the given params applied.

Copy link
Copy Markdown
Contributor Author

@Joerger Joerger Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rosstimothy I did the above in 403b158

I moved the http client out of lib/auth/clt.go and moved some code around to improve the code quality (Adding HTTPClientConfig, CheckAndSetDefaults, etc.), but didn't make any functional changes, other than adding HTTPClient.Clone().

Comment thread lib/auth/methods.go Outdated
jakule
jakule reviewed Mar 14, 2023
View reviewed changes
Comment thread lib/services/local/headlessauthn.go Outdated
Comment thread rfd/0105-headless-authentication.md Outdated
Comment thread lib/auth/auth_with_roles_test.go Outdated
Comment thread lib/auth/methods.go Outdated
@Joerger Joerger force-pushed the joerger/headless-authn-server branch from f25b823 to 38dc560 Compare March 15, 2023 01:58
@Joerger Joerger requested review from jakule and rosstimothy March 15, 2023 02:02
@Joerger Joerger force-pushed the joerger/headless-authn-server branch from 75a75e2 to 31ed159 Compare March 16, 2023 01:05
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Mar 16, 2023

This wasn't apparent from RFD, but can we assume Teleport Node is accessed via Teleport session? If yes, then the TELEPORT_SESSION env variable should be set

Do you mean if the tsh --headless request is made from a Teleport Node? We don't want to make this assumption, headless login should work from any machine regardless of how you are connected to it.

@Joerger Joerger enabled auto-merge March 16, 2023 19:04
@Joerger Joerger disabled auto-merge March 16, 2023 19:45
@Joerger Joerger force-pushed the joerger/headless-authn-server branch from 0035329 to e27445a Compare March 17, 2023 00:49
@Joerger Joerger enabled auto-merge March 17, 2023 00:49
@Joerger Joerger added this pull request to the merge queue Mar 17, 2023
Merged via the queue into master with commit a05a0ed Mar 17, 2023
@Joerger Joerger deleted the joerger/headless-authn-server branch March 17, 2023 01:18
Joerger added a commit that referenced this pull request Mar 20, 2023
@Joerger @rosstimothy
Headless Login: auth server changes (#22726)
a43f88f 
* Add Headless Authn service.

* Add/fix 3 minute headless login timeout.

* * Prevent repeated updates to headless authentication state

* Prevent user lock out from headless authentication failure

* Delete headless authentication on failed attempts

* Add auth_with_roles test.

* Extend timeout in test to reduce flakiness.

* Fix error typo.

* Add context timeouts, remove initial GetHeadlessAuthentication call.

* Resolve comments.

* Move http client to it's own file; Add ability to clone HTTP client for per-request configuration changes.

* Fix flaky test.

* Remove shared state from test.

* Update error handling and testing for auth_with_roles.

* Fix rebase misshap.

* Fix race condition in test.

* update e ref

* Fix ctx missing.

* Extend test timeout to prevent flakiness.

* Fix issue with roundtrip.ClientParams not being applied due to roundtripper wrapping.

---------

Co-authored-by: Tim Ross <tim.ross@goteleport.com>
@Joerger Joerger mentioned this pull request Mar 20, 2023
Merged
18 tasks
Joerger added a commit that referenced this pull request Apr 3, 2023
@Joerger @rosstimothy
@jakule @hatched @zmb3
[v12] Headless Login (#23360)
2a4eba9 
* RFD 105 - Headless Authentication (#21005)

* Draft UX section.

* Complete draft.

* Minor edits.

* Address comments, polish.

* Condense headless login request into a single HTTP endpoint.

* Update security section for limited certificate permissions.

* Address doyensec comments.

* Update RFD.

* Remove certificate limitation from RFD scope; Add RFD number; smaller
edits.

* Small fixes.

* * Update auth flow to use auth.AuthenticateSSHUser endpoint instead of CreateHeadlessAuthRequest and GenerateUserCerts endpoints

 * Remove CreateHeadlessAuthRequest rpc

 * Remove token and other unneeded fields from headless authentication

* * Add resource watcher section

* Don't insert backend data without authenticaion

* Remove view headless requests page

* Update diagram

* Use the client's public key to derive a request ID.

* Add HeadlessAuthentication protobuf type and Resource implementation. (#22350)

* Add headless auth preference logic. (#22148)

* Add Headless Authn backend service. (#22553)

* Headless Login: add headless authentication resource watcher (#22699)

* Add headless authentication resource watcher.

* Handle OpInit event and Watcher errors.

* Headless Login: proxy server changes (#22734)

* * Add proper context handling to auth.AuthenticateUser.

 * Move PublicKey field to AuthenticateUserRequest where it can be used
   for actual authentication.

 * Use a simple switch statement in /webapi/ssh/certs logic to switch
   between password, otp, and eventually headless login.

* Add Headless flow to /webapi/ssh/certs login enpdoint.

* Add 3 minute callback timeout.

* Headless Login: protobuf service (#22750)

* Add Headless Authn proto server.

* Add Headless Authn proto client.

* Resolve comments.

* Headless Login: tsh implementation (#22751)

* * Implement tsh --headless

 * Implement tsh headless approve

* Add better headless authn state handling.

* Add godoc for new tsh field.

* Headless login: Mlockall (#23159)

* Use Mlockall for Headless login.

* Skip memory lock on unsupported OSs.

Resolve comments

* Headless Login: auth server changes (#22726)

* Add Headless Authn service.

* Add/fix 3 minute headless login timeout.

* * Prevent repeated updates to headless authentication state

* Prevent user lock out from headless authentication failure

* Delete headless authentication on failed attempts

* Add auth_with_roles test.

* Extend timeout in test to reduce flakiness.

* Fix error typo.

* Add context timeouts, remove initial GetHeadlessAuthentication call.

* Resolve comments.

* Move http client to it's own file; Add ability to clone HTTP client for per-request configuration changes.

* Fix flaky test.

* Remove shared state from test.

* Update error handling and testing for auth_with_roles.

* Fix rebase misshap.

* Fix race condition in test.

* update e ref

* Fix ctx missing.

* Extend test timeout to prevent flakiness.

* Fix issue with roundtrip.ClientParams not being applied due to roundtripper wrapping.

---------

Co-authored-by: Tim Ross <tim.ross@goteleport.com>

* Extend context timeouts in TestHeadlessAuthenticationWatcher tests to reduce flakiness. (#23160)

* Fix flaky test due to context deadline. (#23260)

* Fix headless login with `second_factor: on | optional` (#23271)

* Fix headless login with second_factor:on|optional.

* Update ssh/certs endpoint to only configure necessary authentication fields; clarify comments; update test to cover headless authenticaiton preference.

* Update test to cover user locking logic.

* Change generic headless error. (#23331)

* Headless SSO web endpoint and UI (#22914)

* Update UI

Update UI text

Update the code to add headless request get

Remove commented code

Added simple UI and endpoints

* Update UI
Implement reject SSO handler and UI

* Fix linter issues

* Fix more linter issues

* Fix UI tests

* Use url.JoinPath.

* center spinner on the page and animate it.

* Address code review comments

* Address code review comments

* Renamed React component

* Address PR comments

---------

Co-authored-by: joerger <bjoerger@goteleport.com>
Co-authored-by: Jeff Pihach <jeff.pihach@goteleport.com>

* Fix flaky test `TestHeadlessAuthenticationWatcher` (#23417)

* Fix race condition in test by using a helper function instead of complex channel mechanisms.

* Avoid creating new methods solely for testing; resolve other comments.

* Reuse more code; resolve other comments.

* Fix race condition that could cause a new watcher to be marked as stale before the channel is consumed; Fix minor test issues.

* Remove race condition on headless authentication expires field when (#23578)

using memory storage.

* Headless Authn: documentation (#23272)

* Add docs.

* Update docs/pages/access-controls/guides/headless-login.mdx

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Fix lint error.

* Ellaborate on how headless login differs from standard login.

* Resolve comments; Fix capitalization.

* Resolves comments.

* Add cli reference docs.

* Restructure guide; Remove scoped blocks; Update descriptions; resolve other comments.

* Make configuration options/alternatives collapsible; Fix typos.

* Fix file names, titles, and make new config details begin as closed.

* Fix hidden merge conflict.

* Add line breaks.

* Fix dead link.

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Tim Ross <tim.ross@goteleport.com>
Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com>
Co-authored-by: Jeff Pihach <jeff.pihach@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Tener Tener Tener approved these changes

@rosstimothy rosstimothy Awaiting requested review from rosstimothy

+1 more reviewer

@jakule jakule jakule approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

rfd Request for Discussion size/lg size/md

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Joerger @Tener @jakule @rosstimothy