redwood
redwood copied to clipboard
[RFC]: Add a authenticated props for Private Set
Summary
Add an authenticated
props to the Private Set that would redirect the user if he is authenticated.
Motivation
Currently, we only have the unauthenticated
props for Private Set, which works well in most cases. However, let's take the example of the "Signup" and "Signin" routes. These routes are only supposed to be accessible if the user is not logged in. Apart from using the "isAuthenticated" of the "isAuth" hook, it is not possible to protect the routes in question.
Detailed proposal
It would be a simple copy and paste of the unauthenticated
props logic, but with the condition reversed.
Are you interested in working on this?
- [X] I'm interested in working on this
Thanks for the RFC @Olyno!
If I remember correctly there is a more general RFC somewhere about protecting routes based on anything, not just auth status. I want to revisit that before I know what the best next step here is. It might have been an issue or PR here on GitHub, or maybe a community forum post. If you want to help me find it, that'd be great. Otherwise I'll look for it myself later today.
I'm not sure to see which RFC you're looking for, sorry 😅
I found it! Was on the forums
https://community.redwoodjs.com/t/feature-proposal-useguard-hook-for-checking-preconditions-on-page-render/2732/13
I'll read it and get back to you when I have thought about it all some more 🙂 @Olyno would be great if you could have a look as well and see if you think it would help for your use case
Ok, so reading that forum post again it's about this exact same use case! That's great 🙂 So the value of such a feature is getting clearer, but I'd still really like a few more use-cases before adding it to the framework I think.
For now, would something like this work for you? Possibly used as a wrapper component in a <Set>
around the Sign up/Sign in routes
const { currentUser } = useAuth()
if (currentUser) return <Redirect to={routes.userDashboard()} />
@Tobbe What if I have a landing splash page /
public and then a/dashboard
that needs auth. If I am already logged and authenticated and go to /
, I'd like them to be easily redirected to /dashboard
?
But /
needs to be public, too (I think). That might be different.
Ok, so reading that forum post again it's about this exact same use case! That's great slightly_smiling_face So the value of such a feature is getting clearer, but I'd still really like a few more use-cases before adding it to the framework I think.
For now, would something like this work for you? Possibly used as a wrapper component in a
<Set>
around the Sign up/Sign in routesconst { currentUser } = useAuth() if (currentUser) return <Redirect to={routes.userDashboard()} />
This is the most efficient way to do the redirection indeed. The problem is that it quickly becomes redundant if several pages use the same logic (e.g the /signin
and /signup
pages). That's why I suggested a new prop, or at the limit to rework the Private Set workflow (even if I am less favorable to this rework idea, because it would bring a breaking change).
To incorporate the redirect after a user has logged in and refrain them from ever being able to go back to those signup and login pages, this is what I do:
Routes.js
<Private unauthenticated="login" roles="admin">
<Set wrap={LoggedInLayout}>
<Route path="/admin/users" page={UsersPage} name="UsersPage" />
</Set>
</Private>
<Private unauthenticated="login" roles="user">
<Set wrap={LoggedInLayout}>
<Route path="/dashboard" page={DashboardPage} name="dashboard" />
</Set>
</Private>
<Set wrap={[LoggedOffLayout, AuthRedirect]}>
<Route path="/login" page={LoginPage} name="login" />
<Route path="/signup" page={SignupPage} name="signup" />
<Route path="/forgot-password" page={ForgotPasswordPage} name="forgotPassword" />
<Route path="/reset-password" page={ResetPasswordPage} name="resetPassword" />
</Set>
AuthRedirect.js
import { useAuth } from '@redwoodjs/auth'
import { Redirect, routes } from '@redwoodjs/router'
const AuthRedirect = ({ children }) => {
const { loading, isAuthenticated } = useAuth()
if (loading) {
return null
}
if (isAuthenticated) {
return <Redirect to={routes.dashboard()} />
}
return children
}
export default AuthRedirect
Thank you @ladderschool that's exactly how I meant you can do it!
If you want you can combine <Private>
and <Set>
Routes.js
<Set private unauthenticated="login" wrap={LoggedInLayout}>
<Route path="/dashboard" page={DashboardPage} name="dashboard" />
</Set>
<Set wrap={[LoggedOffLayout, AuthRedirect]}>
<Route path="/" page={HomePage} name="home" />
<Route path="/login" page={LoginPage} name="login" />
<Route path="/signup" page={SignupPage} name="signup" />
<Route path="/forgot-password" page={ForgotPasswordPage} name="forgotPassword" />
<Route path="/reset-password" page={ResetPasswordPage} name="resetPassword" />
</Set>
AuthRedirect.js
import { useAuth } from '@redwoodjs/auth'
import { Redirect, routes } from '@redwoodjs/router'
const AuthRedirect = ({ children }) => {
const { loading, isAuthenticated } = useAuth()
if (loading) {
return null
}
if (isAuthenticated) {
return <Redirect to={routes.dashboard()} />
}
return children
}
export default AuthRedirect
@dthyresson I think what you mentioned is also possible with what @ladderschool showed. I added it to my tweaked version of his solution above
@Olyno You said
The problem is that it quickly becomes redundant if several pages use the same logic (e.g the /signin and /signup pages).
Do you think this solution solves that problem? Or is it still too redundant?
Do you think this solution solves that problem? Or is it still too redundant?
I wasn't thinking of adding multiple layouts, so I thought I would have to check each time. By adding multiple layouts, it solves the redundancy problem :)
Hi there! Didn't know if it is proper to make another issue for this question/proposal, just stumbled upon this minor wishy thing: <Private unauthenticated="login" rolesMismatch="page404" roles="user"> I think the point is obvious, separate redirect for logged in yet unauthorized folks, is there a way we can implement something alike?
Oh sorry, Routes.js example:
<Private unauthenticated="login" rolesMismatch='page404' roles="user">
@NetLancer I think, again, the real solution is what's described in that forum post I linked above. So thanks for providing yet another usecase for it 🙂
And, also again, the workaround for now is to have a <Set>
with a redirect. It would check the user's roles (by destructuring the hasRole
function from the useAuth
hook), and redirect as appropriate.
Pardon me @Tobbe, the above workaround i'm trying to get it working:
So, unless the current user hasRole('admin')
<Set private unauthenticated={() => (hasRole('someuser') ? "home" : "login")}>
or
<Set private unauthenticated={hasRole('someuser') ? "home" : "login"}>
-with the above destructured hasRole in Routes.js, both don't do what i'm aiming at, could you please exemplify the solution, would be much grateful :)
If the user is unauthenticated they will never have any roles. So checking hasRole
in the unauthenticated
component is never going return anything useful.
Instead I'd do something like (untested)
<Set private unauthenticated="login" wrap={RolesRedirect}>
<Route ...
</Set>
// RolesRedirect.jsx
import { useAuth } from '@redwoodjs/auth'
import { Redirect, routes } from '@redwoodjs/router'
const RolesRedirect = ({ children }) => {
const { loading, hasRole } = useAuth()
if (loading) {
return null
}
if (hasRole('someuser')) {
return <Redirect to={routes.home()} />
} else if (hasRole('admin')) {
return <Redirect to={routes.admin()} />
}
return children
}
export default RolesRedirect
You'll have to adjust the roles you check for and where you redirect. With the implementation I showed here a user with none of the roles will get whatever route(s) is nested inside the Set
Thank u so much!
<Set wrap={RolesRedirect} private unauthenticated="login">
worked for me,
perhaps the order (wrap, unauthenticated) matters too