Skip to content

Commit 21a9d54

Browse files
feat(auth): auto-redirect after authentication (#212)
* feat(auth): auto-redirect after authentication When a user tries to access a protected route without being authenticated, the original URL is saved in session and they are redirected back after successful magic link login. Closes rb-logic.1 Amp-Thread-ID: https://ampcode.com/threads/T-019c17af-6967-74be-8e42-6191059b8d63 Co-authored-by: Amp <amp@ampcode.com> * test(auth): add unit and e2e tests for auto-redirect - Unit tests for redirect URL construction and preservation - E2E tests for unauthenticated redirect flow and authenticated redirect Amp-Thread-ID: https://ampcode.com/threads/T-019c17af-6967-74be-8e42-6191059b8d63 Co-authored-by: Amp <amp@ampcode.com> * feat(auth): redirect to dashboard for insufficient permissions - Add permission check in dashboard.transactions.$transactionId.$action loader - Standardize all permission redirects to /dashboard instead of various paths - Add comprehensive unit tests for permission functions - Add E2E tests for permission redirect behavior Closes rb-logic.2 Amp-Thread-ID: https://ampcode.com/threads/T-019c17de-be40-76a8-a962-78347f08fdda Co-authored-by: Amp <amp@ampcode.com> * fix(e2e): add admin fixture to global setup The permission-redirect.spec.ts tests use authFixtures.admin, but the global setup was not generating admin.local.json. This caused all Admin role tests to fail in CI with: 'ENOENT: no such file or directory, open e2e/fixtures/auth/admin.local.json' --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent 366b49b commit 21a9d54

10 files changed

Lines changed: 410 additions & 14 deletions

app/routes/dashboard.course.$lessonId.edit.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export const loader: LoaderFunction = async ({ request, params }) => {
2424
}
2525

2626
if (!requireCourseAuthor(user, course)) {
27-
return redirect(`/dashboard/course/${lessonId}`)
27+
return redirect('/dashboard')
2828
}
2929

3030
const lesson = await getLessonById(lessonId)
@@ -40,7 +40,7 @@ export const action: ActionFunction = async ({ request, params }) => {
4040
const course = await getFirstCourse()
4141

4242
if (!requireCourseAuthor(user, course)) {
43-
return redirect('/dashboard/transactions')
43+
return redirect('/dashboard')
4444
}
4545

4646
const { lessonId } = params

app/routes/dashboard.transactions.$transactionId.$action.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ import {
2828
} from '~/models/subscription'
2929
import { Field } from '~/components/form-elements'
3030

31-
export const loader: LoaderFunction = async ({ params }) => {
32-
// TODO: block if the current user is not an admin or the author of the course
31+
export const loader: LoaderFunction = async ({ request, params }) => {
32+
const user = await requireUser(request)
33+
const course = await getFirstCourse()
34+
35+
if (!requireCourseAuthor(user, course)) {
36+
return redirect('/dashboard')
37+
}
38+
3339
const { transactionId } = params
3440

3541
if (!transactionId) {
@@ -50,7 +56,7 @@ export const action: ActionFunction = async ({ request, params }) => {
5056
const course = await getFirstCourse()
5157

5258
if (!requireCourseAuthor(user, course)) {
53-
return redirect('/dashboard/transactions')
59+
return redirect('/dashboard')
5460
}
5561

5662
const { transactionId } = params

app/routes/dashboard.transactions.$transactionId.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export const loader: LoaderFunction = async ({ request, params }) => {
2323
const course = await getFirstCourse()
2424

2525
if (!requireCourseAuthor(user, course)) {
26-
return redirect('/dashboard/transactions')
26+
return redirect('/dashboard')
2727
}
2828

2929
const { transactionId } = params

app/routes/login.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,20 @@ import { auth } from '~/services/auth.server'
77
import { commitSession, getUserSession } from '~/services/session.server'
88

99
export const loader: LoaderFunction = async ({ request }) => {
10-
await auth.isAuthenticated(request, { successRedirect: '/dashboard' })
10+
const url = new URL(request.url)
11+
const redirectTo = url.searchParams.get('redirectTo') ?? '/dashboard'
12+
13+
await auth.isAuthenticated(request, { successRedirect: redirectTo })
1114
const session = await getUserSession(request)
1215
// This session key `auth:magiclink` is the default one used by the EmailLinkStrategy
1316
// you can customize it passing a `sessionMagicLinkKey` when creating an
1417
// instance.
1518
const error = session.get(auth.sessionErrorKey) as
1619
| { message: string }
1720
| undefined
21+
22+
session.set('redirectTo', redirectTo)
23+
1824
return json(
1925
{
2026
user: session.get('user'),
@@ -30,14 +36,17 @@ export const loader: LoaderFunction = async ({ request }) => {
3036
}
3137

3238
export const action: ActionFunction = async ({ request }) => {
39+
const url = new URL(request.url)
40+
const redirectTo = url.searchParams.get('redirectTo') ?? '/dashboard'
41+
3342
// The success redirect is required in this action, this is where the user is
3443
// going to be redirected after the magic link is sent, note that here the
3544
// user is not yet authenticated, so you can't send it to a private page.
3645
await auth.authenticate('email-link', request, {
37-
successRedirect: '/login',
46+
successRedirect: `/login?redirectTo=${encodeURIComponent(redirectTo)}`,
3847
// If this is not set, any error will be throw and the ErrorBoundary will be
3948
// rendered.
40-
failureRedirect: '/login',
49+
failureRedirect: `/login?redirectTo=${encodeURIComponent(redirectTo)}`,
4150
})
4251
}
4352

app/routes/magic.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import type { LoaderFunction } from '@remix-run/node'
22
import { auth } from '~/services/auth.server'
3+
import { getUserSession } from '~/services/session.server'
34

45
export const loader: LoaderFunction = async ({ request }) => {
6+
const session = await getUserSession(request)
7+
const redirectTo = (session.get('redirectTo') as string) || '/dashboard'
8+
59
await auth.authenticate('email-link', request, {
6-
// If the user was authenticated, we redirect them to their profile page
7-
// This redirect is optional, if not defined the user will be returnted by
8-
// the `authenticate` function and you can render something on this page
9-
// manually redirect the user.
10-
successRedirect: '/dashboard',
10+
// If the user was authenticated, we redirect them to their saved redirectTo
11+
// or dashboard as fallback.
12+
successRedirect: redirectTo,
1113
// If something failed we take them back to the login page
1214
// This redirect is optional, if not defined any error will be throw and
1315
// the ErrorBoundary will be rendered.
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { describe, it, expect } from 'vitest'
2+
3+
describe('Auth redirect URL construction', () => {
4+
it('should construct correct login URL with redirectTo param from pathname', () => {
5+
const request = new Request('http://localhost:3000/dashboard/courses')
6+
const redirectTo = new URL(request.url).pathname
7+
8+
const searchParams = new URLSearchParams([['redirectTo', redirectTo]])
9+
const loginUrl = `/login?${searchParams}`
10+
11+
expect(loginUrl).toBe('/login?redirectTo=%2Fdashboard%2Fcourses')
12+
})
13+
14+
it('should use custom redirectTo when provided', () => {
15+
const redirectTo = '/custom/path'
16+
17+
const searchParams = new URLSearchParams([['redirectTo', redirectTo]])
18+
const loginUrl = `/login?${searchParams}`
19+
20+
expect(loginUrl).toBe('/login?redirectTo=%2Fcustom%2Fpath')
21+
})
22+
23+
it('should handle root path', () => {
24+
const request = new Request('http://localhost:3000/')
25+
const redirectTo = new URL(request.url).pathname
26+
27+
const searchParams = new URLSearchParams([['redirectTo', redirectTo]])
28+
const loginUrl = `/login?${searchParams}`
29+
30+
expect(loginUrl).toBe('/login?redirectTo=%2F')
31+
})
32+
33+
it('should handle paths with query parameters', () => {
34+
const redirectTo = '/dashboard/courses?filter=active'
35+
36+
const searchParams = new URLSearchParams([['redirectTo', redirectTo]])
37+
const loginUrl = `/login?${searchParams}`
38+
39+
expect(loginUrl).toContain('redirectTo=')
40+
expect(decodeURIComponent(loginUrl)).toContain(
41+
'/dashboard/courses?filter=active'
42+
)
43+
})
44+
})
45+
46+
describe('Login redirect URL preservation', () => {
47+
it('should extract redirectTo from URL search params', () => {
48+
const url = new URL(
49+
'http://localhost:3000/login?redirectTo=/dashboard/profile'
50+
)
51+
const redirectTo = url.searchParams.get('redirectTo') ?? '/dashboard'
52+
53+
expect(redirectTo).toBe('/dashboard/profile')
54+
})
55+
56+
it('should default to /dashboard when redirectTo is not provided', () => {
57+
const url = new URL('http://localhost:3000/login')
58+
const redirectTo = url.searchParams.get('redirectTo') ?? '/dashboard'
59+
60+
expect(redirectTo).toBe('/dashboard')
61+
})
62+
63+
it('should preserve redirectTo in success/failure redirects', () => {
64+
const redirectTo = '/dashboard/transactions'
65+
const successRedirect = `/login?redirectTo=${encodeURIComponent(
66+
redirectTo
67+
)}`
68+
const failureRedirect = `/login?redirectTo=${encodeURIComponent(
69+
redirectTo
70+
)}`
71+
72+
expect(successRedirect).toBe(
73+
'/login?redirectTo=%2Fdashboard%2Ftransactions'
74+
)
75+
expect(failureRedirect).toBe(
76+
'/login?redirectTo=%2Fdashboard%2Ftransactions'
77+
)
78+
})
79+
})
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
import { User, Course, Subscription } from '@prisma/client'
2+
import { describe, it, expect } from 'vitest'
3+
import {
4+
requireAdmin,
5+
requireAuthor,
6+
requireCourseAuthor,
7+
requireActiveSubscription,
8+
} from '../permissions'
9+
import { ROLES, SUBSCRIPTION_STATUS } from '~/models/enum'
10+
import { UserWithSubscriptions } from '~/models/user'
11+
12+
const createMockUser = (overrides: Partial<User>): User =>
13+
({
14+
id: '1',
15+
createdAt: new Date(),
16+
updatedAt: new Date(),
17+
name: 'Test User',
18+
email: 'test@example.com',
19+
phoneNumber: null,
20+
telegram: null,
21+
instagram: null,
22+
role: ROLES.MEMBER,
23+
...overrides,
24+
} as User)
25+
26+
const createMockSubscription = (
27+
overrides: Partial<Subscription>
28+
): Subscription =>
29+
({
30+
id: 'sub-1',
31+
createdAt: new Date(),
32+
updatedAt: new Date(),
33+
authorId: 'author-1',
34+
courseId: 'course-1',
35+
userId: 'user-1',
36+
status: SUBSCRIPTION_STATUS.ACTIVE,
37+
...overrides,
38+
} as Subscription)
39+
40+
const createMockUserWithSubscriptions = (
41+
userOverrides: Partial<User>,
42+
subscriptions: Partial<Subscription>[] = []
43+
): UserWithSubscriptions => {
44+
return {
45+
...createMockUser(userOverrides),
46+
subscriptions: subscriptions.map((sub) => createMockSubscription(sub)),
47+
} as UserWithSubscriptions
48+
}
49+
50+
const createMockCourse = (overrides: Partial<Course>): Course =>
51+
({
52+
id: 'course-1',
53+
createdAt: new Date(),
54+
updatedAt: new Date(),
55+
name: 'Test Course',
56+
description: null,
57+
price: 100000,
58+
authorId: '2',
59+
...overrides,
60+
} as Course)
61+
62+
describe('Permission functions', () => {
63+
const adminUser = createMockUser({ id: '1', role: ROLES.ADMIN })
64+
const authorUser = createMockUser({ id: '2', role: ROLES.AUTHOR })
65+
const memberUser = createMockUser({ id: '3', role: ROLES.MEMBER })
66+
67+
const course = createMockCourse({ id: 'course-1', authorId: '2' })
68+
const otherCourse = createMockCourse({ id: 'course-2', authorId: '99' })
69+
70+
describe('requireAdmin', () => {
71+
it('returns true for admin user', () => {
72+
expect(requireAdmin(adminUser)).toBe(true)
73+
})
74+
75+
it('returns false for author user', () => {
76+
expect(requireAdmin(authorUser)).toBe(false)
77+
})
78+
79+
it('returns false for member user', () => {
80+
expect(requireAdmin(memberUser)).toBe(false)
81+
})
82+
})
83+
84+
describe('requireAuthor', () => {
85+
it('returns true for admin user', () => {
86+
expect(requireAuthor(adminUser)).toBe(true)
87+
})
88+
89+
it('returns true for author user', () => {
90+
expect(requireAuthor(authorUser)).toBe(true)
91+
})
92+
93+
it('returns false for member user', () => {
94+
expect(requireAuthor(memberUser)).toBe(false)
95+
})
96+
})
97+
98+
describe('requireCourseAuthor', () => {
99+
it('returns true for admin regardless of course', () => {
100+
expect(requireCourseAuthor(adminUser, course)).toBe(true)
101+
expect(requireCourseAuthor(adminUser, otherCourse)).toBe(true)
102+
expect(requireCourseAuthor(adminUser)).toBe(true)
103+
})
104+
105+
it('returns true for author of the course', () => {
106+
expect(requireCourseAuthor(authorUser, course)).toBe(true)
107+
})
108+
109+
it('returns false for author of a different course', () => {
110+
expect(requireCourseAuthor(authorUser, otherCourse)).toBe(false)
111+
})
112+
113+
it('returns false for member user', () => {
114+
expect(requireCourseAuthor(memberUser, course)).toBe(false)
115+
})
116+
})
117+
118+
describe('requireActiveSubscription', () => {
119+
const memberWithActiveSubscription = createMockUserWithSubscriptions(
120+
{ id: '3', role: ROLES.MEMBER },
121+
[{ courseId: 'course-1', status: SUBSCRIPTION_STATUS.ACTIVE }]
122+
)
123+
124+
const memberWithInactiveSubscription = createMockUserWithSubscriptions(
125+
{ id: '3', role: ROLES.MEMBER },
126+
[{ courseId: 'course-1', status: SUBSCRIPTION_STATUS.INACTIVE }]
127+
)
128+
129+
const memberWithNoSubscription = createMockUserWithSubscriptions(
130+
{ id: '3', role: ROLES.MEMBER },
131+
[]
132+
)
133+
134+
const authorWithSubscriptions = createMockUserWithSubscriptions(
135+
{ id: '2', role: ROLES.AUTHOR },
136+
[]
137+
)
138+
139+
const adminWithSubscriptions = createMockUserWithSubscriptions(
140+
{ id: '1', role: ROLES.ADMIN },
141+
[]
142+
)
143+
144+
it('returns true for admin regardless of subscription', () => {
145+
expect(requireActiveSubscription(adminWithSubscriptions, course)).toBe(
146+
true
147+
)
148+
})
149+
150+
it('returns true for course author regardless of subscription', () => {
151+
expect(requireActiveSubscription(authorWithSubscriptions, course)).toBe(
152+
true
153+
)
154+
})
155+
156+
it('returns true for member with active subscription to the course', () => {
157+
expect(
158+
requireActiveSubscription(memberWithActiveSubscription, course)
159+
).toBe(true)
160+
})
161+
162+
it('returns false for member with inactive subscription', () => {
163+
expect(
164+
requireActiveSubscription(memberWithInactiveSubscription, course)
165+
).toBe(false)
166+
})
167+
168+
it('returns false for member with no subscription', () => {
169+
expect(requireActiveSubscription(memberWithNoSubscription, course)).toBe(
170+
false
171+
)
172+
})
173+
174+
it('returns false for member with subscription to different course', () => {
175+
expect(
176+
requireActiveSubscription(memberWithActiveSubscription, otherCourse)
177+
).toBe(false)
178+
})
179+
})
180+
})

0 commit comments

Comments
 (0)