Skip to content

Commit

Permalink
Re: fix token refreshment
Browse files Browse the repository at this point in the history
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!
  • Loading branch information
sleepyfran committed Jan 2, 2025
1 parent 6f346a1 commit 4e4b273
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/core/types/src/model/broadcast-requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ export class ProviderAuthInfoChanged extends S.Class<ProviderAuthInfoChanged>(
authInfo: AuthenticationInfo,
}) {
get [Serializable.symbol]() {
return ProviderStatusChanged;
return ProviderAuthInfoChanged;
}
}
1 change: 1 addition & 0 deletions packages/core/types/src/services/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type Authentication = {
*/
connectSilent: (
cachedCredentials: AuthenticationInfo,
forceRefresh?: boolean,
) => Effect.Effect<AuthenticationInfo, AuthenticationError>;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -102,6 +105,7 @@ export const MsalAuthenticationLive = Layer.effect(
app.acquireTokenSilent({
...authRequest,
account,
forceRefresh,
}),
catch: (e) => {
console.error(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -23,14 +23,15 @@ 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),
createMediaProvider: (fallbackAuthInfo) => {
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),
Expand Down
12 changes: 12 additions & 0 deletions packages/services/reauthentication/src/authentication-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
),
Expand All @@ -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,
),
),
});
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand Down

0 comments on commit 4e4b273

Please sign in to comment.