Skip to content

Add Refresh Token System for OAuth Tokens#1997

Merged
nabeelio merged 8 commits intophpvms:mainfrom
arthurpar06:oauthTokensRefresh
Mar 13, 2025
Merged

Add Refresh Token System for OAuth Tokens#1997
nabeelio merged 8 commits intophpvms:mainfrom
arthurpar06:oauthTokensRefresh

Conversation

@arthurpar06
Copy link
Copy Markdown
Member

This pull request introduces several changes to enhance the OAuth token management system by adding a new service for refreshing tokens and updating related database schema and tests. The most important changes include creating a new OAuthService for token refresh, updating the database schema to rename a column, and modifying the OAuthController and UserOAuthToken model accordingly.

New Service for OAuth Token Refresh:

Database Schema Update:

Controller and Model Adjustments:

Cron Job and Service Provider:

Test Updates:

Note:

While VATSIM and Discord (like most OAuth providers) issue access tokens valid for 7 days, it seems that IVAO's tokens are only valid for 30 minutes. I’ve reached out to them to confirm this behavior. If that's the case, we’ll need a specific refresh strategy for IVAO. This is why this is still in draft - I need to figure that out first.

Closes #1995

@arthurpar06 arthurpar06 marked this pull request as draft February 9, 2025 10:28
@nabeelio
Copy link
Copy Markdown
Member

Generally, I believe they should be refreshed before they're used - so the token get's decoded and the expiry checked, and if it's within the time, it's refreshed, then saved and used. Maybe check the best practices to make sure this is ok

@arthurpar06
Copy link
Copy Markdown
Member Author

I emailed IVAO and they confirmed that atm the access token is only valid for 30min. I considered implementing a "refresh only when needed" system, but we can't do that because the refresh token also expires. If we don't use this refresh token to get a new access token before it expires, the user will have to relink their account...

According to the references they provided, I assume their refresh token is only valid for 24hrs...

They also mentioned that they might consider extending this duration in the future, but it's not a priority for them right now as they are focused on other matters.

Should we put IVAO aside for the moment and focus on Discord/VATSIM instead? I'm not sure how we can handle these tokens effectively if they expire very 30min and become completely invalid after 24hrs

@arthurpar06
Copy link
Copy Markdown
Member Author

Okay, so I added two methods to the OAuthService: refreshToken and refreshTokenIfExpired.

I also added a custom refresh policy for IVAO. Tokens will be refreshed every week. This will ensure that tokens don't remain expired for too long, but they won't always be valid. Therefore, people using IVAO tokens should always check for validity and refresh if required. A refresh will probably always be required.

@arthurpar06 arthurpar06 marked this pull request as ready for review February 16, 2025 17:39
@nabeelio nabeelio merged commit eb8bc63 into phpvms:main Mar 13, 2025
5 checks passed
@arthurpar06 arthurpar06 deleted the oauthTokensRefresh branch March 13, 2025 19:22
nabeelio added a commit that referenced this pull request May 7, 2025
* Version output fixes (#1954)

* Set the label to be blank

* Fix how the version is parsed when there's no pre-release version specified

* Update the latest metadata

* Correctly output the build metadata

* Update the version file

* Version Tweaks

* Set the mode and incrementals

* Tweaks to version file

* fix: fix hcaptcha sitekey tag (#1979)

* [7.x] Add log viewer (#1972)a

* add opcodesio/log-viewer

* set up with custom permission

---------

Co-authored-by: Nabeel S. <[email protected]>

* Fix pint workflow (#1974)

* [7.x] Add `access_admin` gate (#1975)

* Add access_admin gate

* Add example in seven theme nav

* Add missing ',' in email-plain.blade.php (#1987)

* Update airport IDs/ICAOs to enforce 8-character format (#1988)

* Update airport IDs/ICAOs to 8 chars

* Add nullable (L11 support)

* Update FlightImporter.php (#1983)

* Fix module menu icon

* Update FlightImporter.php

Closes #1982 

Use `0` instead of `null` to follow the `setDays()` function and have a proper match for `firstorNew()`

---------

Co-authored-by: Nabeel S. <[email protected]>

* Register UpdateSimBriefData Listener (#1999)

* Update default activity log retention time (#2000)

* Merge commit from fork

* Optimize JournalRepository (#2007)

* Remove vendor from .htaccess (#1994)

* Move policies

* update guessPolicyNamesUsing

* wip

* Run pint

* Fixes float -> int overflow problem in certain systems.

Fixes float -> int overflow problem in certain systems.

* Add Refresh Token System for OAuth Tokens (#1997)

* Add OAuthTokens refresh logic

* Add tests

* Run pint

* Update IVAO tokens policy

* Update tests

---------

Co-authored-by: Nabeel S. <[email protected]>

* Aircraft's FIN should be unique (#2015)

* Add Pre-Commit Hooks (#1993)

* Add pre commit hooks

* Fix hooks registration

* Fix hooks permissions

* Test pre commit hooks

* Update pint workflow

---------

Co-authored-by: Nabeel S. <[email protected]>

* Update PirepFiled.php (#2036)

Do not load relationships of the `pirep` model while using it for notifications.

* Update Notification (PirepFiled) (#2036)

* Update PirepFiled.php

Do not load relationships of the `pirep` model while using it for notifications.

* Update NotificationEventsHandler.php

Add `->withoutRelations()` call to outgoing mails

---------

Co-authored-by: Nabeel S. <[email protected]>

* Merge branch 'feature/8.0' (#2041)

* Run rector

* Run rector

* update packages

* refactor RolePolicy

---------

Co-authored-by: Nabeel S. <[email protected]>
Co-authored-by: Arthur Hetem <[email protected]>
Co-authored-by: B.Fatih KOZ <[email protected]>
Co-authored-by: Nabeel S. <[email protected]>
Co-authored-by: Taylor Broad <[email protected]>
Co-authored-by: Fikret Anıl Haksever <[email protected]>
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.

Add Refresh Token System for OAuth Tokens

2 participants