-
Notifications
You must be signed in to change notification settings - Fork 0
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
[PM-10394] Add new item type ssh key #52
base: main
Are you sure you want to change the base?
Changes from all commits
0093725
cd5baf4
ddec883
c755ec3
39ab9a7
f582cf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
using Bit.Api.Vault.Models.Response; | ||
using Bit.Core; | ||
using Bit.Core.AdminConsole.Entities; | ||
using Bit.Core.AdminConsole.Enums.Provider; | ||
using Bit.Core.AdminConsole.Repositories; | ||
using Bit.Core.Context; | ||
using Bit.Core.Entities; | ||
using Bit.Core.Enums; | ||
using Bit.Core.Exceptions; | ||
|
@@ -10,6 +12,7 @@ | |
using Bit.Core.Services; | ||
using Bit.Core.Settings; | ||
using Bit.Core.Tools.Repositories; | ||
using Bit.Core.Vault.Models.Data; | ||
using Bit.Core.Vault.Repositories; | ||
using Microsoft.AspNetCore.Authorization; | ||
using Microsoft.AspNetCore.Mvc; | ||
|
@@ -30,6 +33,8 @@ public class SyncController : Controller | |
private readonly IPolicyRepository _policyRepository; | ||
private readonly ISendRepository _sendRepository; | ||
private readonly GlobalSettings _globalSettings; | ||
private readonly ICurrentContext _currentContext; | ||
private readonly Version _sshKeyCipherMinimumVersion = new(Constants.SSHKeyCipherMinimumVersion); | ||
|
||
public SyncController( | ||
IUserService userService, | ||
|
@@ -41,7 +46,8 @@ public SyncController( | |
IProviderUserRepository providerUserRepository, | ||
IPolicyRepository policyRepository, | ||
ISendRepository sendRepository, | ||
GlobalSettings globalSettings) | ||
GlobalSettings globalSettings, | ||
ICurrentContext currentContext) | ||
{ | ||
_userService = userService; | ||
_folderRepository = folderRepository; | ||
|
@@ -53,6 +59,7 @@ public SyncController( | |
_policyRepository = policyRepository; | ||
_sendRepository = sendRepository; | ||
_globalSettings = globalSettings; | ||
_currentContext = currentContext; | ||
} | ||
|
||
[HttpGet("")] | ||
|
@@ -74,7 +81,8 @@ await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, | |
var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled); | ||
|
||
var folders = await _folderRepository.GetManyByUserIdAsync(user.Id); | ||
var ciphers = await _cipherRepository.GetManyByUserIdAsync(user.Id, withOrganizations: hasEnabledOrgs); | ||
var allCiphers = await _cipherRepository.GetManyByUserIdAsync(user.Id, withOrganizations: hasEnabledOrgs); | ||
var ciphers = FilterSSHKeys(allCiphers); | ||
var sends = await _sendRepository.GetManyByUserIdAsync(user.Id); | ||
|
||
IEnumerable<CollectionDetails> collections = null; | ||
|
@@ -95,4 +103,16 @@ await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, | |
collectionCiphersGroupDict, excludeDomains, policies, sends); | ||
return response; | ||
} | ||
|
||
private ICollection<CipherDetails> FilterSSHKeys(ICollection<CipherDetails> ciphers) | ||
{ | ||
if (_currentContext.ClientVersion >= _sshKeyCipherMinimumVersion && _currentContext.DeviceType != DeviceType.Android && _currentContext.DeviceType != DeviceType.iOS && _currentContext.DeviceType != DeviceType.AndroidAmazon) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: This condition is complex and may be difficult to maintain. Consider extracting it into a separate method for better readability |
||
{ | ||
return ciphers; | ||
} | ||
else | ||
{ | ||
return ciphers.Where(c => c.Type != Core.Vault.Enums.CipherType.SSHKey).ToList(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
using Bit.Core.Utilities; | ||
using Bit.Core.Vault.Models.Data; | ||
|
||
namespace Bit.Api.Vault.Models; | ||
|
||
public class CipherSSHKeyModel | ||
{ | ||
public CipherSSHKeyModel() { } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Empty constructor might allow creation of invalid objects. Consider making it private if not needed publicly. |
||
|
||
public CipherSSHKeyModel(CipherSSHKeyData data) | ||
{ | ||
PrivateKey = data.PrivateKey; | ||
PublicKey = data.PublicKey; | ||
KeyFingerprint = data.KeyFingerprint; | ||
} | ||
Comment on lines
+10
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Constructor doesn't validate input. Consider adding null checks for data and its properties. |
||
|
||
[EncryptedString] | ||
[EncryptedStringLength(5000)] | ||
public string PrivateKey { get; set; } | ||
Comment on lines
+17
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: 5000 characters might be excessive for a private key. Verify if this limit is appropriate. |
||
[EncryptedString] | ||
[EncryptedStringLength(5000)] | ||
public string PublicKey { get; set; } | ||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Public keys are typically shorter than private keys. Consider using a smaller limit. |
||
[EncryptedString] | ||
[EncryptedStringLength(1000)] | ||
public string KeyFingerprint { get; set; } | ||
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Key fingerprints are usually much shorter. 1000 characters seems excessive. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,12 @@ public CipherMiniResponseModel(Cipher cipher, IGlobalSettings globalSettings, bo | |
cipherData = identityData; | ||
Identity = new CipherIdentityModel(identityData); | ||
break; | ||
case CipherType.SSHKey: | ||
var sshKeyData = JsonSerializer.Deserialize<CipherSSHKeyData>(cipher.Data); | ||
Data = sshKeyData; | ||
cipherData = sshKeyData; | ||
SSHKey = new CipherSSHKeyModel(sshKeyData); | ||
break; | ||
Comment on lines
+51
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider adding a null check for sshKeyData after deserialization to prevent potential null reference exceptions |
||
default: | ||
throw new ArgumentException("Unsupported " + nameof(Type) + "."); | ||
} | ||
|
@@ -76,6 +82,7 @@ public CipherMiniResponseModel(Cipher cipher, IGlobalSettings globalSettings, bo | |
public CipherCardModel Card { get; set; } | ||
public CipherIdentityModel Identity { get; set; } | ||
public CipherSecureNoteModel SecureNote { get; set; } | ||
public CipherSSHKeyModel SSHKey { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Ensure that the SSHKey property is properly documented with XML comments for consistency with other properties |
||
public IEnumerable<CipherFieldModel> Fields { get; set; } | ||
public IEnumerable<CipherPasswordHistoryModel> PasswordHistory { get; set; } | ||
public IEnumerable<AttachmentResponseModel> Attachments { get; set; } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,4 +8,5 @@ public enum CipherType : byte | |
SecureNote = 2, | ||
Card = 3, | ||
Identity = 4, | ||
SSHKey = 5, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
namespace Bit.Core.Vault.Models.Data; | ||
|
||
public class CipherSSHKeyData : CipherData | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Add XML documentation for the class and its purpose |
||
{ | ||
public CipherSSHKeyData() { } | ||
|
||
public string PrivateKey { get; set; } | ||
public string PublicKey { get; set; } | ||
public string KeyFingerprint { get; set; } | ||
Comment on lines
+7
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider 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.
style: Consider moving this constant to a centralized location for version-related constants