Sqlite rebased #21
Sqlite rebased #21
Conversation
This is just a toy example database where we can store users and associated credentials, as an example for the server, don't use this in production.
I was running into a few compile errors which didn't show up because the Main module wasn't touched. The cause was that cabal didn't want to recompile the Database module every time I touched it. That is also what caused some of Ruud's commits to not build.
This way we still have access to the display name and username.
This ensures that we always finish our transactions whenever any errors occur (like UNIQUE constraint violations). Without this, the server could keep transactions opened meaning subsequent requests would not work correctly.
|
Perhaps we can store the EC2 Points in compressed form ( |
|
lets not make that a blocker. I'll open a new issue for it |
Overflow in what sense? They were |
| @@ -234,7 +220,9 @@ completeLogin sessions users = do | |||
| verifyLogin :: SessionId -> Session -> Fido2.UserId -> Fido2.Challenge -> Scotty.ActionM () | |||
| verifyLogin sessionId session userId challenge = do | |||
| credential <- Scotty.jsonData @(Fido2.PublicKeyCredential Fido2.AuthenticatorAssertionResponse) | |||
| credentials <- (liftIO $ getUserCredentials userId users) >>= handleError | |||
| credentials <- liftIO $ Database.withTransaction db $ \tx -> do | |||
There was a problem hiding this comment.
We already have the credential here, so we can look up only that one by id, right?
There was a problem hiding this comment.
That would require us to trust that the user actually respected the list of allowedCredentials that we passed to them. I'm not sure whether that is safe (but I suspect it isn't).
This code ensures that we only check the challenge against the list of credentials that are actually associated with the current user in the DB.
We can also change this to a lookup by both the user ID and the credential ID of course, but I'm not sure if there is a reason for choosing one over the other.
Do you have a preference?
There was a problem hiding this comment.
We can also change this to a lookup by both the user ID and the credential ID of course
Yes let's do this.
I observed that the signature verification during login didn't work anymore after I got things to compile. When I printed the public keys after reading them back from the DB, they seemed to have overflowed. But you're right: these fields were Seems like the |
This allows us to avoid partiality in the publicKeyX and publicKeyY accessors, as well as expose a smart constructor which always checks whether the point is valid.
|
Lets merge this "as is" for now. I'm planning to do some major refactoring and I dont want this to become stale again |
This is #15 rebased on the latest
master.I tried to preserve the original git history as much as possible, while also making the following changes:
Fido2.PublicKeyinstead ofFido2.Ec2Key.gitignoreand the Cabal fileData.Binary.{decode,encode}before storing the public key parameters in the DB. (Otherwise, these can overflow).This now uses SQLite for the user state. Everything still seems to work now.