Add todos and stuff

src/Api/Auth/Controllers/AccountsController.cs

@@ -209,14 +209,15 @@ public class AccountsController : Controller
             throw new UnauthorizedAccessException();
         }
 
-        Guid? sessionId = null;
+        // TODO: should this be feature flagged
+        Guid? opaqueSessionId = null;
         if (model.OpaqueSessionId != null)
         {
-            sessionId = Guid.Parse(model.OpaqueSessionId);
+            opaqueSessionId = Guid.Parse(model.OpaqueSessionId);
         }
 
         var result = await _userService.ChangePasswordAsync(user, model.MasterPasswordHash,
-            model.NewMasterPasswordHash, model.MasterPasswordHint, model.Key, sessionId);
+            model.NewMasterPasswordHash, model.MasterPasswordHint, model.Key, opaqueSessionId);
         if (result.Succeeded)
         {
             return;

src/Api/Auth/Controllers/OpaqueKeyExchangeController.cs

@@ -1,15 +1,19 @@
 using Bit.Api.Auth.Models.Request.Opaque;
 using Bit.Api.Auth.Models.Response.Opaque;
+using Bit.Core;
 using Bit.Core.Auth.Models.Api.Request.Opaque;
 using Bit.Core.Auth.Models.Api.Response.Opaque;
 using Bit.Core.Auth.Services;
 using Bit.Core.Services;
+using Bit.Core.Utilities;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 
 namespace Bit.Api.Auth.Controllers;
 
+// TODO: move to identity
 [Route("opaque")]
+[RequireFeature(FeatureFlagKeys.OpaqueKeyExchange)]
 public class OpaqueKeyExchangeController : Controller
 {
     private readonly IOpaqueKeyExchangeService _opaqueKeyExchangeService;
@@ -24,7 +28,8 @@ public class OpaqueKeyExchangeController : Controller
         _userService = userService;
     }
 
-    [Authorize("Web")]
+    // TODO: investigate removing ~/opaque from all routes and using controller level route attribute
+    [Authorize("Application")]
     [HttpPost("~/opaque/start-registration")]
     public async Task<OpaqueRegistrationStartResponse> StartRegistrationAsync([FromBody] OpaqueRegistrationStartRequest request)
     {
@@ -34,7 +39,7 @@ public class OpaqueKeyExchangeController : Controller
     }
 
 
-    [Authorize("Web")]
+    [Authorize("Application")]
     [HttpPost("~/opaque/finish-registration")]
     public async void FinishRegistrationAsync([FromBody] OpaqueRegistrationFinishRequest request)
     {
@@ -42,7 +47,7 @@ public class OpaqueKeyExchangeController : Controller
         await _opaqueKeyExchangeService.FinishRegistration(request.SessionId, Convert.FromBase64String(request.RegistrationUpload), user, request.KeySet);
     }
 
-    [Authorize("Web")]
+    [Authorize("Application")]
     [HttpPost("~/opaque/set-registration-active")]
     public async void SetRegistrationActive([FromBody] OpaqueSetRegistrationActiveRequest request)
     {
@@ -51,6 +56,7 @@ public class OpaqueKeyExchangeController : Controller
     }
 
     // TODO: Remove and move to token endpoint
+    [AllowAnonymous]
     [HttpPost("~/opaque/start-login")]
     public async Task<OpaqueLoginStartResponse> StartLoginAsync([FromBody] OpaqueLoginStartRequest request)
     {
@@ -59,6 +65,7 @@ public class OpaqueKeyExchangeController : Controller
     }
 
     // TODO: Remove and move to token endpoint
+    [AllowAnonymous]
     [HttpPost("~/opaque/finish-login")]
     public async Task<bool> FinishLoginAsync([FromBody] OpaqueLoginFinishRequest request)
     {

src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs

@@ -5,6 +5,7 @@ using Bit.Core.Models.Api;
 
 namespace Bit.Core.Auth.Models.Api.Response;
 
+// TODO: in order to support opaque decryption via export key, we must update this to have a new option for opaque
 public class UserDecryptionOptions : ResponseModel
 {
     public UserDecryptionOptions() : base("userDecryptionOptions")

src/Core/Auth/Services/Implementations/OpaqueKeyExchangeService.cs

@@ -15,6 +15,8 @@ namespace Bit.Core.Auth.Services;
 
 #nullable enable
 
+// TODO: feature flag this service
+// TODO: add test suite
 public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
 {
     private readonly BitwardenOpaqueServer _bitwardenOpaque;
@@ -22,7 +24,7 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
     private readonly IDistributedCache _distributedCache;
     private readonly IUserRepository _userRepository;
 
-    const string REGISTER_SESSION_KEY = "opaque_register_session_{0}";
+    const string REGISTRATION_SESSION_KEY = "opaque_register_session_{0}";
     const string LOGIN_SESSION_KEY = "opaque_login_session_{0}";
 
     public OpaqueKeyExchangeService(
@@ -41,16 +43,21 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
     {
         var registrationRequest = _bitwardenOpaque.StartRegistration(cipherConfiguration.ToNativeConfiguration(), null, request, user.Id.ToString());
 
-        var sessionId = Guid.NewGuid();
+        // We must persist the registration session state to the cache so we can have the server setup and cipher
-        var registerSession = new OpaqueKeyExchangeRegisterSession() { SessionId = sessionId, ServerSetup = registrationRequest.serverSetup, CipherConfiguration = cipherConfiguration, UserId = user.Id };
+        // Ïconfiguration available when the client finishes registration.
-        await _distributedCache.SetAsync(string.Format(REGISTER_SESSION_KEY, sessionId), Encoding.ASCII.GetBytes(JsonSerializer.Serialize(registerSession)));
+        var registrationSessionId = Guid.NewGuid();
+        var registrationSession = new OpaqueKeyExchangeRegistrationSession() { RegistrationSessionId = registrationSessionId, ServerSetup = registrationRequest.serverSetup, CipherConfiguration = cipherConfiguration, UserId = user.Id };
 
-        return new OpaqueRegistrationStartResponse(sessionId, Convert.ToBase64String(registrationRequest.registrationResponse));
+        // TODO: We need to audit this approach to make sure it works for all our use cases. We probably need a timeout to avoid persisting this indefinitely.
+        await _distributedCache.SetAsync(string.Format(REGISTRATION_SESSION_KEY, registrationSessionId), Encoding.ASCII.GetBytes(JsonSerializer.Serialize(registrationSession)));
+
+        return new OpaqueRegistrationStartResponse(registrationSessionId, Convert.ToBase64String(registrationRequest.registrationResponse));
     }
 
     public async Task FinishRegistration(Guid sessionId, byte[] registrationUpload, User user, RotateableOpaqueKeyset keyset)
     {
-        var serializedRegisterSession = await _distributedCache.GetAsync(string.Format(REGISTER_SESSION_KEY, sessionId));
+        // Look up the user's registration session
+        var serializedRegisterSession = await _distributedCache.GetAsync(string.Format(REGISTRATION_SESSION_KEY, sessionId));
         if (serializedRegisterSession == null)
         {
             throw new InvalidOperationException("Session not found");
@@ -58,25 +65,34 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
 
         try
         {
-            var registerSession = JsonSerializer.Deserialize<OpaqueKeyExchangeRegisterSession>(Encoding.ASCII.GetString(serializedRegisterSession))!;
+            // Deserialize the registration session and finish the registration
-            var registrationFinish = _bitwardenOpaque.FinishRegistration(registerSession.CipherConfiguration.ToNativeConfiguration(), registrationUpload);
+            var registrationSession = JsonSerializer.Deserialize<OpaqueKeyExchangeRegistrationSession>(Encoding.ASCII.GetString(serializedRegisterSession))!;
-            registerSession.PasswordFile = registrationFinish.serverRegistration;
+            var registrationFinish = _bitwardenOpaque.FinishRegistration(registrationSession.CipherConfiguration.ToNativeConfiguration(), registrationUpload);
-            registerSession.KeySet = Encoding.ASCII.GetBytes(JsonSerializer.Serialize(keyset));
+            // Save the keyset and password file to the registration session. In order to set their registration as
-            await _distributedCache.SetAsync(string.Format(REGISTER_SESSION_KEY, sessionId), Encoding.ASCII.GetBytes(JsonSerializer.Serialize(registerSession)));
+            // active, clients must call SetRegistrationActive or the user must change their password
+            registrationSession.PasswordFile = registrationFinish.serverRegistration;
+            registrationSession.KeySet = Encoding.ASCII.GetBytes(JsonSerializer.Serialize(keyset));
+            await _distributedCache.SetAsync(string.Format(REGISTRATION_SESSION_KEY, sessionId), Encoding.ASCII.GetBytes(JsonSerializer.Serialize(registrationSession)));
         }
         catch (Exception e)
         {
-            await _distributedCache.RemoveAsync(string.Format(REGISTER_SESSION_KEY, sessionId));
+            // If anything goes wrong, we need to remove the session from the cache
+            await ClearRegistrationSession(sessionId);
             throw new Exception(e.Message);
         }
     }
 
+    private async Task ClearRegistrationSession(Guid sessionId)
+    {
+        await _distributedCache.RemoveAsync(string.Format(REGISTRATION_SESSION_KEY, sessionId));
+    }
+
     public async Task<(Guid, byte[])> StartLogin(byte[] request, string email)
     {
         var user = await _userRepository.GetByEmailAsync(email);
         if (user == null)
         {
-            // todo don't allow user enumeration
+            // TODO: don't allow user enumeration
             throw new InvalidOperationException("User not found");
         }
 
@@ -122,7 +138,7 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
 
         var loginState = loginSession.LoginState;
         var cipherConfiguration = loginSession.CipherConfiguration;
-        await _distributedCache.RemoveAsync(string.Format(LOGIN_SESSION_KEY, sessionId));
+        await ClearAuthenticationSession(sessionId);
 
         try
         {
@@ -158,12 +174,12 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
 
     public async Task SetRegistrationActiveForAccount(Guid sessionId, User user)
     {
-        var serializedRegisterSession = await _distributedCache.GetAsync(string.Format(REGISTER_SESSION_KEY, sessionId));
+        var serializedRegisterSession = await _distributedCache.GetAsync(string.Format(REGISTRATION_SESSION_KEY, sessionId));
         if (serializedRegisterSession == null)
         {
             throw new InvalidOperationException("Session not found");
         }
-        var session = JsonSerializer.Deserialize<OpaqueKeyExchangeRegisterSession>(Encoding.ASCII.GetString(serializedRegisterSession))!;
+        var session = JsonSerializer.Deserialize<OpaqueKeyExchangeRegistrationSession>(Encoding.ASCII.GetString(serializedRegisterSession))!;
 
         if (session.UserId != user.Id)
         {
@@ -196,6 +212,8 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
             CreationDate = DateTime.UtcNow
         };
 
+        // Delete any existing registration and then enroll user with latest
+        // TODO: this could be a single atomic replace / upsert
         await Unenroll(user);
         await _opaqueKeyExchangeCredentialRepository.CreateAsync(credential);
     }
@@ -209,6 +227,10 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
         }
     }
 
+    // TODO: evaluate if this should live in a utility class and/or under KM control
+    /// <summary>
+    /// Creates a genuinely random GUID as the login session IDs for opaque need to be cryptographically secure.
+    /// </summary>
     private static Guid MakeCryptoGuid()
     {
         // Get 16 cryptographically random bytes
@@ -227,9 +249,9 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService
     }
 }
 
-public class OpaqueKeyExchangeRegisterSession
+public class OpaqueKeyExchangeRegistrationSession
 {
-    public required Guid SessionId { get; set; }
+    public required Guid RegistrationSessionId { get; set; }
     public required byte[] ServerSetup { get; set; }
     public required OpaqueKeyExchangeCipherConfiguration CipherConfiguration { get; set; }
     public required Guid UserId { get; set; }

src/Core/Services/Implementations/UserService.cs

@@ -668,6 +668,7 @@ public class UserService : UserManager<User>, IUserService, IDisposable
             user.Key = key;
             user.MasterPasswordHint = passwordHint;
 
+            // TODO: feature flag this
             if (opaqueSessionId != null)
             {
                 await _opaqueKeyExchangeService.SetRegistrationActiveForAccount((Guid)opaqueSessionId, user);

src/Identity/Controllers/AccountsController.cs

@@ -292,6 +292,7 @@ public class AccountsController : Controller
         };
     }
 
+    // TODO: (1) This should be on own controller (2) reconcile this w/ start login on existing controller
     [HttpPost("opaque-ke/start-login")]
     [RequireFeature(FeatureFlagKeys.OpaqueKeyExchange)]
     public async Task<OpaqueLoginStartResponse> GetOpaqueKeyExchangeStartLoginMaterial([FromBody] OpaqueLoginStartRequest request)

src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs

@@ -431,6 +431,7 @@ public abstract class BaseRequestValidator<T> where T : class
     /// </summary>
     private async Task<UserDecryptionOptions> CreateUserDecryptionOptionsAsync(User user, Device device, ClaimsPrincipal subject)
     {
+        // TODO: update this builder to retrieve Opaque credentials by user id and build the OpaqueUserDecryptionOption
         var ssoConfig = await GetSsoConfigurationDataAsync(subject);
         return await UserDecryptionOptionsBuilder
             .ForUser(user)

src/Identity/IdentityServer/RequestValidators/OpaqueKeyExchangeGrantValidator.cs

@@ -79,6 +79,10 @@ public class OpaqueKeyExchangeGrantValidator : BaseRequestValidator<ExtensionGra
             return;
         }
 
+        // TODO: we need to validate that the email sent up is the same one pulled from the session
+        // TODO: discuss with Ike if pulling over existing AuthEmailHeaderIsValid logic from
+        // ResourceOwnerPasswordValidator is best or if we should should refactor in some way.
+
         await ValidateAsync(context, context.Request, new CustomValidatorRequestContext { User = user });
     }