Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

SharedDirectoryInfoRequest#966

Merged
ibeckermayer merged 21 commits intomasterfrom
isaiah/sd-info-request
Jul 19, 2022
Merged

SharedDirectoryInfoRequest#966
ibeckermayer merged 21 commits intomasterfrom
isaiah/sd-info-request

Conversation

@ibeckermayer
Copy link
Copy Markdown

Adds SharedDirectoryInfoRequest handling and does some tidying up in the codec test.

base branch should be changed to master once #965 is merged

requires backport to v9/v10

SharedDirectoryInfoResponse is going to be a bigger project. I'm going to take a detour to the backend for a moment to clean up the path names being sent to us (both in SharedDirectoryInfoRequests and future shared directory messages).

How to test manually

teleport

Build and run the windows-desktop-directory-sharing branch, editing this line in the Makefile to have the directory_sharing build flag:

$(BUILDDIR)/teleport: ensure-webassets bpf-bytecode rdpclient
	GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG) go build -tags "$(PAM_TAG) $(FIPS_TAG) $(BPF_TAG) $(WEBASSETS_TAG) $(RDPCLIENT_TAG) directory_sharing" -o $(BUILDDIR)/teleport $(BUILDFLAGS) ./tool/teleport

webapps

Change the value below to true and run the dev server

enableDirectorySharing: false, // note to reviewers: should be false in any PRs.

@ravicious
Copy link
Copy Markdown
Member

The code in this PR doesn't throw the "invalid message type: 13" error like #965 (#965 (comment)). However, after selecting the directory and clicking "View files" I don't see the directory being shared on the desktop. In the console I see

[TDPClient] Started sharing directory: slack-emoji

Is this something that still needs to be implemented on the backend side?


handleSharedDirectoryInfoRequest(buffer: ArrayBuffer) {
const req = this.codec.decodeSharedDirectoryInfoRequest(buffer);
this.logger.debug(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is debug a temporary logger (like we remove it once TODO is implemented)? if not, what is diff between info and debug?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In chrome dev tools, I see it as its own section "No verbose"
image

I haven't checked as to whether its a no-op in production builds and/or how it works in other browsers. I'll take a look.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@kimlisa debug here is equivalent to console.debug(), whether its displayed in the console depends on the console settings. I will add a TODO to remove it.

Comment thread packages/teleport/src/lib/tdp/codec.ts Outdated
Comment thread packages/teleport/src/lib/tdp/codec.test.ts Outdated
@ibeckermayer ibeckermayer changed the base branch from isaiah/sd-acknowledge to master July 18, 2022 23:14
@ibeckermayer
Copy link
Copy Markdown
Author

The code in this PR doesn't throw the "invalid message type: 13" error like #965 (#965 (comment)). However, after selecting the directory and clicking "View files" I don't see the directory being shared on the desktop. In the console I see

[TDPClient] Started sharing directory: slack-emoji

Is this something that still needs to be implemented on the backend side?

@ravicious correct that we won't see anything happen on the Windows Machine yet.

In the console, we should see:

[TDPClient] Started sharing directory: webapps
[TDPClient] Received SharedDirectoryInfoRequest: {"completionId":2,"directoryId":2,"path":""}

The backend is implemented, what's not implemented is SharedDirectoryInfoResponse. We're basically getting that SharedDirectoryInfoRequest and then leaving it hanging.

I will start adding a section explaining what to expect for each of these, since everything will be more or less "broken" until it suddenly isn't.

@ibeckermayer ibeckermayer requested a review from kimlisa July 18, 2022 23:41
@ibeckermayer ibeckermayer merged commit 10040dc into master Jul 19, 2022
ibeckermayer pushed a commit that referenced this pull request Jul 19, 2022
ibeckermayer pushed a commit that referenced this pull request Jul 19, 2022
ibeckermayer pushed a commit that referenced this pull request Jul 20, 2022
* Adds directory sharing flag to the ACL, protected by a config variable (#951)

* Directory sharing menu item (#952)

* `SharedDirectoryAnnounce` (#960)

* `SharedDirectoryAcknowledge` (#965)

* `SharedDirectoryInfoRequest` (#966)
ibeckermayer pushed a commit that referenced this pull request Jul 20, 2022
* Adds directory sharing flag to the ACL, protected by a config variable (#951)

* Directory sharing menu item (#952)

* `SharedDirectoryAnnounce` (#960)

* `SharedDirectoryAcknowledge` (#965)

* `SharedDirectoryInfoRequest` (#966)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants