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

[Tracking] Validate token refresh issue across providers #1636

Closed
6 of 7 tasks
dac09 opened this issue Jan 13, 2021 · 16 comments · Fixed by #1627 or #2277
Closed
6 of 7 tasks

[Tracking] Validate token refresh issue across providers #1636

dac09 opened this issue Jan 13, 2021 · 16 comments · Fixed by #1627 or #2277

Comments

@dac09
Copy link
Contributor

dac09 commented Jan 13, 2021

This is a "meta issue" to track what auth providers have an issue, where the token is not automatically refreshed

Verified working

  • firebase
    automagically refreshes the token, when getToken is called and the token is expired
  • supabase
    needs client config. This needs to be configured by the user, not within RW.

Not verified:

  • goTrue

As is:

  • magicLink

Needs fix:

Related to: #1627, #1576, #1608, #927, #738


Help needed with:

a) Working out if this issue exists in each of the providers above
b) Comment on this issue if you find a unticked provider with their default expiry times
c) Working out if the auth client needs any extra code or configuration in the packages/src/authClients to refresh its token automatically when getToken is called, or if it is handled internally by the authClient (such as firebase)


How to reproduce

  • Log into your RW app
  • Wait for access token to expire (this will vary, based on your provider)
  • Navigate to another page that needs auth to fetch data

Very important! You won't notice the issue if you refresh the page, but its decent way of validating if the token refresh is handled for us. If you refresh and find that you got logged out, it is an indication that the client probably does not auto refresh the accessToken.

@dac09 dac09 linked a pull request Jan 13, 2021 that will close this issue
3 tasks
@dac09
Copy link
Contributor Author

dac09 commented Jan 13, 2021

Provider: supabase

It looks like supabase needs a config https://github.com/supabase/supabase-js/blob/a5dc5d252a1ec7c9c2c2a5682dd77868fe2c3a97/src/lib/SupabaseAuthClient.ts#L8

thanks @dthyresson for the link

And this is how its used

@thedavidprice
Copy link
Contributor

I'm fairly certain @cannikin confirmed the ✅ with Netlify Identity

@cannikin
Copy link
Member

cannikin commented Jan 13, 2021

I'm fairly certain @cannikin confirmed the ✅ with Netlify Identity

Oh yeah, sorry, Netlify Identity did not show any problems for me! But I don't know what/how it did that, I just clicked a link in the UI after an hour and it went to that new page as expected.

Is there something I can watch somewhere to get more info? Network tab in Web Inspector?

@dac09
Copy link
Contributor Author

dac09 commented Jan 13, 2021

For Netlify, you might be able to see the expiry time for your token in localStorage.

I briefly looked through the code earlier today, but unlike supabase I couldn't see where in the code it would refresh without explicitly calling the refresh function.

@cannikin
Copy link
Member

cannikin commented Jan 14, 2021

Here's my gotrue.user from LocalStorage (please don't hack me):

{
  "url":"/.netlify/identity",
  "token":{
    "access_token":"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVC<snip>",
    "token_type":"bearer",
    "expires_in":3600,
    "refresh_token":"xxx", // removed
    "expires_at":1610482607000
  },
  "id":"2c76074d-2c35-44ae-84e5-c78c65d701ce",
  "aud":"",
  "role":"",
  "email":"rob@prestonwernerventures.com",
  "confirmed_at":"2019-12-04T21:25:00Z",
  "invited_at":"2019-12-04T21:14:48Z",
  "app_metadata":{"provider":"email"},
  "user_metadata":{},
  "created_at":"2019-12-04T21:14:48Z",
  "updated_at":"2019-12-04T21:14:48Z"
}

Definitely a 3600 in there but it looks like it gets refreshed automatically?

@dac09
Copy link
Contributor Author

dac09 commented Jan 14, 2021

Looks good, I'm going to check it off the list :). Thanks Rob!

@AndrewLamYW
Copy link
Contributor

azureActiveDirectory token default expiry in an hour. After the token is expired, I will receive HTTP error code 500 from the GraphQL. @jeliasson may I ask how do you overcome this? 🙏

@dac09
Copy link
Contributor Author

dac09 commented Jan 18, 2021

Thank you for confirming @AndrewLamYW - #1627 may solve this. Any chance you could try the PR build please and see if the issue goes away for you?

@jeliasson
Copy link
Contributor

@AndrewLamYW If you can try the PR build as suggested by @dac09 that would be great. If it's not working, please let me know and I dive into it. Thanks 🙏🏻

@jeliasson
Copy link
Contributor

jeliasson commented Jan 27, 2021

@dac09 In regards to azureActiveDirectory as of 0.23.1-canary.10, it looks like the user is redirected back to the login page upon hitting a 500 with expired token (which is 3600 seconds). I'll look into renew the access token in #1672.

/cc @AndrewLamYW

Edit: Fixed in cab9e5f

@viperfx
Copy link
Contributor

viperfx commented Feb 4, 2021

Would it be possible to merge these changes in as each provider is done? rather than delay the fix. I am facing this very commonly on my project and I am holding on launch for this fix.

@dac09
Copy link
Contributor Author

dac09 commented Feb 4, 2021

@viperfx the change has already been merged here #1627

@dthyresson
Copy link
Contributor

dthyresson commented Feb 4, 2021

@viperfx FYI this issue #1672 is in https://github.com/redwoodjs/redwood/releases/tag/v0.24.0 (see notes and fixed list)

@viperfx
Copy link
Contributor

viperfx commented Feb 4, 2021

Oh! Thank you both for pointing that out! I will upgrade today!

@benatkin
Copy link

benatkin commented Apr 2, 2021

It might be worth doing this with CI. The integration test providers allow for serializing and deserializing cookies. This was the biggest slowdown for me in my first Redwood project, being logged out of Netlify Identity after an hour. If we could say that once you log in you stay logged in for at least a week (if you click remember me) and back that up with a clever CI workflow, it might make us more confident in Redwood Auth.

Another part of the puzzle is auth providers that require an SMS or email validation. For this we could log in manually, grab the tokens, and set the CI environment variable of the tokens. This is something we'd want to do with a bot account (search for robot here to see what I mean on GitHub). Accounts are also subject to login checks so it may be tricky with some providers such as Google, but still possible.

@dac09
Copy link
Contributor Author

dac09 commented Apr 11, 2021

Ok @dthyresson. Confirmed netlify and auth0 do not refresh their tokens.

jtoar pushed a commit that referenced this issue Oct 26, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [react-player](https://github.com/CookPete/react-player) | [`2.12.0`
->
`2.13.0`](https://renovatebot.com/diffs/npm/react-player/2.12.0/2.13.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-player/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-player/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-player/2.12.0/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-player/2.12.0/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>CookPete/react-player (react-player)</summary>

###
[`v2.13.0`](https://github.com/CookPete/react-player/blob/HEAD/CHANGELOG.md#v2130)

[Compare
Source](https://github.com/CookPete/react-player/compare/v2.12.0...v2.13.0)

- Fix
[#&#8203;1604](https://github.com/CookPete/react-player/issues/1604) -
FilePlayer does not work if I passed an array of urls
[`#1612`](https://github.com/cookpete/react-player/pull/1612)
- fix: `src` sttribute become "undefinded" if `url` is an array
[`#1648`](https://github.com/cookpete/react-player/pull/1648)
- Adding keepPlaying to other player types
[`#1639`](https://github.com/cookpete/react-player/pull/1639)
-   CI [`#1654`](https://github.com/cookpete/react-player/pull/1654)
- Swap out broken youtube URL
[`#1659`](https://github.com/cookpete/react-player/pull/1659)
- Add keepPlaying to seekTo
[`#1620`](https://github.com/cookpete/react-player/pull/1620)
- Added forceDisableHls option for FilePlayer
[`#1625`](https://github.com/cookpete/react-player/pull/1625)
- added onPlaybackQualityChange prop
[`#1636`](https://github.com/cookpete/react-player/pull/1636)
- Update the list of supported YouTube domains
[`#1599`](https://github.com/cookpete/react-player/pull/1599)
- Fix
[#&#8203;1604](https://github.com/CookPete/react-player/issues/1604) -
FilePlayer does not work if I passed an array of urls
([#&#8203;1612](https://github.com/CookPete/react-player/issues/1612))
[`#1604`](https://github.com/cookpete/react-player/issues/1604)
- Support Wisita URLs with query params
[`#1591`](https://github.com/cookpete/react-player/issues/1591)
- Support vimeo manage links
[`#1593`](https://github.com/cookpete/react-player/issues/1593)
- Update readme
[`90237f5`](https://github.com/cookpete/react-player/commit/90237f51d43fc63870b0e6d0c86f4497f97ca586)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/redwoodjs/redwood).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMS41IiwidXBkYXRlZEluVmVyIjoiMzcuMzEuNSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
jtoar pushed a commit that referenced this issue Nov 2, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [react-player](https://github.com/CookPete/react-player) | [`2.12.0`
->
`2.13.0`](https://renovatebot.com/diffs/npm/react-player/2.12.0/2.13.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-player/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-player/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-player/2.12.0/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-player/2.12.0/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>CookPete/react-player (react-player)</summary>

###
[`v2.13.0`](https://github.com/CookPete/react-player/blob/HEAD/CHANGELOG.md#v2130)

[Compare
Source](https://github.com/CookPete/react-player/compare/v2.12.0...v2.13.0)

- Fix
[#&#8203;1604](https://github.com/CookPete/react-player/issues/1604) -
FilePlayer does not work if I passed an array of urls
[`#1612`](https://github.com/cookpete/react-player/pull/1612)
- fix: `src` sttribute become "undefinded" if `url` is an array
[`#1648`](https://github.com/cookpete/react-player/pull/1648)
- Adding keepPlaying to other player types
[`#1639`](https://github.com/cookpete/react-player/pull/1639)
-   CI [`#1654`](https://github.com/cookpete/react-player/pull/1654)
- Swap out broken youtube URL
[`#1659`](https://github.com/cookpete/react-player/pull/1659)
- Add keepPlaying to seekTo
[`#1620`](https://github.com/cookpete/react-player/pull/1620)
- Added forceDisableHls option for FilePlayer
[`#1625`](https://github.com/cookpete/react-player/pull/1625)
- added onPlaybackQualityChange prop
[`#1636`](https://github.com/cookpete/react-player/pull/1636)
- Update the list of supported YouTube domains
[`#1599`](https://github.com/cookpete/react-player/pull/1599)
- Fix
[#&#8203;1604](https://github.com/CookPete/react-player/issues/1604) -
FilePlayer does not work if I passed an array of urls
([#&#8203;1612](https://github.com/CookPete/react-player/issues/1612))
[`#1604`](https://github.com/cookpete/react-player/issues/1604)
- Support Wisita URLs with query params
[`#1591`](https://github.com/cookpete/react-player/issues/1591)
- Support vimeo manage links
[`#1593`](https://github.com/cookpete/react-player/issues/1593)
- Update readme
[`90237f5`](https://github.com/cookpete/react-player/commit/90237f51d43fc63870b0e6d0c86f4497f97ca586)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/redwoodjs/redwood).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMS41IiwidXBkYXRlZEluVmVyIjoiMzcuMzEuNSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants