softwaremill / akka-http-session

Web & mobile client-side akka-http sessions, with optional JWT support
https://softwaremill.com/open-source/
Apache License 2.0
440 stars 58 forks source link

add csrf and session implementations for Tapir #86

Closed fupelaqu closed 1 year ago

adamw commented 1 year ago

Thanks for the PR! It will take some time to review as it's massive, but I've got two preliminary remarks:

  1. the new integration would need to have its own module, next to core, so that any users of core don't get tapir as a dependency
  2. it would be great to add some examples (in the readme, for example), which would explain how to use the new functionality. That way reviewing will also be easier.
fupelaqu commented 1 year ago

You are welcome ! I will first work on a specific module 'tapir' as you requested

fupelaqu commented 1 year ago

@adamw I just finished to move all resources related to tapir to a new module 'tapir'

If you want to start reviewing, I suggest you take a look at the unit tests which are exactly the same as those already implemented to test the Directives

I will add documentation later

fupelaqu commented 1 year ago

@adamw I added some examples for Tapir and update the README accordingly

fupelaqu commented 1 year ago

@adamw hello, any update ?

adamw commented 1 year ago

@fupelaqu Sorry for the delay, the vacation season is in full swing :).

The work you've done is impressive, but to be honest I don't think I'll be able to commit to maintaining this kind of integration. While your approach works - as exemplified by the tests - I have a feeling that after some design work (which I haven't done, and would probably take a while), a "tapir security" module would look differently.

I think the best course of action would be to publish your work as a separate project (providing Future-based security for tapir), of course then using your own group id / project id. I'd be happy to list it as one of the available extensions in the tapir documentation, as well as here.

Sorry again for the long time and that the outcome might not be what you probably aimed at initially, but I think that the separate project route might ultimately be the most flexible and safest from a maintenance point of view.

fupelaqu commented 1 year ago

@adamw thank you so much for your time ! I can't wait to see the tapir security extension !!!!

fupelaqu commented 1 year ago

Hi @adamw I hope you are doing well I would like to let you know that I just finalized a first version of tapir-http-session as an extension to akka-http-session available here resolvers += "Softnetwork releases" at "https://softnetwork.jfrog.io/artifactory/releases/" libraryDependencies += "app.softnetwork.tapir-http-session" %% "core" % "0.1.0" Let me know if you need any further information about it

adamw commented 1 year ago

Great! I've added a note to tapir's docs: https://github.com/softwaremill/tapir/commit/0978abb3e602fafbfd9e7f67fa7b8a92b001e509 and submitted the project to the next scala times issue.