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

Fix authentication issues with code-server #2

Merged
merged 16 commits into from
Jul 20, 2020
Merged

Conversation

msquee
Copy link
Contributor

@msquee msquee commented Apr 23, 2020

Upgrade code-server to version v3.4.1 (https://github.com/cdr/code-server/releases/tag/v3.4.1)

Known bugs:

  • Multiple code-server sessions running at once may cause the browser to not set cookies properly, refreshing the Interactive Sessions is the solution for now.

Todo:

  • Update README

Further improvement ideas for future releases:

  • Download code-server binary automatically, create symlink automatically.

template/script.sh.erb Outdated Show resolved Hide resolved
@msquee msquee force-pushed the upgrade/codeserver branch from 47a928a to 0395747 Compare May 4, 2020 14:28
@msquee
Copy link
Contributor Author

msquee commented May 4, 2020

When code-server upgraded to 3.x they removed a command line option to set the base path for URL redirection after login. --base-path was removed from code-server in this commit: coder/code-server#1272

code-server operates on completely relative paths now, you don't have the option to set a base path anymore.

When code-server is ran as a batch connect application, our URL looks something like this: https://ondemand-test.osc.edu/rnode/o0808.ten.osc.edu/18513/

Relevant Files:

  1. https://github.com/cdr/code-server/blob/master/src/node/app/proxy.ts
  2. https://github.com/cdr/code-server/blob/master/src/node/app/login.ts
  3. https://github.com/cdr/code-server/blob/master/src/node/http.ts#L738
  4. https://github.com/cdr/code-server/blob/master/src/node/http.ts#L682
  5. https://github.com/cdr/code-server/blob/master/src/common/util.ts#L33
  6. https://github.com/cdr/code-server/blob/81411b2af9b088cefb460c719c0abe4bbcefd5d3/src/node/util.ts#L53
  7. https://github.com/cdr/code-server/blob/master/src/node/app/login.ts#L92
  8. https://github.com/cdr/code-server/blob/master/src/node/http.ts#L738

Problem:
We require authentication, when we send a POST request to /rnode/o0808.ten.osc.edu/18513/login/ we are redirected to /rnode/o0808.ten.osc.edu/18513/login/rnode/o0808.ten.osc.edu/18513 because code-server doesn't like anything that isn't on the root path. See comment about this from code server developers here: coder/code-server#1272 (comment)

Cause:
When you post to /login/ the redirect logic is done here: https://github.com/cdr/code-server/blob/81411b2af9b088cefb460c719c0abe4bbcefd5d3/src/node/app/login.ts#L87 and we're redirected to route.query.to which comes from /login/?to=${HERE} or if that's not set, we're redirected to / but our base path from /login/ is /rnode/o0808.ten.osc.edu/18513/login/ so we're redirected to /rnode/o0808.ten.osc.edu/18513/login/rnode/o0808.ten.osc.edu/18513/ causing our bug!

Potential Solutions:

  1. Modify the Stanford Authentication proxy to support proxying directly to a socket instead of a port https://github.com/stanford-rc/sh_ood-apps/blob/master/sh_tensorboard/template/bin/authrevproxy.py

  2. Use @johrstrom's modified authrevproxy with support for network namespaces https://github.com/OSC/bc_osc_tensorboard/blob/gtc/template/script.sh.erb this would let us start code-server with the option --auth=none and redirecting from login wouldn't be an issue anymore.
    The problem with this option is that namespaces are not enabled system wide.

  3. Create a new sidecar proxy that acts as a middleware that validates a POST request against the environment variable $PASSWORD, if they match then set a cookie with the key key https://github.com/cdr/code-server/blob/master/src/node/app/login.ts#L92 so that the user can visit the code-server running already authenticated.

code-server sets the cookie key value to a hashed SHA256 string here: https://github.com/cdr/code-server/blob/master/src/node/util.ts#L53 from the environment variable here: https://github.com/cdr/code-server/blob/master/src/node/app/login.ts#L82
$PASSWORD so we can just mimick that behavior in a sidecar proxy as long as we have access to the environment variable.

view.html.erb Outdated Show resolved Hide resolved
@msquee msquee changed the title Upgrade code-server to 3.1 Upgrade code-server to 3.2.0 May 13, 2020
@msquee msquee changed the title Upgrade code-server to 3.2.0 Upgrade code-server to version 3.3.1 May 26, 2020
* Use Stimulus.js to solve JS scoping problems when sessions are active

* Use Cookies.js for backwards compatability with older browsers
@msquee
Copy link
Contributor Author

msquee commented Jun 15, 2020

I need to find a solution to load these external scripts from inside the project root instead of relying on a CDN to host these files.

var cookiesJs = document.createElement('script')
cookiesJs.id = 'cookiesJs'
cookiesJs.src = 'https://cdn.jsdelivr.net/npm/js-cookie@2/src/js.cookie.min.js'
var stimulusJs = document.createElement('script')
stimulusJs.id = 'stimulusJs'
stimulusJs.src = 'https://unpkg.com/stimulus/dist/stimulus.umd.js'
var scripts = [cookiesJs, stimulusJs]
scripts.forEach(function (script) {
if (document.getElementById(script.id) == null) {
document.head.appendChild(script)
}
})

This isn't possible currently, ideally it would be nice to have a /static folder in the root of interactive applications that is autoloaded and exposed at a URL like /pun/sys/dashboard/batch_connect/dev/bc_osc_codeserver/static for situations like these.

template/script.sh.erb Outdated Show resolved Hide resolved
@SpontaneousDuck
Copy link

Just used the changes to view.html.erb with code-server:v3.4.1 and all worked swimmingly!

@msquee msquee force-pushed the upgrade/codeserver branch from c4cd9af to 7e01f8c Compare July 2, 2020 15:51
@msquee msquee force-pushed the upgrade/codeserver branch from 7e01f8c to c3a0b9f Compare July 2, 2020 15:53
@msquee msquee marked this pull request as ready for review July 2, 2020 15:54
@msquee msquee requested a review from ericfranz July 2, 2020 15:54
@msquee msquee changed the title Upgrade code-server to version 3.3.1 Fix authentication issues with code-server Jul 2, 2020
template/script.sh.erb Outdated Show resolved Hide resolved
template/script.sh.erb Outdated Show resolved Hide resolved
@nathanweeks
Copy link

It appears that the problem described in in #2 (comment) is again occurring with bc_osc_codeserver 0.4.0 and code-server >= 3.11.0 (but 3.10.2 works OK)

@johrstrom
Copy link
Contributor

What version of OnDemand are you on? OSC/ondemand#521 in OnDemand part of this.

@johrstrom
Copy link
Contributor

oh, it was fixed in 2.0 by the way. So 1.8 may be buggy here.

@nathanweeks
Copy link

nathanweeks commented Mar 8, 2022

OOD v2.0.12. Given its release date (2021-07-01), it seems that should have the aforementioned commits?

This issue was previously noticed by another user & mentioned in the Discourse forum: https://discourse.osc.edu/t/vscode-code-server/882/14

@vallerul
Copy link

May i know if there are updates to this issue?
I am currently using codeserver 4.1.0, and it does not seem to work.

@johrstrom
Copy link
Contributor

OOD v2.0.12. Given its release date (2021-07-01), it seems that should have the aforementioned commits?

Yes, it looks like this was fixed in 2.0.6 and beyond.

May i know if there are updates to this issue?

We have no updates as we're using version 3.9.3 and fixed it at least for that version. I haven't looked into upgrading in some time.

@nathanweeks
Copy link

We have no updates as we're using version 3.9.3 and fixed it at least for that version. I haven't looked into upgrading in some time.

It would be helpful to be able to upgrade at some point. Aside from missing features present in newer VS Code releases, VS Code 1.56.1 (provided by Code Server 3.10.2) isn't compatible with newer versions of some important extensions (e.g., the ms-python.python extension is stuck at v2021.5.926500501).

Would it be worth submitting a new issue to this repo to track the issue?

@johrstrom
Copy link
Contributor

I opened #26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't change working directory on repeated launches of app
6 participants