Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing Middlewares #10

Merged
merged 20 commits into from
Jul 16, 2024
Merged

Implementing Middlewares #10

merged 20 commits into from
Jul 16, 2024

Conversation

PizieDust
Copy link
Collaborator

@PizieDust PizieDust commented Jul 11, 2024

This PR introduces continues the work on authentication. It introduces Middlewares, and a first middleware for authentication.

cc @hannesm

@PizieDust PizieDust requested a review from hannesm July 11, 2024 08:00
@PizieDust PizieDust self-assigned this Jul 11, 2024
@PizieDust PizieDust marked this pull request as ready for review July 11, 2024 08:01
@PizieDust PizieDust added the enhancement New feature or request label Jul 11, 2024
let email =
json
|> Yojson.Basic.Util.member "email"
|> Yojson.Basic.to_string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not clear to me what the to_string does, we want to ensure that the "email" thingy is a `String -- so in the end I guess we need a json_utils module (since we've already similar code in user_model and albatross_json...

@hannesm
Copy link
Contributor

hannesm commented Jul 11, 2024

overall looks great (on my mobile phone), do we need to check the expiration of the cookie somewhere? (fine to put this as todo, but I think we need to encode the timestamp when the cookie was issued to compare now - expiration_duration < issuing_timestamp.

I have no idea why the ocamlformat CI errored.

@PizieDust
Copy link
Collaborator Author

overall looks great (on my mobile phone), do we need to check the expiration of the cookie somewhere? (fine to put this as todo, but I think we need to encode the timestamp when the cookie was issued to compare now - expiration_duration < issuing_timestamp.

I have no idea why the ocamlformat CI errored.

Thank you for the review. These are very helpful.
Indeed we will need to add timestamps.

@hannesm hannesm merged commit 28eebd8 into main Jul 16, 2024
1 of 2 checks passed
@hannesm hannesm deleted the registration branch July 16, 2024 09:34
@hannesm
Copy link
Contributor

hannesm commented Jul 16, 2024

thanks, merged. subsequent changes can follow in new prs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants