Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor GenerateUserCerts to create App Sessions #39970

Merged
merged 22 commits into from
Apr 25, 2024

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Mar 28, 2024

Implement the changes to GenerateUserCerts described in RFD 169

#### `rpc GenerateUserCerts`

Currently, `rpc CreateAppSession` is used by direct clients to create an App
Session, retrieve it's session ID, and request local app certs linked to the
session ID. In order for direct clients to mark both the App Session cert and
local app cert as MFA verified, the user would need to perform two MFA checks
and two roundtrips.

This should be avoided by allowing clients to call `rpc GenerateUserCerts` to
generate both the local app cert and App Session with a single MFA response.
The client only needs the local app cert after all.

Backwards compatibility between v16 clients and v15 auth servers is handled through a best-effort version check to determine whether the client should create an app session separately. This replaces one roundtrip with another (Ping) but can be removed in v17.

Based off #39742

@github-actions github-actions bot requested review from greedy52 and strideynet March 28, 2024 17:54
@github-actions github-actions bot added machine-id size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Mar 28, 2024
Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch from f29af5d to ed8189a Compare March 28, 2024 18:02
Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@Joerger Joerger added no-changelog Indicates that a PR does not require a changelog entry application-access machine-id and removed machine-id labels Mar 28, 2024
@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch from ed8189a to a50d782 Compare March 28, 2024 23:35
@Joerger Joerger force-pushed the joerger/refactor-new-web-session-request-follow-up branch from 96809c9 to cd7c33f Compare March 29, 2024 00:07
@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch from a50d782 to 59e5e9c Compare March 29, 2024 00:08
@Joerger Joerger force-pushed the joerger/refactor-new-web-session-request-follow-up branch from cd7c33f to fcd67c8 Compare March 29, 2024 17:54
@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch from 59e5e9c to e9da3de Compare March 29, 2024 17:55
@Joerger Joerger requested a review from zmb3 March 29, 2024 17:59
@Joerger Joerger force-pushed the joerger/refactor-new-web-session-request-follow-up branch from fcd67c8 to 7b63d01 Compare April 1, 2024 17:56
@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch 2 times, most recently from 15cc1f9 to db5611b Compare April 1, 2024 19:41
lib/auth/sessions.go Outdated Show resolved Hide resolved
lib/auth/sessions.go Show resolved Hide resolved
lib/auth/sessions.go Show resolved Hide resolved
lib/auth/sessions.go Show resolved Hide resolved
@Joerger Joerger force-pushed the joerger/refactor-new-web-session-request-follow-up branch from 7b63d01 to 83a8afb Compare April 2, 2024 17:55
@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch from d74c3b9 to 93b7f22 Compare April 2, 2024 18:43
Base automatically changed from joerger/refactor-new-web-session-request-follow-up to master April 2, 2024 19:04
@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch 3 times, most recently from 44d68fa to 2c40e72 Compare April 4, 2024 00:44
@Joerger
Copy link
Contributor Author

Joerger commented Apr 15, 2024

@zmb3 @strideynet friendly ping to review

@Joerger Joerger force-pushed the joerger/generate-app-certs-and-session branch from 2c40e72 to 834a32e Compare April 15, 2024 17:25
Copy link
Contributor

@strideynet strideynet left a comment

Choose a reason for hiding this comment

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

Apologies - got through most of reviewing this and forgot to finish my review. Looks glad - I'm glad this is a single RPC now.

@Joerger Joerger enabled auto-merge April 16, 2024 17:54
@Joerger Joerger added this pull request to the merge queue Apr 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 23, 2024
@Joerger Joerger added this pull request to the merge queue Apr 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 23, 2024
@Joerger Joerger enabled auto-merge April 24, 2024 00:09
@Joerger Joerger added this pull request to the merge queue Apr 25, 2024
Merged via the queue into master with commit 12c7689 Apr 25, 2024
42 checks passed
@Joerger Joerger deleted the joerger/generate-app-certs-and-session branch April 25, 2024 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
application-access machine-id no-changelog Indicates that a PR does not require a changelog entry size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants