Skip to content

Commit 192c51d

Browse files
committed
Address PR #813 bot review comments
- MfaService: Store null instead of "EXHAUSTED" sentinel when recovery codes are exhausted - MfaCredential.SetRecoveryCodes: Allow null/empty to clear codes (was throwing on empty) - MfaCredential.Revoke(): Fix doc comment - clarify secret is preserved for audit trail - AuthenticationRegistration: Add SignInScheme cookie for OAuth/OIDC external auth flow - LoginView: Move type import before executable statements - MfaSetup: Fix misleading "scan QR code" copy when no QR is rendered
1 parent 9112435 commit 192c51d

File tree

5 files changed

+34
-15
lines changed

5 files changed

+34
-15
lines changed

backend/src/Taskdeck.Api/Extensions/AuthenticationRegistration.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Security.Claims;
22
using System.Text;
33
using Microsoft.AspNetCore.Authentication;
4+
using Microsoft.AspNetCore.Authentication.Cookies;
45
using Microsoft.AspNetCore.Authentication.JwtBearer;
56
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
67
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
@@ -14,6 +15,10 @@ namespace Taskdeck.Api.Extensions;
1415

1516
public static class AuthenticationRegistration
1617
{
18+
/// <summary>
19+
/// Cookie scheme used for temporary external auth state (OAuth/OIDC handshake).
20+
/// </summary>
21+
public const string ExternalAuthenticationScheme = "External";
1722
public static IServiceCollection AddTaskdeckAuthentication(
1823
this IServiceCollection services,
1924
JwtSettings jwtSettings,
@@ -28,7 +33,21 @@ public static IServiceCollection AddTaskdeckAuthentication(
2833
return services;
2934
}
3035

31-
var authBuilder = services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
36+
var authBuilder = services.AddAuthentication(options =>
37+
{
38+
options.DefaultAuthenticateScheme = JwtBearerDefaults.AuthenticationScheme;
39+
options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme;
40+
options.DefaultSignInScheme = ExternalAuthenticationScheme;
41+
})
42+
.AddCookie(ExternalAuthenticationScheme, options =>
43+
{
44+
options.Cookie.Name = ".Taskdeck.ExternalAuth";
45+
options.Cookie.HttpOnly = true;
46+
options.Cookie.SameSite = SameSiteMode.Lax;
47+
options.Cookie.SecurePolicy = CookieSecurePolicy.SameAsRequest;
48+
options.ExpireTimeSpan = TimeSpan.FromMinutes(5);
49+
options.SlidingExpiration = false;
50+
})
3251
.AddJwtBearer(options =>
3352
{
3453
options.TokenValidationParameters = new TokenValidationParameters
@@ -92,6 +111,7 @@ await context.Response.WriteAsJsonAsync(new ApiErrorResponse(
92111
{
93112
authBuilder.AddOAuth("GitHub", options =>
94113
{
114+
options.SignInScheme = ExternalAuthenticationScheme;
95115
options.ClientId = gitHubOAuthSettings.ClientId;
96116
options.ClientSecret = gitHubOAuthSettings.ClientSecret;
97117
options.AuthorizationEndpoint = "https://github.com/login/oauth/authorize";
@@ -127,6 +147,7 @@ await context.Response.WriteAsJsonAsync(new ApiErrorResponse(
127147

128148
authBuilder.AddOpenIdConnect(schemeName, provider.DisplayName, options =>
129149
{
150+
options.SignInScheme = ExternalAuthenticationScheme;
130151
options.Authority = provider.Authority;
131152
options.ClientId = provider.ClientId;
132153
options.ClientSecret = provider.ClientSecret;

backend/src/Taskdeck.Application/Services/MfaService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ private bool TryUseRecoveryCode(MfaCredential credential, string code)
290290
hashedCodes.RemoveAt(i);
291291
credential.SetRecoveryCodes(hashedCodes.Count > 0
292292
? string.Join(",", hashedCodes)
293-
: "EXHAUSTED"); // Sentinel value when all recovery codes are used
293+
: null); // Clear recovery codes when all are exhausted
294294
return true;
295295
}
296296

backend/src/Taskdeck.Domain/Entities/MfaCredential.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,18 @@ public void Confirm()
6969

7070
/// <summary>
7171
/// Sets the hashed recovery codes for this credential.
72+
/// Pass null or empty to clear all recovery codes (e.g., when exhausted).
7273
/// </summary>
73-
public void SetRecoveryCodes(string hashedRecoveryCodes)
74+
public void SetRecoveryCodes(string? hashedRecoveryCodes)
7475
{
75-
if (string.IsNullOrWhiteSpace(hashedRecoveryCodes))
76-
throw new DomainException(ErrorCodes.ValidationError, "Recovery codes cannot be empty");
77-
78-
RecoveryCodes = hashedRecoveryCodes;
76+
RecoveryCodes = string.IsNullOrWhiteSpace(hashedRecoveryCodes) ? null : hashedRecoveryCodes;
7977
Touch();
8078
}
8179

8280
/// <summary>
83-
/// Revokes this MFA credential by clearing the secret and marking as unconfirmed.
84-
/// The entity remains for audit trail purposes.
81+
/// Revokes this MFA credential by marking as unconfirmed.
82+
/// Note: The secret is intentionally preserved for audit trail purposes;
83+
/// full deletion should use repository deletion instead.
8584
/// </summary>
8685
public void Revoke()
8786
{

frontend/taskdeck-web/src/components/MfaSetup.vue

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,12 @@ onMounted(loadStatus)
107107
<!-- Setup in progress -->
108108
<div v-else-if="setupResponse" class="td-mfa-setup__wizard">
109109
<p class="td-mfa-setup__step">
110-
1. Scan the QR code below with your authenticator app (Google Authenticator, Authy, etc.)
110+
1. Add this secret to your authenticator app (Google Authenticator, Authy, etc.)
111111
</p>
112-
<div class="td-mfa-setup__qr">
112+
<div class="td-mfa-setup__secret-container">
113113
<code class="td-mfa-setup__secret">{{ setupResponse.sharedSecret }}</code>
114114
<p class="td-mfa-setup__hint">
115-
Or manually enter this secret in your authenticator app.
115+
Copy and paste this secret into your authenticator app.
116116
</p>
117117
</div>
118118

@@ -256,7 +256,7 @@ onMounted(loadStatus)
256256
font-size: var(--td-font-sm);
257257
}
258258
259-
.td-mfa-setup__qr {
259+
.td-mfa-setup__secret-container {
260260
background: var(--td-surface-container);
261261
padding: var(--td-space-4);
262262
border-radius: var(--td-radius-md);

frontend/taskdeck-web/src/views/LoginView.vue

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<script setup lang="ts">
2+
import type { OidcProviderInfo } from '../types/auth'
23
import { ref, onMounted } from 'vue'
34
import { useRouter, useRoute } from 'vue-router'
45
import { useSessionStore } from '../store/sessionStore'
@@ -10,8 +11,6 @@ const router = useRouter()
1011
const route = useRoute()
1112
const session = useSessionStore()
1213
13-
import type { OidcProviderInfo } from '../types/auth'
14-
1514
const username = ref('')
1615
const password = ref('')
1716
const formError = ref<string | null>(null)

0 commit comments

Comments
 (0)