Eosu 404 auth token reauth p2p disconnects#1237
Eosu 404 auth token reauth p2p disconnects#1237JessTello wants to merge 3 commits intorelease-6.0.1from
Conversation
[Public] Release 6.0.0
- Added Connect AuthExpiration notify registration with minimal diagnostic logging - Cached the last Connect LoginOptions and credential type to support safe re-auth attempts - Implemented PUID change detection to log when the ProductUserId remains stable or changes mid-session - Introduced a reauth-in-progress guard to prevent concurrent Connect.Login storms during token refresh - Added a debug hook to force a Connect re-login using a fresh Epic IdToken (CopyIdToken -> Connect.Login)
| { | ||
| Log($"Connect login was not successful. ResultCode: {connectLoginData.ResultCode}", LogType.Error); | ||
| OnConnectLogin?.Invoke(connectLoginData); | ||
| onloginCallback?.Invoke(connectLoginData); |
There was a problem hiding this comment.
With this here onLoginCallback would be called twice (bc it's also in the finally block)
| onloginCallback?.Invoke(connectLoginData); |
There was a problem hiding this comment.
Fixed in the next commit. I removed the extra onloginCallback?.Invoke()
| public void StartConnectLoginWithOptions(Epic.OnlineServices.Connect.LoginOptions connectLoginOptions, | ||
| OnConnectLoginCallback onloginCallback) | ||
| { | ||
| Debug.Log($"[EOS][ConnectLogin] t={Time.realtimeSinceStartup:F1} puid(prev)={s_lastKnownProductUserId} cred={s_lastConnectCredentialType}"); |
There was a problem hiding this comment.
Logging in this file should use the Log() function because it's conditionally compiled out with the ENABLE_DEBUG_EOSMANAGER flag
There was a problem hiding this comment.
Fixed in the next commit. I switched to using the Log() wrapper instead of Debug.Log
| ConfigureConnectStatusCallback(); | ||
| ConfigureConnectExpirationCallback(connectLoginOptions); | ||
| ConfigureConnectExpirationCallback(); | ||
| OnConnectLogin?.Invoke(connectLoginData); |
There was a problem hiding this comment.
Moving the OnConnectLogin call into the finally block also cleans up the code above and is a nice way to make sure it's always called
There was a problem hiding this comment.
I centralized onloginCallback into the finally to guarantee single delivery. For OnConnectLogin, I kept the explicit invokes in each outcome branch to preserve the early-return flow and clarity, but I can refactor it into the finally as well (single invoke) if you prefer.
| public void StartConnectLoginWithOptions(Epic.OnlineServices.Connect.LoginOptions connectLoginOptions, | ||
| OnConnectLoginCallback onloginCallback) | ||
| { | ||
| Debug.Log($"[EOS][ConnectLogin] t={Time.realtimeSinceStartup:F1} puid(prev)={s_lastKnownProductUserId} cred={s_lastConnectCredentialType}"); |
There was a problem hiding this comment.
We shouldn't log PUID/Epic account ids, created a thread to discuss this further
There was a problem hiding this comment.
Updated in the next commit the remaining PUID/EAS identifier logs (including the legacy SetLocalProductUserId() message) so no user IDs are printed.
| var EOSConnectInterface = GetEOSConnectInterface(); | ||
| var addNotifyAuthExpirationOptions = new AddNotifyAuthExpirationOptions(); | ||
|
|
||
| ulong callbackHandle = EOSConnectInterface.AddNotifyAuthExpiration( |
There was a problem hiding this comment.
Not your change but we might as well fix the var name whilst here
| var EOSConnectInterface = GetEOSConnectInterface(); | |
| var addNotifyAuthExpirationOptions = new AddNotifyAuthExpirationOptions(); | |
| ulong callbackHandle = EOSConnectInterface.AddNotifyAuthExpiration( | |
| var eosConnectInterface = GetEOSConnectInterface(); | |
| var addNotifyAuthExpirationOptions = new AddNotifyAuthExpirationOptions(); | |
| ulong callbackHandle = eosConnectInterface.AddNotifyAuthExpiration( |
|
|
||
|
|
| /// | ||
|
|
||
|
|
| { | ||
| Debug.Log("[EOS][ReauthTest] Manual reauth triggered"); | ||
| RefreshConnectLoginWithFreshEpicIdToken(); | ||
| } |
There was a problem hiding this comment.
Test function needs removing (at least it appears to be unused)
| { | ||
| Debug.LogWarning( | ||
| $"{nameof(EOSManager)} Connect auth expired but last credential type was {s_lastConnectCredentialType}. " + | ||
| $"Plugin cannot refresh that token automatically. Game should re-fetch external token and call StartConnectLoginWithOptions again."); |
There was a problem hiding this comment.
Is there documentation for this? It's a bit of an unusual case where it handles it in certain circumstances
There was a problem hiding this comment.
Added handling for auth-expiration by credential type: we only auto-refresh when the last Connect credential is EpicIdToken (via fresh CopyIdToken -> Connect.Login). For other credential types we log a warning and require the game to re-fetch the external token and re-initiate StartConnectLoginWithOptions.
| $"[EOS][PUID] changed {s_lastKnownProductUserId} => {newPuid}. " + | ||
| "This can break P2P socket mapping. Refusing to overwrite silently."); | ||
| OnConnectLogin?.Invoke(connectLoginData); | ||
| return; |
There was a problem hiding this comment.
Let's discuss this in the thread
| private void OnConnectAuthExpiration(ref AuthExpirationCallbackInfo callbackInfo) | ||
| { | ||
| Debug.Log($"[EOS][AuthExp] fired t={Time.realtimeSinceStartup:F1} puid={s_lastKnownProductUserId} cred={s_lastConnectCredentialType}"); | ||
| lock (s_connectReauthLock) |
There was a problem hiding this comment.
This code should be always running on the main thread anyway because all callbacks will be fired from the thread where PlatformInterface.Tick() is called. Was this a safety measure or did that not happen?
There was a problem hiding this comment.
Removed the lock and replaced it with a simple reauth-in-progress guard to prevent concurrent reauth storms.
| } | ||
|
|
||
|
|
||
| private void RefreshConnectLoginWithFreshEpicIdToken() |
There was a problem hiding this comment.
The code in this function overlaps a lot with what's in StartConnectLoginWithEpicAccount().
With a little work you should be able to move most of the logic in that function to a new one and re-use it here? The other function is async but it could call GetUserLoginInfo upfront, so no issues there
There was a problem hiding this comment.
Agreed on the overlap. Since StartConnectLoginWithEpicAccount() already performs the fresh IdToken + Connect.Login flow, I updated RefreshConnectLoginWithFreshEpicIdToken() to be a thin wrapper that reuses that existing codepath (silent, no callback) and keeps the reauth-in-progress guard.
JessTello
force-pushed
the
eosu-404-auth-token-reauth-p2p-disconnects
branch
from
9d7ab44 to
1fa8dd5
Compare
- Updated new diagnostics to use the Log() wrapper instead of Debug.Log - Moved OnConnectLogin call into the finally block - Prevented onLoginCallback from being called twice - Removed Debug_RefreshConnectLoginWithFreshEpicIdToken() debug helper - Fixed variable name for better readability and consistency - Added handling for auth-expiration: if credential type isn't EpicIdToken, log warning and require game to re-fetch the token. - Added safety check to prevent silently overwriting the local PUID if a different ProductUserId is returned. - Reduced duplication by making RefreshConnectLoginWithFreshEpicIdToken() reuse the StartConnectLoginWithEpicAccount() flow where applicable.
JessTello
force-pushed
the
eosu-404-auth-token-reauth-p2p-disconnects
branch
from
1fa8dd5 to
7da495b
Compare
3 participants
Added Connect AuthExpiration notify registration with minimal diagnostic logging