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 for #71 #72

Merged
merged 8 commits into from
Aug 13, 2023
Merged

Fix for #71 #72

merged 8 commits into from
Aug 13, 2023

Conversation

acupofjose
Copy link
Collaborator

@acupofjose acupofjose commented Aug 9, 2023

Provides breaking changes to fix #71.

Overview:
Client.SetSession will do the following:

  1. Will destroy the current session (if existing)
  2. Raise a AuthState.SignedOut event.
  3. Decode token
    3a. If expired (or forceAccessTokenRefresh set), force an access token refresh.
    3b. If not expired, set the CurrentSession and retrieve CurrentUser from the server using the accessToken.
  4. Raise a AuthState.SignedIn event if successful.

Non-Breaking Changes:

  • Clarifies naming of parameters in StatelessClient, moving from jwt to serviceRoleToken when needed.

Minor Breaking changes:

  • Removes RefreshToken(string refreshToken) and SetAuth(string accessToken in favor of SetSession(string accessToken, string refreshToken)
  • Makes RefreshAccessToken require accessToken and refreshToken as parameters - overrides the authorization headers to use the supplied token
  • Migrates project internal times to use DateTime.UtcNow over DateTime.Now.

… parameters - overrides the authorization headers to use the supplied token
…sToken` in favor of `SetSession(string accessToken, string refreshToken)`
…t likely a case where this would be called.
@xaguzman
Copy link

xaguzman commented Aug 9, 2023

Just wondering, will this refresh the token? I ask because I see that now providing a refreshToken is required as well

Edit:

I believe the fact that refresh token is being called and a new JWT token being created will not yield the expected results.
Imagine the following scenario where we have an API (no actual client):

  1. User calls /api/signup (example endpoint). My aspnet core application will do a supabase.auth.signup(email,password)
  2. User verifies email
  3. User calls /api/signin (example endpoint). My aspnet core application will do a supabase.auth.signIn(email, password) call, and provide the user with a JWT token (this jwt will be used to allow access to my endpoints via the [Authorize] attribute). This will return the auth token (the jwt generated by supabase.auth) to the api consumer.

Up to this point everything is alright.

After this, user calls (again, example endpoint) /api/mybooks providing the jwt token generated by Supabase in as a Authorization: bearer <jwt> header to my aspnet core site.

If the session is refreshed and a new jwt token is generated everytime the authorization header is updated, that means that the api would need to constantly return the newly generated token (and refreshToken) to the api consumer.

The jwt should not need be regenerated as long as it's still valid / not expired in order to use supabase's auth in a convenient way. C# is mostly used as a backend language and this does sound to me like a very very common usecase when using an auth provider.

Any thoughts?

@acupofjose
Copy link
Collaborator Author

Agreed. Checking the expiration before renewing is no big deal! I’ll add that in this morning. Thanks!

@acupofjose
Copy link
Collaborator Author

@xaguzman - I've added your suggestion and updated the description to reflect those changes. Thoughts?

@xaguzman
Copy link

It seems good! Thank you @acupofjose

I was just thinking of something ... It might also be that you do not want your server to automatically refresh the session ... since the case I mentioned before could happen at random times....

I can still see how autorefreshing the token might be useful for some other type of applications...but is there a way in which we can just "forward" the jwt token as is? (just send it as the Authorization header to supabase). In the context of the aspnet server: the jwt middleware should take care of rejecting the request if the token is no longer valid (expired) ... It makes sense for your server to always respect this and assume that if the request made it through the token is valid ....

Do you think this makes sense?

@wiverson
Copy link
Collaborator

I think there are configurations where the refresh token has a limit on it to avoid replay attacks, not sure exactly how this flow would interact with this but depending on the config I could see some value in not burning the refresh token.

Stepping back for a moment, I think you might want to just use the Supabase JavaScript client to handle the log in/log out/sign up stuff instead of trying to pass everything through your C# server. You're basically reimplementing/proxying the GoTrue server APIs through your server app.

Instead, just use the Supabase JS client to handle this stuff, and then when you need to hit your server APIs that's when you pass along the JWT. I set up a listener for the JS client to drop the JWT in as a cookie on a login or as a header when calling your REST API via JS. Then you just look for the JWT as a cookie on your server, verify it and you are good to go.

I'm not sure if that makes sense, lmk thoughts. I do this for my Java Spring Boot stuff and it works quite well.

@wiverson
Copy link
Collaborator

Just took a look at the commits, good stuff. +1 for adding in JWT decoder and updating the JWT args.

@xaguzman
Copy link

I think there are configurations where the refresh token has a limit on it to avoid replay attacks, not sure exactly how this flow would interact with this but depending on the config I could see some value in not burning the refresh token.

Stepping back for a moment, I think you might want to just use the Supabase JavaScript client to handle the log in/log out/sign up stuff instead of trying to pass everything through your C# server. You're basically reimplementing/proxying the GoTrue server APIs through your server app.

Instead, just use the Supabase JS client to handle this stuff, and then when you need to hit your server APIs that's when you pass along the JWT. I set up a listener for the JS client to drop the JWT in as a cookie on a login or as a header when calling your REST API via JS. Then you just look for the JWT as a cookie on your server, verify it and you are good to go.

I'm not sure if that makes sense, lmk thoughts. I do this for my Java Spring Boot stuff and it works quite well.

While I agree that this is generally what we should do ... I do not see the possibility to just be proxying the go true server to be a waste, this allows for your API to be testable via api-only programs, such as Postman ...

Dont you think?

@wiverson
Copy link
Collaborator

You can mint/create JWTs using the secret key, so one way you can generate test users/test JWTs is just by minting your own in a local test environment. Depending on how you are testing things you can provide a test_user endpoint that generates a JWT that corresponds to a test user account. So for API testing you can expose (only on the dev server lol) an endpoint that returns a valid JWT with a valid test user account.

In this scenario you create a test_user REST endpoint. This endpoint generates a complete test user, including inserting a record into the user table, and then returns a minted JWT. Then you use this JWT for your automated test run.

Another option is to fire up a supabase instance via Docker and use that for testing. That's basically what the supabase cli does, and also this repo for testing (eg this script in this repo)

@acupofjose
Copy link
Collaborator Author

I think it would make sense to forward the token in the StatelessClient but that the StatefulClient should expect the session to be refreshable and do so if needed when you’re trying to set it.

Thoughts?

@xaguzman
Copy link

That sounds about right to me @acupofjose .

One question though, how is the stateless client supposed to be used? Can't see any mention or sample of it 😅

@wiverson That is definetly an option...but that looks to me to be very vendor-specific. If I do decide to move my authentication to another provider .. I would need to adjust my tests ... If I was able to just call my endpoints (even if they happen to be just proxies to some functionalities) I would effectively be adjusting my tests to one of my vendor's limitations instead of testing my application flow.

This is just a personal preference of course...but I preffer my sdks / clients to not be opinionated and allow me as the developer to choose the approach I prefer.

Let me know your thoughts :)

@wiverson
Copy link
Collaborator

RE: vendors, yeah. I guess the way I look at it either you write your own or you pick a vendor/API. GoTrue etc are all open source so in practice you can always look at the code to see what's going on. Plus if you wanted to you can self-host.

FWIW I keep the surface area for the JWTs pretty light, mainly validating and grabbing the user id. Compared to, say, switching dbs it's pretty small. There are a few other vendors that do auth-as-a-SaaS and IMHO they were way, way more intrusive and hard to deal with.

1 similar comment
@wiverson
Copy link
Collaborator

RE: vendors, yeah. I guess the way I look at it either you write your own or you pick a vendor/API. GoTrue etc are all open source so in practice you can always look at the code to see what's going on. Plus if you wanted to you can self-host.

FWIW I keep the surface area for the JWTs pretty light, mainly validating and grabbing the user id. Compared to, say, switching dbs it's pretty small. There are a few other vendors that do auth-as-a-SaaS and IMHO they were way, way more intrusive and hard to deal with.

@acupofjose acupofjose merged commit 7ccbe6e into master Aug 13, 2023
1 check passed
@acupofjose acupofjose deleted the dev/fix-71 branch August 13, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calling SetAuth does not actually set Authorization Headers for subsequent requests.
3 participants