From 4e4b2732c8b791bdd40fbbdada5bc9e7bd26826c Mon Sep 17 00:00:00 2001 From: sleepyfran Date: Thu, 2 Jan 2025 13:40:34 +0100 Subject: [PATCH] Re: fix token refreshment So apparently the previous token refreshing logic didn't really work :^) We were not: - Properly executing the on token change callback (map -> tap change) - Properly encoding the auth change request (broadcast-request change) - Not forcing a token refresh (auth interface and MSAL change, Spotify does not support forcing) - Not using the managed runtime when retrieving the token from the cache! --- packages/core/types/src/model/broadcast-requests.ts | 2 +- packages/core/types/src/services/authentication.ts | 1 + .../onedrive-provider/src/msal-authentication.ts | 6 +++++- .../onedrive-provider/src/onedrive-provider.ts | 5 +++-- .../reauthentication/src/authentication-cache.ts | 12 ++++++++++++ .../reauthentication/src/authentication-refresher.ts | 4 ++-- 6 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/core/types/src/model/broadcast-requests.ts b/packages/core/types/src/model/broadcast-requests.ts index 5c3f7ef..8b349af 100644 --- a/packages/core/types/src/model/broadcast-requests.ts +++ b/packages/core/types/src/model/broadcast-requests.ts @@ -54,6 +54,6 @@ export class ProviderAuthInfoChanged extends S.Class( authInfo: AuthenticationInfo, }) { get [Serializable.symbol]() { - return ProviderStatusChanged; + return ProviderAuthInfoChanged; } } diff --git a/packages/core/types/src/services/authentication.ts b/packages/core/types/src/services/authentication.ts index eb3928b..c38d272 100644 --- a/packages/core/types/src/services/authentication.ts +++ b/packages/core/types/src/services/authentication.ts @@ -21,6 +21,7 @@ export type Authentication = { */ connectSilent: ( cachedCredentials: AuthenticationInfo, + forceRefresh?: boolean, ) => Effect.Effect; }; diff --git a/packages/infrastructure/onedrive-provider/src/msal-authentication.ts b/packages/infrastructure/onedrive-provider/src/msal-authentication.ts index c879e74..737d9fd 100644 --- a/packages/infrastructure/onedrive-provider/src/msal-authentication.ts +++ b/packages/infrastructure/onedrive-provider/src/msal-authentication.ts @@ -79,7 +79,10 @@ export const MsalAuthenticationLive = Layer.effect( return yield* handleResponse(authResult); }); - const connectSilent = (cachedCredentials: AuthenticationInfo) => + const connectSilent = ( + cachedCredentials: AuthenticationInfo, + forceRefresh = false, + ) => Effect.gen(function* () { const app = yield* msalAppRef.get; @@ -102,6 +105,7 @@ export const MsalAuthenticationLive = Layer.effect( app.acquireTokenSilent({ ...authRequest, account, + forceRefresh, }), catch: (e) => { console.error(e); diff --git a/packages/infrastructure/onedrive-provider/src/onedrive-provider.ts b/packages/infrastructure/onedrive-provider/src/onedrive-provider.ts index 209df4e..c381230 100644 --- a/packages/infrastructure/onedrive-provider/src/onedrive-provider.ts +++ b/packages/infrastructure/onedrive-provider/src/onedrive-provider.ts @@ -4,7 +4,7 @@ import { MediaProviderFactory, ProviderType, } from "@echo/core-types"; -import { Effect, Layer, Option } from "effect"; +import { Effect, Layer, Option, Runtime } from "effect"; import { MsalAuthentication } from "./msal-authentication"; import { Client, type ClientOptions } from "@microsoft/microsoft-graph-client"; import { createListRoot } from "./apis/list-root.graph-api"; @@ -23,6 +23,7 @@ export const OneDriveProviderLive = Layer.effect( Effect.gen(function* () { const authCache = yield* AuthenticationCache; const msalAuth = yield* MsalAuthentication; + const runtime = yield* Effect.runtime(); return MediaProviderFactory.of({ authenticationProvider: Effect.succeed(msalAuth), @@ -30,7 +31,7 @@ export const OneDriveProviderLive = Layer.effect( const options: ClientOptions = { authProvider: { getAccessToken: () => - Effect.runPromise( + Runtime.runPromise(runtime)( authCache.get(FileBasedProviderId.OneDrive).pipe( Effect.map(Option.getOrElse(() => fallbackAuthInfo)), Effect.map((authInfo) => authInfo.accessToken), diff --git a/packages/services/reauthentication/src/authentication-cache.ts b/packages/services/reauthentication/src/authentication-cache.ts index ab40bcc..b0a32a5 100644 --- a/packages/services/reauthentication/src/authentication-cache.ts +++ b/packages/services/reauthentication/src/authentication-cache.ts @@ -33,6 +33,11 @@ export const AuthenticationCacheLive = Layer.scoped( Stream.tap(({ providerId }) => Effect.logInfo(`Received update for ${providerId}, updating cache`), ), + Stream.ensuring( + Effect.logWarning( + "Stopped listening for auth updates, was this intended?", + ), + ), Stream.runForEach(({ providerId, authInfo }) => Ref.update(cache, (cache) => new Map(cache).set(providerId, authInfo)), ), @@ -43,6 +48,13 @@ export const AuthenticationCacheLive = Layer.scoped( get: (providerId) => cache.get.pipe( Effect.map((c) => Option.fromNullable(c.get(providerId))), + Effect.tap((result) => + Option.isNone(result) + ? Effect.logWarning( + `No token found in authentication cache for provider ${providerId}`, + ) + : Effect.void, + ), ), }); }), diff --git a/packages/services/reauthentication/src/authentication-refresher.ts b/packages/services/reauthentication/src/authentication-refresher.ts index b284a9a..7bfca9e 100644 --- a/packages/services/reauthentication/src/authentication-refresher.ts +++ b/packages/services/reauthentication/src/authentication-refresher.ts @@ -113,8 +113,8 @@ const refreshProvider = ( ); const authentication = provider.value.authentication; - yield* authentication.connectSilent(currentAuthInfo).pipe( - Effect.map((authInfo) => onTokenUpdated(metadata.id, authInfo)), + yield* authentication.connectSilent(currentAuthInfo, true).pipe( + Effect.tap((authInfo) => onTokenUpdated(metadata.id, authInfo)), Effect.tap(() => Effect.logInfo( `Token refreshed and stored in cache for provider ${metadata.id}`,