feat: manage ssh keys for accounts #187
Conversation
…turn encoded content instead default content
… to accountQueryRepo
…mdRepo to infra tests
| contentBytes := []byte(content.String()) | ||
| publicKey, _, _, _, err := ssh.ParseAuthorizedKey(contentBytes) | ||
| if err != nil { | ||
| return secureAccessPublicKey, errors.New("SecureAccessPublicKeyParseError") |
There was a problem hiding this comment.
You gotta have an unit test on this situation since there is some logic here that may break. Does the FingerprintSHA256 works for both RSA and ED25519?
There was a problem hiding this comment.
Yes, it validates both types of keys. I tested both!
src/domain/useCase/deleteAccount.go
Outdated
| if err != nil { | ||
| return errors.New("AccountNotFound") | ||
| return err |
There was a problem hiding this comment.
We don't usually return the infra error directly to the user. I guess you forgot the slog.Error here? The previous "AccountNotFound" was the correct to return to the user.
| readRequestDto := dto.ReadSecureAccessPublicKeysRequest{ | ||
| SecureAccessPublicKeyId: &deleteDto.Id, | ||
| } | ||
| keyToDelete, err := accountQueryRepo.ReadFirstSecureAccessPublicKey( |
src/domain/useCase/updateAccount.go
Outdated
| if err != nil { | ||
| return errors.New("AccountNotFound") | ||
| return err |
There was a problem hiding this comment.
Same thing here. Please check all the cases you replaced NotFound with the infra err. It's not right.
| voHelper "github.com/goinfinite/os/src/domain/valueObject/helper" | ||
| ) | ||
|
|
||
| const SecureAccessPublicKeyContentRegex string = `^(?:ssh-(?:rsa|ed25519)) (?:[\w\/\+\=]+)(?: [\w@\-_]{6,32})?$` |
There was a problem hiding this comment.
I removed this regex validation and decided to validate it the same way we do with SslContent: there's no need to use a regex if we already have a parser to handle the validation, as it's done in the entity.
| htmx.ajax( | ||
| 'DELETE', | ||
| `/api/v1/account/secure-access-public-key/${this.secureAccessPublicKey.id}/`, | ||
| {swap: 'none'}, | ||
| ); | ||
| this.$dispatch('delete:secure-access-public-key'); |
There was a problem hiding this comment.
On Ez I started adding dispatch using a .then() on the htmx.ajax to make sure it only gets triggered when the request is successful. Little touch, but avoids future complains.
| '/api/v1/account/' + this.account.id + '/', | ||
| {swap: 'none'}, | ||
| ); | ||
| htmx.ajax('DELETE', `/api/v1/account/`, {swap: 'none'}); | ||
| this.$dispatch('delete:account'); |
| templ SecureAccessPublicKeysList() { | ||
| @componentStructural.Alert( | ||
| componentStructural.AlertTypeInfo, | ||
| "The management of these public keys will only be possible if the OpenSSH service is installed. Otherwise, you won't be able to add or remove public keys, as they are used for SSH access.", |
There was a problem hiding this comment.
Well, you can manage keys, they will just not have any meaning without OpenSSH. I think your description should tell people that. Don't you think?
There was a problem hiding this comment.
Actually, the route does not allow adding or removing keys as it performs a check to ensure the service is running. However, upon reflection, any modification to the authorized_keys file is irrelevant regardless of whether the service is running, which is entirely different from the case of databases, for example.
I removed this validation from the routes, now allowing requests to function normally. However, without the service, it won't serve any purpose. Additionally, I updated the alert content as well!
| @@ -255,3 +319,92 @@ templ UpdateApiKeyModal() { | |||
| @UpdateApiKeyContent() | |||
| } | |||
| } | |||
|
|
|||
| templ SecureAccessPublicKeysList() { | |||
There was a problem hiding this comment.
SecureAccessPublicKeysModalList
There was a problem hiding this comment.
It doesn't make much sense to use this name, considering this isn't the modal itself but its content, similar to how we handle forms, tabs, and other components. Moreover, this name suggests a list of modals rather than a modal containing a list.
| @componentForm.InputField(componentForm.InputFieldDto{ | ||
| Type: settingEntity.Value.GetType(), | ||
| Label: settingEntity.Name.String(), | ||
| BindModelValuePath: "phpConfigs.settings[" + strconv.FormatInt(int64(settingIndex), 10) + "].value", |
There was a problem hiding this comment.
You can use strconv.Itoa here I think.
… using authorized key parser
…s error when key content is wrong
…nfra env that store user data base directory
… swagger with this
…hat the file will be automatically recreated
…messages and code itself
| voHelper "github.com/goinfinite/os/src/domain/valueObject/helper" | ||
| ) | ||
|
|
||
| const SecureAccessPublicKeyNameRegex string = `^[\w@\-_]{6,32}$` |
There was a problem hiding this comment.
The key name may start with a - _ or @? That's weird. I think you should go with ^[A-Za-z0-9][\w@\-_]{5,32}$ instead.
… "@", "_" or "-" as prefix
No description provided.