Skip to content

Commit 90d837f

Browse files
zainfathoniampcode-com
zainfathoni
and
ampcode-com
committed
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>
1 parent 44c6138 commit 90d837f

5 files changed

Lines changed: 251 additions & 6 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/utils/__tests__/permissions.test.ts‎

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+
})

‎e2e/permission-redirect.spec.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { test, expect } from './base-test'
2+
import { authFixtures } from './fixtures'
3+
4+
test.describe('Permission redirect for insufficient permissions', () => {
5+
test.describe('Regular member accessing author-only routes', () => {
6+
test.use({
7+
storageState: authFixtures.member,
8+
})
9+
10+
test('redirects to dashboard home when accessing /dashboard/transactions', async ({
11+
page,
12+
}) => {
13+
await page.goto('/dashboard/transactions')
14+
15+
await page.waitForURL('**/dashboard')
16+
expect(page.url()).not.toContain('/dashboard/transactions')
17+
expect(page.url()).toContain('/dashboard')
18+
})
19+
20+
test('redirects to dashboard home when accessing /dashboard/transactions?status=submitted', async ({
21+
page,
22+
}) => {
23+
await page.goto('/dashboard/transactions?status=submitted')
24+
25+
await page.waitForURL('**/dashboard')
26+
expect(page.url()).not.toContain('/dashboard/transactions')
27+
})
28+
})
29+
30+
test.describe('Author accessing author-only routes', () => {
31+
test.use({
32+
storageState: authFixtures.author,
33+
})
34+
35+
test('allows access to /dashboard/transactions', async ({ page }) => {
36+
await page.goto('/dashboard/transactions')
37+
38+
await expect(
39+
page.getByRole('heading', { name: 'Transaksi' })
40+
).toBeVisible()
41+
expect(page.url()).toContain('/dashboard/transactions')
42+
})
43+
})
44+
45+
test.describe('Admin accessing all routes', () => {
46+
test.use({
47+
storageState: authFixtures.admin,
48+
})
49+
50+
test('allows access to /dashboard/transactions', async ({ page }) => {
51+
await page.goto('/dashboard/transactions')
52+
53+
await expect(
54+
page.getByRole('heading', { name: 'Transaksi' })
55+
).toBeVisible()
56+
expect(page.url()).toContain('/dashboard/transactions')
57+
})
58+
})
59+
})

0 commit comments

Comments
 (0)
⚡