[Tutorial/TypeScript] TS warning when no RBAC scheme is implemented

[Philzen]

Reporting this non-critical finding here as advised:

Discovered in https://redwoodjs.com/docs/tutorial/chapter4/authentication#an-admin-section

The auth.ts file generated by yarn rw setup auth dbAuth expects a roles property to exist on the user object. However, implementers may choose (like in this tutorial chapter) not to implement role-based access control.

In that case, we're getting

Property 'roles' does not exist on type '{ id: number; email: string; }'.ts(2339)

in three places in api/src/lib/auth.ts.

Maybe there is way to expand the interface InferredCurrentUser, making this an optional field?

[Philzen]

Just commited this chapter to Philzen/redwoodblog-tutorial-typescript@2c45a51 if wanna checkout the warning straightaway.

[Tobbe]

Thanks for the issue report @Philzen. Do you want to have a look at fixing this, or do you want me to do that?

[Philzen]

@Tobbe it would be an honor, however i'm still relatively new to TypeScript and how to fix this relatively complex / inferred type (at least in the most appropriate way) seems a bit over the top of my head currently, at the least i'm unsure which would be the correct spot.

If you have a clue, kindly go ahead with a fix.

[dac09]

Thanks again for yet another good issue @Philzen ✌️

This is expected behaviour, though I believe. We ship auth templates with the ability to handle roles, but there's no reason you have to use it - all you have to do is remove the hasRole function if you don't want roles in your project.

I think its a good thing that you get that TypeScript error - imagine trying to implement Roles, if we made it optional - you wouldn't be able to spot that Roles isn't being picked out from the DB.

[thedavidprice]

Off Topic
@Philzen your participation is 🤯 It would be a pleasure to have you join a future Contributors Meetup if you're interested:
https://community.redwoodjs.com/t/contributors-meetups-a-next-step-for-redwoodjs-participation/2470

DM me @thedavidprice on Twitter, Forums, or Discord. And/or email via my GH profile.

Most of all, thank you! 🙏

Someone else might have already invited you. Just know we're all super excited to have your help with these things.

[Tobbe]

This is expected behaviour, though I believe. We ship auth templates with the ability to handle roles, but there's no reason you have to use it - all you have to do is remove the hasRole function if you don't want roles in your project.

I think its a good thing that you get that TypeScript error - imagine trying to implement Roles, if we made it optional - you wouldn't be able to spot that Roles isn't being picked out from the DB.

I can't decide if I agree or not here 😆

I love the help TS gives you in spotting errors like what you describe above. But I also don't like it when generated code is throwing errors in my face 🙂 Always leaves me with questions like "Have I done something wrong?", "Is the generator broken?", "What do I need to do to fix this?" etc.

I need to go through the tutorial with a TS project to see this for myself to be able to give any good suggestions on how to handle this

[Tobbe]

Finally got around to try this all out in a TS run-through of the tutorial.

I totally see all the red squiggles. So maybe the easiest and least controversial option is to update the tutorial to instruct the user to just remove the contents of that function and just always make it return false.

Another option is to add a custom type guard. Something like this

const isRbacUser = (
  user: typeof context.currentUser
): user is typeof context.currentUser & { roles: string | string[] } =>
  'roles' in user

And then

export const hasRole = (roles: AllowedRoles): boolean => {
  if (!isAuthenticated() || !isRbacUser(context.currentUser)) {
    return false
  }

@dac09 Do you think adding a type guard is more of a disservice to the end user here?

EDIT:

Now that I actually added roles to the user db schema I'm getting a new error with my solution above

I need to think on this some more... Maybe we don't need to add rbac support from the start to the template. Or maybe we should make the tutorial add support for multiple roles from the start. I think that might get rid of this particular error.

[Philzen]

I had totally forgotten about the fact i had written this issue when i handed in #5856 today 😆
Guess i lost track of it, sorry for not following up earlier.

@Tobbe

I also don't like it when generated code is throwing errors in my face 🙂 Always leaves me with questions like "Have I done something wrong?", "Is the generator broken?", "What do I need to do to fix this?" etc

That's exactly where i'm coming from. The question "Is the RedwoodJS generator somehow broken?" should never come up in the DX, i guess that's what everybody is working for here. Imho it must not ever generate code that violates its own ESLint rules (see #5827) or worse (like in this case) generates invalid types that will make yarn rw check-types fail in order to build trust.

@dac09

imagine trying to implement Roles, if we made it optional - you wouldn't be able to spot that Roles isn't being picked out from the DB.

What about adding some JSDoc here, linking to the getCurrentUser function and giving the heads-up that roles will always be undefined unless included in the selection ... would that suffice?

[Tobbe]

FYI everyone: Discussion (also) continues in #5866