This repository has been archived by the owner on Feb 8, 2024. It is now read-only.
Lisa/minor fixes #123
Merged
Lisa/minor fixes #123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kimlisa
commented
Jul 14, 2020
packages/teleport/src/console/components/DocumentSsh/useSshSession.ts
Outdated
Show resolved
Hide resolved
kimlisa
commented
Jul 14, 2020
alex-kovoy
reviewed
Jul 14, 2020
| const check = value => { | ||
| const match = SSH_STR_REGEX.exec(value); | ||
| return match !== null; | ||
| const hasWhiteSpace = /\s/.test(value); |
There was a problem hiding this comment.
why checking it separately from main regex?
| @@ -63,10 +63,14 @@ export default function FieldInputSsh({ | |||
| ); | |||
| } | |||
|
|
|||
| const SSH_STR_REGEX = /(^(\w+-?\w+)+@(\S+)$)/; | |||
| // SSH_STR_REGEX is a modified regex from teleport's lib/sshutils/scp/scp.go. | |||
There was a problem hiding this comment.
This scp information here is not relevant (confusing) and this regex can be greater simplified by checking for just 2 conditions: no spaces and no empty values. Doing data validation in here is more of a cosmetic requirement rather than a security concern. If anything, an error, return by the server, will be shown to the user in the terminal tab.
| const url = cfg.getSshSessionRoute({ sid, clusterId }); | ||
| ctx.updateSshDocument(docId, { | ||
| title: `${login}@${hostname}`, | ||
| // DELETE IN 5.0.0 |
There was a problem hiding this comment.
Are we sure that this is an upgrade issue similarly to clusterId and not one of the valid use cases (when hostname is not-specified) ? If not, then lets remove this comment.
There was a problem hiding this comment.
reverted, handled by webapi server: gravitational/teleport#4027
kimlisa
force-pushed
the
lisa/minor-fixes
branch
from
355e3bf to
c9acdc2
Compare
* use hostname instead of relying on serverId as fallback (handled in backend) * simplified quicklaunch regex to just check for white spaces * encode URL when finding documents by url to handle special characters
kimlisa
force-pushed
the
lisa/minor-fixes
branch
from
c9acdc2 to
615efaa
Compare
alex-kovoy
approved these changes
Jul 15, 2020
kimlisa
force-pushed
the
lisa/minor-fixes
branch
from
f58d60e to
4a06b68
Compare
russjones
pushed a commit
that referenced
this pull request
Jul 20, 2021
russjones
pushed a commit
that referenced
this pull request
Jul 20, 2021
zmb3
added a commit
to gravitational/teleport
that referenced
this pull request
Feb 8, 2022
The script for updating webassets uses the commit message from
webapps as the commit message for the PR to teleport.
This commit message is almost always a merged PR, which has the format:
do some awesome thing (#123)
Where '#123' is the number of the **webapps** PR that was merged.
The problem with this is, when the teleport PR is created, it interprets
the #123 as the number of a **teleport** PR. And since the Teleport repo
has a lot more issues/PRs than webapps, Github ends up linking to an old
and completely unrelated PR.
Fix this by replacing (#123) with (gravitational/webapps#123), which
Github correctly renders as a link to the webapps PR in question.
zmb3
added a commit
to gravitational/teleport
that referenced
this pull request
Feb 8, 2022
The script for updating webassets uses the commit message from
webapps as the commit message for the PR to teleport.
This commit message is almost always a merged PR, which has the format:
do some awesome thing (#123)
Where '#123' is the number of the **webapps** PR that was merged.
The problem with this is, when the teleport PR is created, it interprets
the #123 as the number of a **teleport** PR. And since the Teleport repo
has a lot more issues/PRs than webapps, Github ends up linking to an old
and completely unrelated PR.
Fix this by replacing (#123) with (gravitational/webapps#123), which
Github correctly renders as a link to the webapps PR in question.
zmb3
added a commit
to gravitational/teleport
that referenced
this pull request
Feb 8, 2022
The script for updating webassets uses the commit message from
webapps as the commit message for the PR to teleport.
This commit message is almost always a merged PR, which has the format:
do some awesome thing (#123)
Where '#123' is the number of the **webapps** PR that was merged.
The problem with this is, when the teleport PR is created, it interprets
the #123 as the number of a **teleport** PR. And since the Teleport repo
has a lot more issues/PRs than webapps, Github ends up linking to an old
and completely unrelated PR.
Fix this by replacing (#123) with (gravitational/webapps#123), which
Github correctly renders as a link to the webapps PR in question.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes gravitational/teleport#4008
fixes gravitational/teleport#4013
fixes gravitational/teleport#4044
Description
Dependent PR
gravitational/teleport#4027