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

fix: signOut should ignore 403s #894

Merged
merged 4 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions infra/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
version: '3'
services:
gotrue: # Signup enabled, autoconfirm off
image: supabase/auth:v2.129.0
image: supabase/auth:v2.150.1
ports:
- '9999:9999'
environment:
Expand Down Expand Up @@ -42,7 +42,7 @@ services:
- db
restart: on-failure
autoconfirm: # Signup enabled, autoconfirm on
image: supabase/auth:v2.129.0
image: supabase/auth:v2.150.1
ports:
- '9998:9998'
environment:
Expand Down Expand Up @@ -71,7 +71,7 @@ services:
- db
restart: on-failure
disabled: # Signup disabled
image: supabase/auth:v2.129.0
image: supabase/auth:v2.150.1
ports:
- '9997:9997'
environment:
Expand Down Expand Up @@ -106,7 +106,7 @@ services:
- '9000:9000' # web interface
- '1100:1100' # POP3
db:
image: supabase/postgres:14.1.0
image: supabase/postgres:15.1.1.46
ports:
- '5432:5432'
command: postgres -c config_file=/etc/postgresql/postgresql.conf
Expand Down
7 changes: 6 additions & 1 deletion src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,12 @@ export default class GoTrueClient {
if (error) {
// ignore 404s since user might not exist anymore
// ignore 401s since an invalid or expired JWT should sign out the current session
if (!(isAuthApiError(error) && (error.status === 404 || error.status === 401))) {
if (
!(
isAuthApiError(error) &&
(error.status === 404 || error.status === 401 || error.status === 403)
)
) {
return { error }
}
}
Expand Down
3 changes: 2 additions & 1 deletion test/GoTrueApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,8 @@ describe('GoTrueAdminApi', () => {
})

expect(user).toBeNull()
expect(error?.message).toEqual('User not found')
expect(error?.message).toEqual('Token has expired or is invalid')
expect(error?.status).toEqual(403)
})

test('verifyOTP() with invalid phone number', async () => {
Expand Down
20 changes: 5 additions & 15 deletions test/GoTrueClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,6 @@ describe('GoTrueClient', () => {
expect(error).toBeNull()
expect(data.session).not.toBeNull()

/**
* Sign out the user to verify setSession, getSession and updateUser
* are truly working; because the signUp method will already save the session.
* And that session will be available to getSession and updateUser,
* even if setSession isn't called or fails to save the session.
* The tokens are still valid after logout, and therefore usable.
*/
await authWithSession.signOut()

const {
data: { session },
error: setSessionError,
Expand Down Expand Up @@ -363,7 +354,8 @@ describe('GoTrueClient', () => {
expect(data.session).toBeNull()
expect(data.user).toBeNull()

expect(error?.message).toEqual('Error sending confirmation sms: missing Twilio account SID')
expect(error?.message).toEqual('Unable to get SMS provider')
expect(error?.status).toEqual(500)
})

test('signUp() with phone', async () => {
Expand Down Expand Up @@ -950,7 +942,7 @@ describe('GoTrueClient with storageisServer = true', () => {
expect(warnings.length).toEqual(0)
})

test('getSession() emits one insecure warning', async () => {
test('getSession() emits insecure warning when user object is accessed', async () => {
const storage = memoryLocalStorageAdapter({
[STORAGE_KEY]: JSON.stringify({
access_token: 'jwt.accesstoken.signature',
Expand All @@ -969,14 +961,12 @@ describe('GoTrueClient with storageisServer = true', () => {
storage,
})

await client.getUser() // should suppress the first warning

const {
data: { session },
} = await client.getSession()

console.log('User is ', session!.user!.id)

const user = session?.user // accessing the user object from getSession should emit a warning
expect(user).not.toBeNull()
expect(warnings.length).toEqual(1)
expect(
warnings[0][0].startsWith(
Expand Down