wizrad: add validation to ssh key field (HMS-5349) #2764
Conversation
|
/jira-epic HMS-4181 |
schutzbot
changed the title
mgold1234
force-pushed
the
add_ssh_valid
branch
from
9171d04 to
512a04e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## main #2764 +/- ##
==========================================
+ Coverage 84.75% 84.76% +0.01%
==========================================
Files 187 187
Lines 21270 21285 +15
Branches 2088 2095 +7
==========================================
+ Hits 18027 18042 +15
Misses 3221 3221
Partials 22 22
Continue to review full report in Codecov by Sentry.
|
mgold1234
force-pushed
the
add_ssh_valid
branch
7 times, most recently
from
1018e74 to
f10fad2
Compare
| export const isSshKeyValid = (sshKey: string) => { | ||
| const isLengthValid = sshKey !== undefined && sshKey.length >= 2; | ||
| const isPatternValid = | ||
| /^(ssh-(rsa|dss|ed25519)|ecdsa-sha2-nistp(256|384|521)) \S+/.test(sshKey); |
There was a problem hiding this comment.
Do you have any source for the regex? If so, could you please add it to a comment so we can reference it if needed?
I wonder if we should try do to a full match instead of a partial one, should be safer that way. We would need to add a few more things to allow for the label part of the key. So maybe something like:
^(ssh-(rsa|dss|ed25519)|ecdsa-sha2-nistp(256|384|521))\s+[A-Za-z0-9+\/=]+\s+\S+$
What do you think?
There was a problem hiding this comment.
I tried this - 'ssh-rsa d' value with your suggestion and it isnt valid.
I took this pattern from 'edge' , they also check the ssh key validation
There was a problem hiding this comment.
Hmm, comment might be optional doesn't it 🤔 this should work then
^(ssh-(rsa|dss|ed25519)|ecdsa-sha2-nistp(256|384|521))\s+[A-Za-z0-9+\/=]+(\s+\S+)?$
There was a problem hiding this comment.
do you think about this comment?
// 1. Key types: ssh-rsa, ssh-dss, ssh-ed25519, or ecdsa-sha2-nistp(256|384|521).
// 2. Base64-encoded key material.
// 3. Optional comment at the end.
src/test/Components/CreateImageWizard/steps/Users/Users.test.tsx
Outdated
Show resolved
Hide resolved
src/test/Components/CreateImageWizard/steps/Users/Users.test.tsx
Outdated
Show resolved
Hide resolved
mgold1234
force-pushed
the
add_ssh_valid
branch
7 times, most recently
from
b7fe00e to
a2d67d8
Compare
mgold1234
force-pushed
the
add_ssh_valid
branch
7 times, most recently
from
663b854 to
9063e9f
Compare
src/test/Components/CreateImageWizard/steps/Users/Users.test.tsx
Outdated
Show resolved
Hide resolved
mgold1234
force-pushed
the
add_ssh_valid
branch
from
9063e9f to
b2fa4d5
Compare
|
Noticed on more thing, very nit picky 😅 the two describe blocks 'User request generated correctly' and 'Users edit mode' are nested inside 'Step Users'. In other unit tests they're the same level as the 'Step Users'. Could you move those please?
|
mgold1234
force-pushed
the
add_ssh_valid
branch
3 times, most recently
from
2902012 to
0b5fdf7
Compare
|
Just a heads up, #2772 got merged yesterday so lint autofix should work again without adding unwanted changes :) |
mgold1234
force-pushed
the
add_ssh_valid
branch
from
0b5fdf7 to
0f86336
Compare
|
Did you close the PR on purpose? |
|
I meant to delete the brach and close it by mistake |
this commit add validation to ssh key field
JIRA: HMS-5349