NextJS 'use server' Trap (Accidentally Leaking Data)

SdĂ­let
VloĆŸit
  • čas pƙidĂĄn 3. 07. 2024
  • Ever since React moved onto the server, the new directives "use client" and "use server" have caused some confusion. Lets look at how the top level directive in an actions.ts file may leak some data you may not want, and why this is more an issue of developer confusion rather than an issue with NextJS.
    Rhys Tweet: x.com/RhysSullivan/status/180...
    -------
    🐩 Twitter (X): jollycod.ing/x
    đŸ€“ Personal Site: jollycod.ing/me
    đŸ’» GitHub: jollycod.ing/git
    JollyUI: jollycod.ing/ui

Komentáƙe • 65

  • @deepjyotideb1173
    @deepjyotideb1173 Pƙed 22 dny +19

    Security threat starts from 9:30

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny +3

      Sorry, considering the mistake is geared more to beginners I think an explainer of the concepts is needed first

  • @user-qj4rr1rm8i
    @user-qj4rr1rm8i Pƙed 21 dnem +2

    Auth for every server action functions. I've got it.
    Thank you very much. Maybe you just saved my life.

  • @nasko235679
    @nasko235679 Pƙed 21 dnem +2

    That's basic javascript. You're exporting 2 separate functions. Even if they weren't separate endpoints the authentication check of one function is clearly outside the scope of the second. Another mistake is fetching data from server actions. Next have explicitly stated in their docs that server actions are only to be used for mutations. They get queued and can't run in parallel so if you start fetching with them and you have 2 different server actions for fetching data inside 1 component one has to wait for the other to complete. Instead fetching in server components can be done with simple helper functions that then get imported at the top level of the server component and fetching in client components (if you want it to be secure) still has to be done through api endpoints so the cookie can be send to the route handler and checked there.

  • @michaelfrieze
    @michaelfrieze Pƙed 23 dny +23

    I wish people would read the docs. Server Actions are not meant for fetching data. Regardless, you should always check for auth in any exported server action function and you should always check auth before fetching data.

    • @JollyCoding
      @JollyCoding  Pƙed 23 dny +6

      In an ideal world we would all read the docs!
      It is an interesting one, as I said it’s a trap for beginners and experts will be aware of this. In terms of reading the docs I think it’s up to educators to make sure this point is clear now.
      At the end of the day beginners will probably find a way to leak data somehow and learn from that, I just hope they have experts up the chain to teach them.

    • @michaelfrieze
      @michaelfrieze Pƙed 23 dny +2

      @@JollyCoding I saw a very popular next course on Udemy teaching to use server actions for data fetching. The instructor had no idea they were meant to be used for mutations.

    • @JohnMurphy04f
      @JohnMurphy04f Pƙed 22 dny

      ​@@michaelfriezewhos the instructor

    • @pjosxyz
      @pjosxyz Pƙed 22 dny +1

      @@michaelfrieze saw a udemy course teaching people to use floats for money too, oh lord

    • @MichaelKire
      @MichaelKire Pƙed 22 dny +1

      Server actions can be used for data fetching no problem. There is even several doc examples showing use with react/tanstack query etc. You should just not think that theres any connection between two different function that only has being in the same file in common, it’s literally just a way of defining endpoints

  • @HugoDuprez449
    @HugoDuprez449 Pƙed 21 dnem

    SvelteKit handles that separation of concern so much better

  • @user-ik7rp8qz5g
    @user-ik7rp8qz5g Pƙed 20 dny +1

    Or just store helper functions in functions.js file. It should work too, right?

  • @rawallon
    @rawallon Pƙed 23 dny +5

    Man, when react added hooks I seen a few arguments complaining they added footguns, but to me every "proof" felt like wasnt an actual use-case. Sadly this time I'm on the oposite camp, server actions feels so cool, but feels like there are oopsies that could happen and no one notice that makes me avoid it completely.
    Maybe it has something to do with experience, I know that a year down the line most we'll all just gotten used to its quirks, but it makes me scared because I know there are projects being started now, without knowledge of any of those quirks, and that will fall onto someone to sift trhough all of it...

    • @JollyCoding
      @JollyCoding  Pƙed 23 dny +2

      There’s a good tweet from Ryan Florence that says “I’ve seen people leak server stuff my entire career”. Just seems another area that we need to educate people on and be very careful with. Think the issue here is how fast this server stuff has moved and how much of a change it is for frontend devs.
      x.com/ryanflorence/status/1808538800005730612?s=46&t=5NfomTO38hUdG85aPtK2SA

  • @williandamascenomusic
    @williandamascenomusic Pƙed 21 dnem +3

    Is there some kind of middleware for server actions? Middlewares can potentially solve this problem.

  • @codePerfect
    @codePerfect Pƙed 23 dny +3

    Informative

  • @capability-snob
    @capability-snob Pƙed 22 dny

    Thanks for covering this. Making clear what the API is - and therefore where the security audit should happen - is really important. I worry about tools that hide this for the sake of convenience. I want this boundary to scream loudly at me so I know what calls can fail due to network partitions or leak data through their parameters or return values. This makes it look like I could maybe understand what the attack surface is with server components, with some effort.

  • @tolulopemalomo8922
    @tolulopemalomo8922 Pƙed 11 dny

    thanks for the great info!

  • @alaskandonut
    @alaskandonut Pƙed 22 dny

    Great! Thank you.

  • @creatorsremose
    @creatorsremose Pƙed 21 dnem

    Drinking game suggestion - whenever he says "go ahead", you take a shot. Bye bye liver.

  • @matthewnicastro2489
    @matthewnicastro2489 Pƙed 22 dny +2

    Just put auth in middleware like a chad

  • @isekaijoucho4812
    @isekaijoucho4812 Pƙed 21 dnem

    how could server side rendering leaks data? it is for rendering, nothing more

  • @oumardicko5593
    @oumardicko5593 Pƙed 22 dny +10

    the issue is: how the heck react became this complicated ?

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny +3

      A growing demand for performance and a move to a full stack framework. Really good talk by Ryan Florence here on the journey of it all and what reacts new role is. czcams.com/video/zqhE-CepH2g/video.htmlsi=ZreHb7iXEBAIDFeA

    • @oumardicko5593
      @oumardicko5593 Pƙed 22 dny +1

      @@JollyCoding oh thanks, i'll be sure to have a look

  • @ZiaCodes
    @ZiaCodes Pƙed 22 dny

    I don't use it here, I've a util function, I don't add everything under 'use server'

  • @BarakaAndrew
    @BarakaAndrew Pƙed 22 dny

    Everything that I said would happen is happening 😂😂
    Things JS devs would do not to split server and client code. I'm a golang backend, JS frontend guy, never mixing code, easy to secure, easy to code, easy to manage.

  • @salman0ansari
    @salman0ansari Pƙed 22 dny

    i don;t see how server actions are a trap.
    if you are not check auth when fetching data its your mistake clearly

  • @mrlectus
    @mrlectus Pƙed 21 dnem

    can't middleware solve this?

  • @abdelmounaimammari9064
    @abdelmounaimammari9064 Pƙed 22 dny

    I’ve found the use zsa very helpful on that matter. The pattern it introduced makes it almost impossible to make these kind of mistakes. Take a look at it as it’s game changing when working with server actions in NextJs

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny +1

      Will have to check that out thanks!

  • @gofullstack
    @gofullstack Pƙed 20 dny

    Reminds me of PHP

  • @greendsnow
    @greendsnow Pƙed 22 dny

    There's a fix: use the npm package called astro

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny

      Good choice, each has their own strengths!

  • @henriquematias1986
    @henriquematias1986 Pƙed 22 dny

    If you going to use server actions you better instead use tRPC

  • @adnanearef2959
    @adnanearef2959 Pƙed 22 dny

    This is not called a TRAP, please read the docs and dont ever use something without knowing exactly what it does.
    Server actions are one hell of a cool feature and I love them. The thing here is that you are explictly exporting two functions within a single "use server" directive.
    Which makes NextJS thinks that you are aware of what you are doing and proceeds to give you exactly what you are looking for. If you don't want to fall for that, then just export one single function and mark it as a default export instead. What I mean, is instead of exporting multiple action in the module, you can export only one default per module/file and that automatically solve your (issuem eventhough it is not an issue).
    One more thing you can do, is add your authentication in the middleware and you are GUCCI, no more similiar data leaking.
    Conclusion: Please stop blaming the framework for your mistakes, read the docs carefully and only when something behaves differently that it's described you can blame them.

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny

      To be clear I wasn’t blaming the framework, I was explaining a technique that it seems people have done that isn’t correct. This video educates them on that. I hope I was clear in the video this isn’t NextJS fault.
      The only thing they could change is tree shake the unused functions which will be a quality of life improvement

  • @zman-1x1
    @zman-1x1 Pƙed 22 dny

    This is so easy to mess up!

  • @denisblack9897
    @denisblack9897 Pƙed 21 dnem

    I dont get it, why use this server stuff anyway?
    You can pick up FastAPI in less than an hour and make a real backend that supports tons of new ai libraries, which your are definately making a wrapper around 😅

  • @masaeedi1
    @masaeedi1 Pƙed 22 dny

    That is why i use authentication in middleware. Problem solved.

  • @gopallohar5534
    @gopallohar5534 Pƙed 22 dny +2

    This should've been just a 1-2 minute video!!!!

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny +1

      Sorry, considering the mistake is geared more to beginners I think an explainer of the concepts is needed first

    • @gopallohar5534
      @gopallohar5534 Pƙed 22 dny +1

      @@JollyCoding yeah not 1-2 minute but the beginning of the video is unnecessary, you could have directly made the application which *has* the problem!!! The statement for the problem is really simple...

    • @gopallohar5534
      @gopallohar5534 Pƙed 22 dny

      @@JollyCoding btw can we use server actions to replace the /APIs ?
      I've seen this suggestion on many yt videos and tried and it works fine...
      I mean the page isn't server rendered, we call server actions on click or some other events (not only on form submition)

  • @dimitrisefstathiadis6562
    @dimitrisefstathiadis6562 Pƙed 21 dnem

    And how is this different from looking the damn Network Tab? Come on. Have y people build real apps? I don’t get the security issues with Server Actions. People put their Google API Keys in their code plain. Y can see them.. what’s the fuss about..?

  • @dimitrisefstathiadis6562
    @dimitrisefstathiadis6562 Pƙed 21 dnem

    Well, the problem is solved from just using the inline “use server”, and then inline the function. Problem solved! Drama again!😂

  • @kal.leroux
    @kal.leroux Pƙed 22 dny

    that's just people using the tool tge wrong way

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny

      Yes correct, but it is easy to see how people can fall into this. It’s that typical battle of being beginner friendly and stopping dangerous errors where we can, but at the same time not holding your hand too much that it limits advanced developers doing cool things and customisability

  • @krtirtho
    @krtirtho Pƙed 22 dny

    Regardless human stupidity this is actually a really stupid API design. Instead of supporting multi exports as different endpoints, Nextjs server actions should only accept a specifically named function. Like export function action() {} and ignore anything else in the file. It's the same concept as Vue's one component per file.
    This is needlessly complicated and less readable

    • @JollyCoding
      @JollyCoding  Pƙed 22 dny

      NestJS does this using decorators. Does remind me a lot of .Net controllers though!

  • @ticler
    @ticler Pƙed 23 dny +3

    React nerds leaking data? What's new about programmers without basic understanding of underlying protocols and technologies doing crappy programming?

    • @JollyCoding
      @JollyCoding  Pƙed 23 dny

      It’s always happened and will continue too. All part of the learning process, you just hope that someone experienced doesn’t let it get to production haha

    • @JarheadCrayonEater
      @JarheadCrayonEater Pƙed 22 dny

      That has absolutely nothing to do with a particular programming language, compiler, or framework.
      Absurd comment.

    • @lodosdurak7913
      @lodosdurak7913 Pƙed 22 dny

      Yea if they are not planning and writing in machine language I aint hiring

    • @RadTwin
      @RadTwin Pƙed 22 dny

      Always one of those people 😅

  • @ghostinplainsight4803
    @ghostinplainsight4803 Pƙed 21 dnem

    This is a job for decorators. It should be closed by default and opened with an explicit decorator.
    ```
    @serverAction
    @auth(USER)
    export const getUser = async () => {}
    export const getAllUsers = async () => {}
    ```