Skip to content

Commit 9de40de

Browse files
Mackinnon Buckwtgodbe
Mackinnon Buck
authored andcommitted
Merged PR 49095: [internal/release/9.0] Add empty string check for recovery code
# Add empty string check for recovery code If an empty string gets passed as the recovery code to `UserStoreBase.RedeemCodeAsync(TUser user, string code, CancellationToken ct)`, the method returns `true`, incorrectly indicating a valid recovery code. This PR resolves the issue by validating that the `code` argument is not an empty string. ## Description The `RedeemCodeAsync()` method already validates that `code` is non-null. This PR: * Extends the logic in this method to handle the empty string (`""`) case * Adds tests validating that an exception gets thrown when `code` is `null` or `""` ---- #### AI description (iteration 1) #### PR Classification Bug fix #### PR Summary This pull request adds a check for empty strings in recovery codes to prevent null or empty values from being processed. - `src/Shared/ThrowHelpers/ArgumentNullThrowHelper.cs`: Added `ThrowIfNullOrEmpty` method to validate strings as non-null and non-empty. - `src/Identity/EntityFrameworkCore/test/EF.Test/UserStoreTest.cs`: Added tests to ensure `RedeemCodeAsync` throws exceptions for null or empty codes. - `src/Identity/Extensions.Stores/src/UserStoreBase.cs`: Updated `ThrowIfNull` to `ThrowIfNullOrEmpty` for code validation in `RedeemCodeAsync`. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> ---- #### AI description (iteration 1) #### PR Classification Bug fix #### PR Summary This pull request adds a check for empty strings in recovery code validation to prevent errors. - `src/Shared/ThrowHelpers/ArgumentNullThrowHelper.cs`: Added `ThrowIfNullOrEmpty` method to validate non-null and non-empty strings. - `src/Identity/EntityFrameworkCore/test/EF.Test/UserStoreTest.cs`: Added tests for null and empty recovery code validation. - `src/Identity/Extensions.Stores/src/UserStoreBase.cs`: Updated recovery code validation to use `ThrowIfNullOrEmpty`.
1 parent c15ce45 commit 9de40de

File tree

3 files changed

+27
-1
lines changed

3 files changed

+27
-1
lines changed

src/Identity/EntityFrameworkCore/test/EF.Test/UserStoreTest.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ await Assert.ThrowsAsync<ArgumentNullException>("user",
144144
await Assert.ThrowsAsync<ArgumentNullException>("user", async () => await store.GetTwoFactorEnabledAsync(null));
145145
await Assert.ThrowsAsync<ArgumentNullException>("user",
146146
async () => await store.SetTwoFactorEnabledAsync(null, true));
147+
await Assert.ThrowsAsync<ArgumentNullException>("user", async () => await store.RedeemCodeAsync(user: null, code: "fake", default));
148+
await Assert.ThrowsAsync<ArgumentNullException>("code", async () => await store.RedeemCodeAsync(new IdentityUser("fake"), code: null, default));
149+
await Assert.ThrowsAsync<ArgumentException>("code", async () => await store.RedeemCodeAsync(new IdentityUser("fake"), code: "", default));
147150
await Assert.ThrowsAsync<ArgumentNullException>("user", async () => await store.GetAccessFailedCountAsync(null));
148151
await Assert.ThrowsAsync<ArgumentNullException>("user", async () => await store.GetLockoutEnabledAsync(null));
149152
await Assert.ThrowsAsync<ArgumentNullException>("user", async () => await store.SetLockoutEnabledAsync(null, false));

src/Identity/Extensions.Stores/src/UserStoreBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,7 @@ public virtual async Task<bool> RedeemCodeAsync(TUser user, string code, Cancell
969969
ThrowIfDisposed();
970970

971971
ArgumentNullThrowHelper.ThrowIfNull(user);
972-
ArgumentNullThrowHelper.ThrowIfNull(code);
972+
ArgumentNullThrowHelper.ThrowIfNullOrEmpty(code);
973973

974974
var mergedCodes = await GetTokenAsync(user, InternalLoginProvider, RecoveryCodeTokenName, cancellationToken).ConfigureAwait(false) ?? "";
975975
var splitCodes = mergedCodes.Split(';');

src/Shared/ThrowHelpers/ArgumentNullThrowHelper.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,29 @@ public static void ThrowIfNull(
3030
#endif
3131
}
3232

33+
/// <summary>Throws an <see cref="ArgumentException"/> if <paramref name="argument"/> is null or empty.</summary>
34+
/// <param name="argument">The <see cref="string"/> argument to validate as non-null and non-empty.</param>
35+
/// <param name="paramName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
36+
public static void ThrowIfNullOrEmpty(
37+
#if INTERNAL_NULLABLE_ATTRIBUTES || NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER
38+
[NotNull]
39+
#endif
40+
string? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null)
41+
{
42+
#if !NET7_0_OR_GREATER || NETSTANDARD || NETFRAMEWORK
43+
if (argument is null)
44+
{
45+
Throw(paramName);
46+
}
47+
else if (argument.Length == 0)
48+
{
49+
throw new ArgumentException("Must not be null or empty", paramName);
50+
}
51+
#else
52+
ArgumentException.ThrowIfNullOrEmpty(argument, paramName);
53+
#endif
54+
}
55+
3356
#if !NET7_0_OR_GREATER || NETSTANDARD || NETFRAMEWORK
3457
#if INTERNAL_NULLABLE_ATTRIBUTES || NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER
3558
[DoesNotReturn]

0 commit comments

Comments
 (0)