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.

Sign up for GitHub

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

Jump to bottom

Emit app.session.start event on tsh app login and tsh proxy app #43636

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Emit app.session.start event on tsh app login and tsh proxy app #43636

merged 1 commit into from
Jul 29, 2024

Conversation

rudream
Copy link
Contributor

@rudream rudream commented Jun 28, 2024 •
edited
Loading

Purpose

Resolves #37054

This PR emits the app.session.start audit event when launching an app from tsh, previously, the event would only be emitted when launching it from the WebUI.

changelog: Emit app.session.start event when using tsh app login

@rudream rudream force-pushed the yassine/emit-app-session-start branch from 3b7e054 to e5fc2b1 Compare July 12, 2024 08:48
@rudream rudream marked this pull request as ready for review July 12, 2024 09:00
@github-actions github-actions bot added size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Jul 12, 2024
@github-actions GitHub Actions
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.

@rudream rudream requested a review from strideynet July 12, 2024 09:22
marcoandredinis
marcoandredinis reviewed Jul 12, 2024
View reviewed changes
Copy link
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

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

Changes look good 👍
Tested it and was able to see the events for both commands.

Can you please add some test coverage?
I spotted a typo in one of the fields and a test could have caught that.

api/proto/teleport/legacy/client/proto/authservice.proto Outdated Show resolved Hide resolved
lib/auth/auth_with_roles.go Outdated Show resolved Hide resolved
lib/auth/sessions.go Show resolved Hide resolved
strideynet
strideynet reviewed Jul 19, 2024
View reviewed changes
lib/auth/sessions.go Show resolved Hide resolved
lib/web/apps.go Show resolved Hide resolved
zmb3
zmb3 approved these changes Jul 22, 2024
View reviewed changes
Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

LGTM with a few comments.

api/proto/teleport/legacy/client/proto/authservice.proto Outdated Show resolved Hide resolved
integrations/operator/crdgen/testdata/protofiles/teleport/legacy/client/proto/authservice.proto Outdated Show resolved Hide resolved
lib/auth/sessions.go Show resolved Hide resolved
lib/auth/sessions.go Outdated Show resolved Hide resolved
@rudream rudream force-pushed the yassine/emit-app-session-start branch from e5fc2b1 to 21f700b Compare July 25, 2024 07:55
@rudream rudream requested a review from marcoandredinis July 25, 2024 07:56
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from hugoShaka July 25, 2024 08:18
@rudream rudream force-pushed the yassine/emit-app-session-start branch 2 times, most recently from 2a804fd to a624a5f Compare July 25, 2024 12:14
@rudream rudream enabled auto-merge July 25, 2024 12:15
@rudream rudream force-pushed the yassine/emit-app-session-start branch 5 times, most recently from 608ad5a to 8af275c Compare July 26, 2024 21:52
@rudream rudream force-pushed the yassine/emit-app-session-start branch from 8af275c to bde93c9 Compare July 26, 2024 22:40
@zmb3
Copy link
Collaborator

zmb3 commented Jul 29, 2024

/excludeflake TestAppCommands

@rudream rudream added this pull request to the merge queue Jul 29, 2024
Merged via the queue into master with commit 50b60de Jul 29, 2024
41 checks passed
@rudream rudream deleted the yassine/emit-app-session-start branch July 29, 2024 15:31
@public-teleport-github-review-bot
Copy link

@rudream See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed

rudream added a commit that referenced this pull request Jul 30, 2024
@rudream
emit app.session.start when creating an app session (#43636)
cf46f43
@rudream rudream mentioned this pull request Jul 30, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 30, 2024
@rudream
emit app.session.start when creating an app session (#43636) (#44792)
f55f55d
@zmb3 zmb3 mentioned this pull request Aug 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strideynet strideynet strideynet left review comments

@zmb3 zmb3 zmb3 approved these changes

@marcoandredinis marcoandredinis marcoandredinis approved these changes

Assignees
No one assigned
Labels
backport/branch/v15 backport/branch/v16 size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tsh app ... does not emit app.session.start
4 participants
@rudream @zmb3 @marcoandredinis @strideynet