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

Refactorings #25

Merged
merged 11 commits into from
Aug 31, 2024
Merged

Refactorings #25

merged 11 commits into from
Aug 31, 2024

Conversation

hannesm
Copy link
Contributor

@hannesm hannesm commented Aug 31, 2024

The main purpose is to remove the levels of indentation in unikernel.ml.

I'm a bit unsure about the authentication, esp on the verify email. My perception is:

  • we always want to authenticate the cookie and have a user around
  • for JS & images & css it's fine to not being authenticated
  • for /sign-up and /sign-in we don't need authentication
  • for /api-register also no need for authentication (cookie is pushed)
  • for /api/login no authentication (it is special, since only at that point the cookie is pushed)

for verify-email the user needs to be logged in (and it must be the same who's verifying the email address).

does this make sense?

@PizieDust
Copy link
Collaborator

The main purpose is to remove the levels of indentation in unikernel.ml.

I'm a bit unsure about the authentication, esp on the verify email. My perception is:

  • we always want to authenticate the cookie and have a user around
  • for JS & images & css it's fine to not being authenticated
  • for /sign-up and /sign-in we don't need authentication
  • for /api-register also no need for authentication (cookie is pushed)
  • for /api/login no authentication (it is special, since only at that point the cookie is pushed)

for verify-email the user needs to be logged in (and it must be the same who's verifying the email address).

does this make sense?

Yes this is perfect.

unikernel.ml Outdated Show resolved Hide resolved
Co-authored-by: PixieDust <[email protected]>
@PizieDust
Copy link
Collaborator

PizieDust commented Aug 31, 2024

Thank you @hannesm .🥇

@PizieDust PizieDust merged commit 9165276 into main Aug 31, 2024
2 checks passed
@PizieDust PizieDust deleted the more branch August 31, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants