Skip to content

Commit

Permalink
fix: signOut should ignore 403s (#894)
Browse files Browse the repository at this point in the history
  • Loading branch information
kangmingtay authored May 1, 2024
1 parent 927ae7a commit eeb77ce
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 21 deletions.
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

0 comments on commit eeb77ce

Please sign in to comment.