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

_invokeWithStreamedResponse doesn't include auth information #634

Open
2 tasks done
bbauman1 opened this issue Jan 8, 2025 · 8 comments · May be fixed by #635
Open
2 tasks done

_invokeWithStreamedResponse doesn't include auth information #634

bbauman1 opened this issue Jan 8, 2025 · 8 comments · May be fixed by #635
Labels
bug Something isn't working
Milestone

Comments

@bbauman1
Copy link

bbauman1 commented Jan 8, 2025

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

I'm new to supabase so apologies if this is not a bug. When using _invokeWithStreamedResponse the supabaseClient on the backend is unable to assume the auth role of the user making the request. I believe this is because of the different URLSession being used, as described in the function documentation, but it wasn't clear if this is a known side affect.

Is there a recommended workaround when using this function? Should I pass the user_id in the function body and assume an admin level role in the backend, or is that an anti-pattern with supabase?

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Make a request with the regular invoke function
  2. Observe that user level auth header is sent
  3. Make the same request using _invokeWithStreamedResponse
  4. Observe that user level auth header is not sent

Expected behavior

Get the same authorization scopes as using the regular invoke function

Screenshots

System information

version 2.24.1

Additional context

Add any other context about the problem here.

@bbauman1 bbauman1 added the bug Something isn't working label Jan 8, 2025
@grdsdev
Copy link
Collaborator

grdsdev commented Jan 8, 2025

Hi, thanks for reporting this, indeed it is a side effect of not using the same URLSession, but this is also considered a bug, as there is a workaround I can do in code to make it work, I'll push a PR with the fix ASAP.

Thanks again.

@grdsdev grdsdev linked a pull request Jan 8, 2025 that will close this issue
@bbauman1
Copy link
Author

bbauman1 commented Jan 9, 2025

hey @grdsdev ive been using your commit in my builds and its generally working but sometimes my app will start to get 401 errors from supabase (jwt invalid apparently?). im not sure if its related to using streaming or not, my only edge function is using streaming.

my signup methods are to do either

                let session = try await supabase.auth.signInWithIdToken(
                    credentials: OpenIDConnectCredentials(
                        provider: .google,
                        idToken: idToken,
                        accessToken: accessToken
                    )
                )

or

                let session = try await supabase.auth.signInWithIdToken(
                    credentials: .init(
                        provider: .apple,
                        idToken: idToken
                    )
                )

and then on subsequent app restarts I check for auth state changes which is what is logging me back in

        .task {
            for await state in supabase.auth.authStateChanges {
                switch state.event {
                case .initialSession:
                    await handleLoginAuthState(state)
                case .signedIn:
                    await handleLoginAuthState(state)
                case .signedOut:
                    await handleLogout(state)
                case .passwordRecovery, .tokenRefreshed, .userUpdated, .userDeleted, .mfaChallengeVerified:
                    break
                }
            }
        }

Not sure if I am supposed to be holding onto the jwt in keychain myself or prompting any refreshes on my own before calling the edge function (am new to supabase). Just wanted to call out this error though. Its hard to reproduce but once it starts happening (getting 401 responses) it happens on every request. I am logged-in though when this is happening

@bbauman1
Copy link
Author

just updating - when I went to use my app this morning I was still signed in but got another 401 error when invoking the edge function. Then I quit the app and reopened it and the 401 errors went away

@grdsdev
Copy link
Collaborator

grdsdev commented Jan 10, 2025

@bbauman1 thanks for trying that commit.

Not sure if I am supposed to be holding onto the jwt in keychain myself or prompting any refreshes on my own before calling the edge function (am new to supabase).
No need to hold jwt, SDK should handle everything internally

Back on my PR, there are still some issues with that implementation, that's why it still marked as draft, I'm working on some edge cases.

@bbauman1
Copy link
Author

@grdsdev appreciate the update. do you think there will be a fix out this week? I'm also fine with a temporary unofficial workaround that I have to apply myself. just wondering because I'm trying to ship an app using streaming this week and want to know if I should set up a different server provider with that timeline

@grdsdev
Copy link
Collaborator

grdsdev commented Jan 13, 2025

@bbauman1 as a workaround, you can manually update functions's auth info before calling your edge function, as:

let accessToken = try await supabase.auth.session.accessToken
supabase.functions.setAuth(accessToken) // Make sure to `setAuth` before calling `_invokeWithStreamedResponse` method.
let response = supabase.functions._invokeWithStreamedResponse(...)

This will make sure that _invokeWithStreamedResponse method grab the latest access token to invoke function.

@bbauman1
Copy link
Author

perfect thank you!

@grdsdev
Copy link
Collaborator

grdsdev commented Jan 14, 2025

This fix unfortunately isn't as straight forward as I first imagined, that is one of the main reasons why the method is still marked as experimental, the way networking layer is structured in the lib, it doesn't account for streamed responses, so for that specific method I create a new URLSession and do all from scratch (without using HTTPClient, which every other API call uses).

To fix this the proper way, we need to add support for streaming responses to the HTTPClient, which will demand more time.

As for now, if anyone faces this issue, please use the workaround at #634 (comment) temporarily, until this gets properly fixed.

Thanks.

@grdsdev grdsdev added this to the v3 milestone Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants