Skip to content

Commit 2643470

Browse files
authored
Improve Code Quality (#301)
* Configure await * Presize lists * Use Ordinal comparer to check string equality * Use singleton for True & False CborBoolean * Fix private variable name * Parse double using invariant culture * Format AuthenticatorGetAssertionCommand * Use Array.Empty<byte>() * Add missing parenthes
1 parent d25a42e commit 2643470

File tree

10 files changed

+53
-47
lines changed

10 files changed

+53
-47
lines changed

Src/Fido2.Ctap2/Commands/AuthenticatorGetAssertionCommand.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,10 @@ public AuthenticatorGetAssertionCommand(
7777
var cbor = new CborMap
7878
{
7979
{ 0x01, RpId },
80-
{ 0x02, ClientDataHash }
80+
{ 0x02, ClientDataHash },
81+
{ 0x03, AllowList.ToCborArray() } // allowList
8182
};
8283

83-
cbor.Add(0x03, AllowList.ToCborArray()); // allowList
84-
8584
if (Extensions != null)
8685
{
8786
cbor.Add(0x04, Extensions);
@@ -102,7 +101,6 @@ public AuthenticatorGetAssertionCommand(
102101
}
103102
}
104103

105-
106104
public sealed class AuthenticatorGetAssertionOptions
107105
{
108106
/// <summary>

Src/Fido2.Ctap2/Devices/FidoAuthenticator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public async ValueTask<NegotiateSharedSecretResult> NegotiateSharedSecretAsync()
158158
{
159159
var command = new AuthenticatorClientPinCommand(pinProtocol: 0x01, subCommand: AuthenticatorClientPinSubCommand.GetKeyAgreement);
160160

161-
var result = await ExecuteClientPinCommandAsync(command);
161+
var result = await ExecuteClientPinCommandAsync(command).ConfigureAwait(false);
162162

163163
var authenticatorKey = result.KeyAgreement!;
164164

Src/Fido2.Models/CredentialCreateOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public static CredentialCreateOptions Create(Fido2Configuration config, byte[] c
7878
Rp = new PublicKeyCredentialRpEntity(config.ServerDomain, config.ServerName, config.ServerIcon),
7979
Timeout = config.Timeout,
8080
User = user,
81-
PubKeyCredParams = new List<PubKeyCredParam>()
81+
PubKeyCredParams = new List<PubKeyCredParam>(10)
8282
{
8383
// Add additional as appropriate
8484
PubKeyCredParam.ES256,

Src/Fido2/AttestationFormat/AndroidSafetyNet.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Globalization;
34
using System.IdentityModel.Tokens.Jwt;
45
using System.Linq;
56
using System.Security.Cryptography;
@@ -126,7 +127,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
126127
}
127128
if (claim is { Type: "timestampMs", ValueType: "http://www.w3.org/2001/XMLSchema#integer64" })
128129
{
129-
timestamp = DateTimeOffset.UnixEpoch.AddMilliseconds(double.Parse(claim.Value));
130+
timestamp = DateTimeOffset.UnixEpoch.AddMilliseconds(double.Parse(claim.Value, CultureInfo.InvariantCulture));
130131
}
131132
}
132133

Src/Fido2/Cbor/CborBoolean.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
namespace Fido2NetLib.Cbor
44
{
5-
internal sealed class CborBoolean : CborObject
5+
public sealed class CborBoolean : CborObject
66
{
7+
public static readonly CborBoolean True = new(true);
8+
public static readonly CborBoolean False = new(false);
9+
710
public CborBoolean(bool value)
811
{
912
Value = value;
@@ -17,5 +20,9 @@ public override int GetHashCode()
1720
{
1821
return HashCode.Combine(Type, Value);
1922
}
23+
24+
public static explicit operator CborBoolean(bool value) => value ? True : False;
25+
26+
public static implicit operator bool(CborBoolean value) => value.Value;
2027
}
2128
}

Src/Fido2/Cbor/CborMap.cs

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,27 @@ namespace Fido2NetLib.Cbor
99
{
1010
public sealed class CborMap : CborObject, IReadOnlyDictionary<CborObject, CborObject>
1111
{
12-
private readonly List<KeyValuePair<CborObject, CborObject>> items;
12+
private readonly List<KeyValuePair<CborObject, CborObject>> _items;
1313

1414
public CborMap()
1515
{
16-
items = new();
16+
_items = new();
1717
}
1818

1919
public CborMap(int capacity)
2020
{
21-
items = new(capacity);
21+
_items = new(capacity);
2222
}
2323

2424
public override CborType Type => CborType.Map;
2525

26-
public int Count => items.Count;
26+
public int Count => _items.Count;
2727

2828
public IEnumerable<CborObject> Keys
2929
{
3030
get
3131
{
32-
foreach (var item in items)
32+
foreach (var item in _items)
3333
{
3434
yield return item.Key;
3535
}
@@ -40,7 +40,7 @@ public IEnumerable<CborObject> Values
4040
{
4141
get
4242
{
43-
foreach (var item in items)
43+
foreach (var item in _items)
4444
{
4545
yield return item.Value;
4646
}
@@ -49,82 +49,82 @@ public IEnumerable<CborObject> Values
4949

5050
public void Add(string key, CborObject value)
5151
{
52-
items.Add(new (new CborTextString(key), value));
52+
_items.Add(new (new CborTextString(key), value));
5353
}
5454

5555
public void Add(string key, bool value)
5656
{
57-
items.Add(new(new CborTextString(key), new CborBoolean(value)));
57+
_items.Add(new(new CborTextString(key), (CborBoolean)value));
5858
}
5959

6060
public void Add(long key, CborObject value)
6161
{
62-
items.Add(new (new CborInteger(key), value));
62+
_items.Add(new (new CborInteger(key), value));
6363
}
6464

6565
public void Add(long key, byte[] value)
6666
{
67-
items.Add(new(new CborInteger(key), new CborByteString(value)));
67+
_items.Add(new(new CborInteger(key), new CborByteString(value)));
6868
}
6969

7070
public void Add(long key, string value)
7171
{
72-
items.Add(new(new CborInteger(key), new CborTextString(value)));
72+
_items.Add(new(new CborInteger(key), new CborTextString(value)));
7373
}
7474

7575
public void Add(long key, long value)
7676
{
77-
items.Add(new(new CborInteger(key), new CborInteger(value)));
77+
_items.Add(new(new CborInteger(key), new CborInteger(value)));
7878
}
7979

8080
public void Add(string key, int value)
8181
{
82-
items.Add(new(new CborTextString(key), new CborInteger(value)));
82+
_items.Add(new(new CborTextString(key), new CborInteger(value)));
8383
}
8484

8585
public void Add(string key, string value)
8686
{
87-
items.Add(new(new CborTextString(key), new CborTextString(value)));
87+
_items.Add(new(new CborTextString(key), new CborTextString(value)));
8888
}
8989

9090
public void Add(string key, byte[] value)
9191
{
92-
items.Add(new(new CborTextString(key), new CborByteString(value)));
92+
_items.Add(new(new CborTextString(key), new CborByteString(value)));
9393
}
9494

9595
internal void Add(CborObject key, CborObject value)
9696
{
97-
items.Add(new (key, value));
97+
_items.Add(new (key, value));
9898
}
9999

100100
internal void Add(string key, COSE.Algorithm value)
101101
{
102-
items.Add(new(new CborTextString(key), new CborInteger((int)value)));
102+
_items.Add(new(new CborTextString(key), new CborInteger((int)value)));
103103
}
104104

105105
internal void Add(COSE.KeyCommonParameter key, COSE.KeyType value)
106106
{
107-
items.Add(new(new CborInteger((int)key), new CborInteger((int)value)));
107+
_items.Add(new(new CborInteger((int)key), new CborInteger((int)value)));
108108
}
109109

110110
internal void Add(COSE.KeyCommonParameter key, COSE.Algorithm value)
111111
{
112-
items.Add(new(new CborInteger((int)key), new CborInteger((int)value)));
112+
_items.Add(new(new CborInteger((int)key), new CborInteger((int)value)));
113113
}
114114

115115
internal void Add(COSE.KeyTypeParameter key, COSE.EllipticCurve value)
116116
{
117-
items.Add(new(new CborInteger((int)key), new CborInteger((int)value)));
117+
_items.Add(new(new CborInteger((int)key), new CborInteger((int)value)));
118118
}
119119

120120
internal void Add(COSE.KeyTypeParameter key, byte[] value)
121121
{
122-
items.Add(new(new CborInteger((int)key), new CborByteString(value)));
122+
_items.Add(new(new CborInteger((int)key), new CborByteString(value)));
123123
}
124124

125125
public bool ContainsKey(CborObject key)
126126
{
127-
foreach (var (k, _) in items)
127+
foreach (var (k, _) in _items)
128128
{
129129
if (k.Equals(key))
130130
return true;
@@ -142,12 +142,12 @@ public bool TryGetValue(CborObject key, [MaybeNullWhen(false)] out CborObject va
142142

143143
public IEnumerator<KeyValuePair<CborObject, CborObject>> GetEnumerator()
144144
{
145-
return items.GetEnumerator();
145+
return _items.GetEnumerator();
146146
}
147147

148148
IEnumerator IEnumerable.GetEnumerator()
149149
{
150-
return items.GetEnumerator();
150+
return _items.GetEnumerator();
151151
}
152152

153153
internal CborObject this[COSE.KeyCommonParameter key] => GetValue((long)key);
@@ -160,7 +160,7 @@ IEnumerator IEnumerable.GetEnumerator()
160160

161161
public CborObject GetValue(long key)
162162
{
163-
foreach (var item in items)
163+
foreach (var item in _items)
164164
{
165165
if (item.Key is CborInteger integerKey && integerKey.Value == key)
166166
{
@@ -177,7 +177,7 @@ public CborObject? this[CborObject key]
177177
#pragma warning disable CS8766
178178
get
179179
{
180-
foreach (var item in items)
180+
foreach (var item in _items)
181181
{
182182
if (item.Key.Equals(key))
183183
{
@@ -194,7 +194,7 @@ public override CborObject? this[string name]
194194
{
195195
get
196196
{
197-
foreach (var item in items)
197+
foreach (var item in _items)
198198
{
199199
if (item.Key is CborTextString keyText && keyText.Value.Equals(name, StringComparison.Ordinal))
200200
{
@@ -208,11 +208,11 @@ public override CborObject? this[string name]
208208

209209
internal void Remove(string key)
210210
{
211-
for (int i = 0; i < items.Count; i++)
211+
for (int i = 0; i < _items.Count; i++)
212212
{
213-
if (items[i].Key is CborTextString textKey && textKey.Value.Equals(key, StringComparison.Ordinal))
213+
if (_items[i].Key is CborTextString textKey && textKey.Value.Equals(key, StringComparison.Ordinal))
214214
{
215-
items.RemoveAt(i);
215+
_items.RemoveAt(i);
216216

217217
return;
218218
}
@@ -221,17 +221,17 @@ internal void Remove(string key)
221221

222222
internal void Set(string key, CborObject value)
223223
{
224-
for (int i = 0; i < items.Count; i++)
224+
for (int i = 0; i < _items.Count; i++)
225225
{
226-
if (items[i].Key is CborTextString textKey && textKey.Value.Equals(key, StringComparison.Ordinal))
226+
if (_items[i].Key is CborTextString textKey && textKey.Value.Equals(key, StringComparison.Ordinal))
227227
{
228-
items[i] = new KeyValuePair<CborObject, CborObject>(new CborTextString(key), value);
228+
_items[i] = new KeyValuePair<CborObject, CborObject>(new CborTextString(key), value);
229229

230230
return;
231231
}
232232
}
233233

234-
items.Add(new(new CborTextString(key), value));
234+
_items.Add(new(new CborTextString(key), value));
235235
}
236236
}
237237
}

Src/Fido2/Cbor/CborObject.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private static CborObject Read(CborReader reader)
6565
CborReaderState.StartMap => ReadMap(reader),
6666
CborReaderState.StartArray => ReadArray(reader),
6767
CborReaderState.TextString => new CborTextString(reader.ReadTextString()),
68-
CborReaderState.Boolean => new CborBoolean(reader.ReadBoolean()),
68+
CborReaderState.Boolean => (CborBoolean)reader.ReadBoolean(),
6969
CborReaderState.ByteString => new CborByteString(reader.ReadByteString()),
7070
CborReaderState.UnsignedInteger => new CborInteger(reader.ReadInt64()),
7171
CborReaderState.NegativeInteger => new CborInteger(reader.ReadInt64()),

Src/Fido2/CryptoUtils.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public static bool ValidateTrustChain(X509Certificate2[] trustPath, X509Certific
113113
if (chain.ChainElements
114114
.Cast<X509ChainElement>()
115115
.Skip(1)
116-
.Any(x => x.Certificate.Thumbprint == attestationRootCertificate.Thumbprint))
116+
.Any(x => x.Certificate.Thumbprint.Equals(attestationRootCertificate.Thumbprint, StringComparison.Ordinal)))
117117
return true;
118118
}
119119
}

Src/Fido2/Metadata/ConformanceMetadataRepository.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public async Task<MetadataBLOBPayload> DeserializeAndValidateBlob(string rawBLOB
145145

146146
var rootCert = GetX509Certificate(ROOT_CERT);
147147
var blobCertificates = new X509Certificate2[blobCertStrings.Length];
148-
var blobPublicKeys = new List<SecurityKey>();
148+
var blobPublicKeys = new List<SecurityKey>(blobCertStrings.Length);
149149

150150
for (int i = 0; i < blobCertStrings.Length; i++)
151151
{
@@ -207,7 +207,7 @@ public async Task<MetadataBLOBPayload> DeserializeAndValidateBlob(string rawBLOB
207207
}
208208

209209
// otherwise we have to manually validate that the root in the chain we are testing is the root we downloaded
210-
if (rootCert.Thumbprint == certChain.ChainElements[^1].Certificate.Thumbprint &&
210+
if (rootCert.Thumbprint.Equals(certChain.ChainElements[^1].Certificate.Thumbprint, StringComparison.Ordinal) &&
211211
// and that the number of elements in the chain accounts for what was in x5c plus the root we added
212212
certChain.ChainElements.Count == (blobCertStrings.Length + 1) &&
213213
// and that the root cert has exactly one status listed against it

Test/AuthenticatorResponse.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ public void TestAuthenticatorAttestationResponseInvalidType()
375375
AttestationObject = new CborMap {
376376
{ "fmt", "testing" },
377377
{ "attStmt", new CborMap() },
378-
{ "authData", new byte[0] }
378+
{ "authData", Array.Empty<byte>() }
379379
}.Encode(),
380380
ClientDataJson = clientDataJson
381381
},

0 commit comments

Comments
 (0)