Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rsc-auth): Implement getRoles function in auth mw & update default ServerAuthState #10656

Merged
merged 29 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a9a9772
feat(rsc-auth): Implement extractRoles function in auth mw
dac09 May 21, 2024
34cec37
Bit more cleanup
dac09 May 21, 2024
940fa63
Bit more cleanup
dac09 May 21, 2024
b88b169
Update invokeMiddleware tests
dac09 May 21, 2024
910e9ad
Update supabase middleware tests
dac09 May 21, 2024
9dc27b1
Update dbAuth middleware tests
dac09 May 21, 2024
11b3712
Merge branch 'main' of github.com:redwoodjs/redwood into feat/extract…
dac09 May 21, 2024
3e22b82
Cleanup red lines in dbAuthMw tests
dac09 May 21, 2024
12e4eea
Add test for extractRoles for supa mw
dac09 May 21, 2024
71fd715
Cleanup createStreamingHandler
dac09 May 21, 2024
5f47249
Fix dbAuth middleware and update tests
dac09 May 22, 2024
dc7dd9c
Merge branch 'main' into feat/extract-role-authmw
dac09 May 22, 2024
a1a2caa
Merge branch 'main' of github.com:redwoodjs/redwood into feat/extract…
dac09 May 22, 2024
a730f93
Add changeset, update dbAuthReadme
dac09 May 22, 2024
f0390de
Merge branch 'feat/extract-role-authmw' of github.com:dac09/redwood i…
dac09 May 22, 2024
21df537
Rename extractRoles to getRoles
dac09 May 25, 2024
8728346
Implement default getRoles for dbAuth
dac09 May 27, 2024
85700ef
Update readme
dac09 May 27, 2024
854fbe0
Add default get roles for supabase
dac09 May 27, 2024
01544bc
Lint
dac09 May 27, 2024
33c6190
Merge branch 'main' into feat/extract-role-authmw
dac09 May 28, 2024
61f6dc1
Merge branch 'main' into feat/extract-role-authmw
dthyresson May 28, 2024
9618e8e
Fix tests to init supabase middleware
dthyresson May 28, 2024
783a76e
explain getRole usage in readme
dthyresson May 28, 2024
dc7ae9c
add how to set roles in supabase
dthyresson May 28, 2024
d2a2772
Update readme with more explanation on roles
dac09 May 28, 2024
5c06594
Merge branch 'main' into feat/extract-role-authmw
dac09 May 30, 2024
0672064
Merge branch 'main' into feat/extract-role-authmw
Tobbe May 30, 2024
ca79741
Merge branch 'main' into feat/extract-role-authmw
Tobbe May 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changesets/10656.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- feat(rsc-auth): Implement extractRoles function in auth mw & update default ServerAuthState (#10656) by @dac09

- Implement extractRoles function in supabase and dbAuth middleware
- Updates default serverAuthState to contain roles
- Make cookieHeader a required attribute
- Introduces new `clear()` function to remove auth state - just syntax sugar

## Example usage
```tsx
// In entry.server.tsx
export const registerMiddleware = () => {
// This actually returns [dbAuthMiddleware, '*']
const authMw = initDbAuthMiddleware({
dbAuthHandler,
getCurrentUser,
extractRoles: (decoded) => {
return decoded.currentUser.roles || []
}
})

return [authMw]
}
```
3 changes: 2 additions & 1 deletion packages/auth-providers/dbAuth/middleware/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ interface Props {
export const registerMiddleware = () => {
// This actually returns [dbAuthMiddleware, '*']
const authMw = initDbAuthMiddleware({
cookieName,
dbAuthHandler,
getCurrentUser,
// cookieName optional
// extractRoles optional
// dbAuthUrl? optional
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
MiddlewareResponse,
} from '@redwoodjs/vite/middleware'

import { middlewareDefaultAuthProviderState } from '../../../../../auth/dist/AuthProvider/AuthProviderState'
import type { DbAuthMiddlewareOptions } from '../index'
import { initDbAuthMiddleware } from '../index'
const FIXTURE_PATH = path.resolve(
Expand Down Expand Up @@ -82,6 +83,7 @@ describe('dbAuthMiddleware', () => {
return { id: 'mocked-current-user-1', email: 'user-1@example.com' }
}),
dbAuthHandler: vi.fn(),
extractRoles: vi.fn(() => ['f1driver']),
}
const [middleware] = initDbAuthMiddleware(options)

Expand Down Expand Up @@ -109,6 +111,15 @@ describe('dbAuthMiddleware', () => {
email: 'user-1@example.com',
id: 'mocked-current-user-1',
},
roles: ['f1driver'],
})

expect(options.extractRoles).toHaveBeenCalledWith({
currentUser: {
email: 'user-1@example.com',
id: 'mocked-current-user-1',
},
mockedSession: 'this_is_the_only_correct_session',
})

// Allow react render, because body is not defined, and status code not redirect
Expand Down Expand Up @@ -152,6 +163,8 @@ describe('dbAuthMiddleware', () => {
email: 'user-1@example.com',
id: 'mocked-current-user-1',
},
// No extract roles function, so it should be empty
roles: [],
})

// Allow react render, because body is not defined, and status code not redirect
Expand Down Expand Up @@ -500,6 +513,12 @@ describe('dbAuthMiddleware', () => {
})

describe('handle exception cases', async () => {
const unauthenticatedServerAuthState = {
...middlewareDefaultAuthProviderState,
cookieHeader: null,
roles: [],
}

beforeAll(() => {
// So that we don't see errors in console when running negative cases
vi.spyOn(console, 'error').mockImplementation(() => {})
Expand Down Expand Up @@ -578,7 +597,11 @@ describe('dbAuthMiddleware', () => {
expect(res).toBeDefined()

const serverAuthState = mwReq.serverAuthState.get()
expect(serverAuthState).toBeNull()
expect(serverAuthState).toEqual({
...unauthenticatedServerAuthState,
cookieHeader:
'session_8911=some-bad-encrypted-cookie;auth-provider=dbAuth',
})

expect(res?.toResponse().headers.getSetCookie()).toEqual([
// Expired cookies, will be removed by browser
Expand Down
7 changes: 5 additions & 2 deletions packages/auth-providers/dbAuth/middleware/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ export interface DbAuthMiddlewareOptions {
req: Request | APIGatewayProxyEvent,
context?: Context,
) => DbAuthResponse
extractRoles?: (decoded: any) => string[]
getCurrentUser: GetCurrentUser
}

export const initDbAuthMiddleware = ({
dthyresson marked this conversation as resolved.
Show resolved Hide resolved
dbAuthHandler,
getCurrentUser,
extractRoles,
cookieName,
dbAuthUrl = '/middleware/dbauth',
}: DbAuthMiddlewareOptions): [Middleware, '*'] => {
Expand Down Expand Up @@ -71,7 +73,7 @@ export const initDbAuthMiddleware = ({
try {
// Call the dbAuth auth decoder. For dbAuth we have direct access to the `dbAuthSession` function.
// Other providers will be slightly different.
const { currentUser } = await validateSession({
const { currentUser, decryptedSession } = await validateSession({
req,
cookieName,
getCurrentUser,
Expand All @@ -84,11 +86,12 @@ export const initDbAuthMiddleware = ({
hasError: false,
userMetadata: currentUser, // dbAuth doesn't have userMetadata
cookieHeader,
roles: extractRoles ? extractRoles(decryptedSession) : [],
})
} catch (e) {
// Clear server auth context
console.error('Error decrypting dbAuth cookie \n', e)
req.serverAuthState.set(null)
req.serverAuthState.clear()

// Note we have to use ".unset" and not ".clear"
// because we want to remove these cookies from the browser
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import {
middlewareDefaultAuthProviderState,
unauthenticatedServerAuthState,

Check failure on line 8 in packages/auth-providers/supabase/middleware/src/__tests__/createSupabaseAuthMiddleware.test.ts

View workflow job for this annotation

GitHub Actions / 🏗 Build, lint, test / ubuntu-latest / node 20 latest

'unauthenticatedServerAuthState' is defined but never used. Allowed unused vars must match /^_/u
// type ServerAuthState,
} from '@redwoodjs/auth'
import { authDecoder } from '@redwoodjs/auth-supabase-api'
Expand Down Expand Up @@ -33,8 +34,6 @@
}
})

// })

vi.mock('@redwoodjs/auth-supabase-api', () => {
return {
authDecoder: vi.fn(() => {
Expand Down Expand Up @@ -74,6 +73,12 @@
}

describe('createSupabaseAuthMiddleware()', () => {
const unauthenticatedServerAuthState = {
...middlewareDefaultAuthProviderState,
roles: [],
cookieHeader: null,
}

it('creates middleware for Supabase SSR auth', async () => {
const middleware = createSupabaseAuthMiddleware(options)
const request = new Request('http://localhost:8911', {
Expand All @@ -90,7 +95,7 @@
expect(result).toHaveProperty('status', 200)

const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toEqual(middlewareDefaultAuthProviderState)
expect(serverAuthState).toEqual(unauthenticatedServerAuthState)
})

it('passes through non-authenticated requests', async () => {
Expand All @@ -107,7 +112,7 @@
expect(result.body).toEqual('original response body')

const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toEqual(middlewareDefaultAuthProviderState)
expect(serverAuthState).toEqual(unauthenticatedServerAuthState)
})
it('passes through when no auth-provider cookie', async () => {
const middleware = createSupabaseAuthMiddleware(options)
Expand All @@ -127,7 +132,10 @@
expect(result.body).toEqual('original response body when no auth provider')

const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toEqual(middlewareDefaultAuthProviderState)
expect(serverAuthState).toEqual({
...unauthenticatedServerAuthState,
cookieHeader: 'missing-the-auth-provider-cookie-header-name=supabase',
})
})

it('passes through when unsupported auth-provider', async () => {
Expand All @@ -147,7 +155,10 @@
'original response body for unsupported provider',
)
const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toEqual(middlewareDefaultAuthProviderState)
expect(serverAuthState).toEqual({
...unauthenticatedServerAuthState,
cookieHeader: 'auth-provider=unsupported',
})
})

it('handles current user GETs', async () => {
Expand All @@ -171,7 +182,10 @@

expect(req.url).toContain('/middleware/supabase/currentUser')
const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toEqual(middlewareDefaultAuthProviderState)
expect(serverAuthState).toEqual({
...unauthenticatedServerAuthState,
cookieHeader: 'auth-provider=supabase',
})
})

it('authenticated request sets currentUser', async () => {
Expand Down Expand Up @@ -199,6 +213,52 @@
})
})

it('authenticated request sets userMetadata', async () => {
const optionsWithExtractRole: SupabaseAuthMiddlewareOptions = {
getCurrentUser: async () => {
return {
id: 1,
email: 'user-1@example.com',
user_metadata: { favoriteColor: 'yellow' },
}
},
extractRoles: vi.fn().mockReturnValue(['admin', 'editor']),
}

const middleware = createSupabaseAuthMiddleware(optionsWithExtractRole)
const request = new Request('http://localhost:8911/authenticated-request', {
method: 'GET',
headers: new Headers({
cookie: 'auth-provider=supabase;sb_access_token=dummy_access_token',
}),
})
const req = new MiddlewareRequest(request)

const result = await middleware(req, MiddlewareResponse.next())
expect(result).toBeDefined()
expect(req).toBeDefined()

expect(authDecoder).toHaveBeenCalledWith(
'auth-provider=supabase;sb_access_token=dummy_access_token',
'supabase',
expect.anything(),
)

const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toBeDefined()
expect(serverAuthState).toHaveProperty('currentUser')
expect(serverAuthState.isAuthenticated).toEqual(true)
expect(serverAuthState.userMetadata).toEqual({
favoriteColor: 'yellow',
})
expect(serverAuthState.roles).toEqual(['admin', 'editor'])

// Called with result of decoding the token
expect(optionsWithExtractRole.extractRoles).toHaveBeenCalledWith({
sub: 'abc123',
})
})

it('authenticated request sets userMetadata', async () => {
const optionsWithUserMetadata: SupabaseAuthMiddlewareOptions = {
getCurrentUser: async () => {
Expand Down Expand Up @@ -268,7 +328,11 @@
// when an exception is thrown, such as when tampering with the cookie,
//the serverAuthState should be cleared
const serverAuthState = req.serverAuthState.get()
expect(serverAuthState).toBeNull()
expect(serverAuthState).toEqual({
...unauthenticatedServerAuthState,
cookieHeader:
'auth-provider=supabase;sb-example-auth-token=dummy_access_token',
})

// the auth-provider cookie should be cleared from the response
const authProviderCookie = res.cookies.get('auth-provider')
Expand Down
4 changes: 4 additions & 0 deletions packages/auth-providers/supabase/middleware/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import { clearAuthState } from './util'

export interface SupabaseAuthMiddlewareOptions {
getCurrentUser: GetCurrentUser
extractRoles?: (decoded: any) => string[]
dthyresson marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Create Supabase Auth Middleware that sets the `serverAuthState` based on the Supabase cookie.
*/
const createSupabaseAuthMiddleware = ({
getCurrentUser,
extractRoles,
}: SupabaseAuthMiddlewareOptions) => {
return async (req: MiddlewareRequest, res: MiddlewareResponse) => {
const type = 'supabase'
Expand Down Expand Up @@ -68,6 +70,8 @@ const createSupabaseAuthMiddleware = ({
isAuthenticated: !!currentUser,
hasError: false,
userMetadata: userMetadata || currentUser,
cookieHeader,
roles: extractRoles ? extractRoles(decoded) : [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to think of a case where the decoded data from the authDecoder is needed to get roles -- and not just the currentUser.

I guess one doesn't have to set roles inside getCurrentUser but typically for the auth providers we do.

Though this does give some flexibility to define roles outside the currentUser .... which might make ABAC permission easier in the future. Ie - check a permission file/config or lookup db --- for if the username starts with admin , then then get the admin role.

Think will have to use it a bit and get the feel for it, but makes sense and can see ow it will work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for example, if you add roles in supabase user_metadata, it doesn't come through as currentUser.roles!

})
} catch (e) {
console.error(e, 'Error in Supabase Auth Middleware')
Expand Down
2 changes: 1 addition & 1 deletion packages/auth-providers/supabase/middleware/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const clearAuthState = (
res: MiddlewareResponse,
) => {
// Clear server auth context
req.serverAuthState.set(null)
req.serverAuthState.clear()

// clear supabase cookies
// We can't call .signOut() because that revokes all refresh tokens,
Expand Down
11 changes: 6 additions & 5 deletions packages/auth/src/AuthProvider/ServerAuthProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import React from 'react'
import type { AuthProviderState } from './AuthProviderState.js'
import { middlewareDefaultAuthProviderState } from './AuthProviderState.js'

export type ServerAuthState = AuthProviderState<never> & {
cookieHeader?: string
export type ServerAuthState = AuthProviderState<any> & {
// Intentionally making these keys non-optional (even if nullable) to make sure
// they are set correctly in middleware
cookieHeader: string | null | undefined
roles: string[]
}

const getAuthInitialStateFromServer = () => {
Expand Down Expand Up @@ -45,9 +48,6 @@ export const ServerAuthProvider = ({
value: ServerAuthState
children?: ReactNode[]
}) => {
// @NOTE: we "Sanitize" to remove cookieHeader
// not totally necessary, but it's nice to not have them in the DOM
// @MARK: needs discussion!
const stringifiedAuthState = `__REDWOOD__SERVER__AUTH_STATE__ = ${JSON.stringify(
sanitizeServerAuthState(value),
)};`
Expand All @@ -67,6 +67,7 @@ export const ServerAuthProvider = ({
</>
)
}

function sanitizeServerAuthState(value: ServerAuthState) {
const sanitizedState = { ...value }
// Remove the cookie from being printed onto the DOM
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-server/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export type GetCurrentUser = (
decoded: AuthContextPayload[0],
raw: AuthContextPayload[1],
req?: AuthContextPayload[2],
) => Promise<null | Record<string, unknown> | string>
) => Promise<null | Record<string, unknown>>
dthyresson marked this conversation as resolved.
Show resolved Hide resolved

export type GenerateGraphiQLHeader = () => string

Expand Down
Loading
Loading