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

Enable secret storage #6450

Merged
merged 3 commits into from
Sep 26, 2023
Merged

Enable secret storage #6450

merged 3 commits into from
Sep 26, 2023

Conversation

code-asher
Copy link
Member

@code-asher code-asher commented Sep 26, 2023

It seems that at some point the secret storage went browser-only, but the browser only persists storage if it gets a key from the server to do encryption. So this implements that endpoint. Normally the process is kicked off by the presence of a cookie that contains the endpoint but I patched the endpoint in directly because making cookies work with a base path is a bit annoying since they cannot be relative (for the login cookie we send the full path from the client to the server to account for that).

Additionally, I tested --github-auth and it works without our patch so I removed it.

Fixes #6408
Fixes #5072
Fixes #6395

@code-asher code-asher requested a review from a team as a code owner September 26, 2023 00:08
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #6450 (55f8bab) into main (5428442) will decrease coverage by 0.57%.
Report is 2 commits behind head on main.
The diff coverage is 20.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6450      +/-   ##
==========================================
- Coverage   73.70%   73.13%   -0.57%     
==========================================
  Files          31       31              
  Lines        1886     1906      +20     
  Branches      407      409       +2     
==========================================
+ Hits         1390     1394       +4     
- Misses        419      433      +14     
- Partials       77       79       +2     
Files Coverage Δ
src/node/routes/vscode.ts 33.33% <20.00%> (-3.38%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 468cf5c...55f8bab. Read the comment docs.

@code-asher code-asher merged commit a1131fa into coder:main Sep 26, 2023
9 of 10 checks passed
@code-asher code-asher deleted the secret-storage branch September 26, 2023 16:35
@benz0li
Copy link
Contributor

benz0li commented Sep 27, 2023

@code-asher Thank you for investigating. This is great news.

Could you please release v4.17.1 incorporating this change?

Thank you.

@code-asher
Copy link
Member Author

Definitely will, there is one extension that still seems to be having trouble authenticating so I am trying to figure that out, but once I do I will put out a patch release.

@code-asher
Copy link
Member Author

Actually I need to test the build changes for arm anyway so I am going to make an RC for now to do that.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants