fix: possible panic if refresh token has a null session_id

supabase / auth

A JWT based API for managing users and issuing JWT tokens

https://supabase.com/docs/guides/auth

MIT License

1.55k stars 374 forks source link

fix: possible panic if refresh token has a null session_id #1822

Closed kangmingtay closed 3 weeks ago

kangmingtay commented 3 weeks ago

What kind of change does this PR introduce?

Prior to the auth.sessions table being created, some refresh tokens can contain a null session_id. In those cases, attempting to use those refresh tokens to obtain a new session will result in a panic.
This PR creates a new session for those refresh tokens that do not have a session_id to prevent panics from happening.

What is the current behavior?

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

coveralls commented 3 weeks ago

Pull Request Test Coverage Report for Build 11592000983

Details

8 of 13 (61.54%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.02%) to 57.143%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
internal/api/token_refresh.go	8	13	61.54%
<!--	Total:	8	13	61.54%	-->

Totals
Change from base Build 11555996021:	-0.02%
Covered Lines:	9564
Relevant Lines:	16737

we don't need to do the same check here because it's using forUpdate = true - the intention in this block is to retry accessing the refresh_tokens or sessions table after some time because the FOR UPDATE lock is being held by another transaction so it's acceptable for the session to not be found here

cstockton commented 3 weeks ago

👍