- Notifications
You must be signed in to change notification settings - Fork907
fix(cli): replace $SESSION_TOKEN placeholder for external apps#17048
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| // strings in the URL with the actual session token. | ||
| // This is consistent behaviour with the frontend. See: site/src/modules/resources/AppLink/AppLink.tsx | ||
| func replacePlaceholderExternalSessionTokenString(client *codersdk.Client, appURL string) string { | ||
| if !strings.Contains(appURL, "$SESSION_TOKEN") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is this the only supported format? Not e.g.${SESSION_TOKEN}?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nope, it's justconst magicTokenString = "$SESSION_TOKEN";
| return appURL | ||
| } | ||
| // We will just re-use the existing session token we're already using. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this is fine for apps. For VS Code I think we generate a new one but since it’s long-lived in the editor, it makes sense to decouple them. Same doesn’t apply here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM. Thanks 👍
674f60f intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Fixes an oversight in#17032
The FE has logic to replace the string
$SESSION_TOKENwith a newly-minted session token.This adds corresponding logic to the
coder open appcommand.