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: do not send non-JWTs in Authorization header #1293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 14 additions & 3 deletions src/SupabaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ import {
DEFAULT_REALTIME_OPTIONS,
} from './lib/constants'
import { fetchWithAuth } from './lib/fetch'
import { stripTrailingSlash, applySettingDefaults } from './lib/helpers'
import {
stripTrailingSlash,
applySettingDefaults,
isJWT,
checkAuthorizationHeader,
} from './lib/helpers'
import { SupabaseAuthClient } from './lib/SupabaseAuthClient'
import { Fetch, GenericSchema, SupabaseClientOptions, SupabaseAuthClientOptions } from './lib/types'

Expand Down Expand Up @@ -96,6 +101,8 @@ export default class SupabaseClient<
this.storageKey = settings.auth.storageKey ?? ''
this.headers = settings.global.headers ?? {}

checkAuthorizationHeader(this.headers)

if (!settings.accessToken) {
this.auth = this._initSupabaseAuthClient(
settings.auth ?? {},
Expand Down Expand Up @@ -285,10 +292,14 @@ export default class SupabaseClient<
headers?: Record<string, string>,
fetch?: Fetch
) {
const authHeaders = {
Authorization: `Bearer ${this.supabaseKey}`,
const authHeaders: { [header: string]: string } = {
apikey: `${this.supabaseKey}`,
}

if (isJWT(this.supabaseKey)) {
authHeaders.Authorization = `Bearer ${this.supabaseKey}`
}

return new SupabaseAuthClient({
url: this.authUrl,
headers: { ...authHeaders, ...headers },
Expand Down
7 changes: 5 additions & 2 deletions src/lib/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @ts-ignore
import nodeFetch, { Headers as NodeFetchHeaders } from '@supabase/node-fetch'
import { isJWT } from './helpers'

type Fetch = typeof fetch

Expand Down Expand Up @@ -31,15 +32,17 @@ export const fetchWithAuth = (
const fetch = resolveFetch(customFetch)
const HeadersConstructor = resolveHeadersConstructor()

const defaultAccessToken = isJWT(supabaseKey) ? supabaseKey : null

return async (input, init) => {
const accessToken = (await getAccessToken()) ?? supabaseKey
const accessToken = (await getAccessToken()) ?? defaultAccessToken
let headers = new HeadersConstructor(init?.headers)

if (!headers.has('apikey')) {
headers.set('apikey', supabaseKey)
}

if (!headers.has('Authorization')) {
if (!headers.has('Authorization') && accessToken) {
headers.set('Authorization', `Bearer ${accessToken}`)
}

Expand Down
57 changes: 57 additions & 0 deletions src/lib/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,60 @@ export function applySettingDefaults<

return result
}

export const BASE64URL_REGEX = /^([a-z0-9_-]{4})*($|[a-z0-9_-]{3}$|[a-z0-9_-]{2}$)$/i

/**
* Checks that the value somewhat looks like a JWT, does not do any additional parsing or verification.
*/
export function isJWT(value: string): boolean {
if (value.startsWith('Bearer ')) {
value = value.substring('Bearer '.length)
}

value = value.trim()

if (!value) {
return false
}

const parts = value.split('.')

if (parts.length !== 3) {
return false
}

for (let i = 0; i < parts.length; i += 1) {
const part = parts[i]

if (part.length < 4 || !BASE64URL_REGEX.test(part)) {
return false
}
}

return true
}

export function checkAuthorizationHeader(headers: { [header: string]: string }) {
if (headers.authorization && headers.Authorization) {
console.warn(
'@supabase-js: Both `authorization` and `Authorization` headers specified in createClient options. `Authorization` will be used.'
)
Comment on lines +104 to +107
Copy link
Member

Choose a reason for hiding this comment

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

can we initialise the headers field using the Headers interface instead since it's case-insensitive? (https://developer.mozilla.org/en-US/docs/Web/API/Headers)


delete headers.authorization
}

const authorization = headers.Authorization ?? headers.authorization ?? null

if (!authorization) {
return
}

if (authorization.startsWith('Bearer ') && authorization.length > 'Bearer '.length) {
Copy link
Member

Choose a reason for hiding this comment

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

authorization.length > 'Bearer '.length

not immediately obvious what this check is for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it's making sure that we only check for a valid JWT if the authorization header actually has a value after "Bearer ".

It's worth noting that this check will not cover a scenario where someone only passes in "Bearer" (without a trailing space), but I'm guessing this whole scenario would be extremely rare anyway. Still, you might consider covering both.

if (!isJWT(authorization)) {
throw new Error(
'@supabase-js: createClient called with global Authorization header that does not contain a JWT'
)
}
}
}
Loading