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

Fix for "Google signin no longer works since 2.7.0 on Mac" #311 #313

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

viktor44
Copy link
Collaborator

Workaround for new problem.
session.webRequest.onBeforeSendHeaders( is never called for any session except default. session.webRequest.onBeforeRequest( works as expected.
Don't understand what I'm doing wrong. Any ideas would be very appreciated.
So for sessions with partition attribute we don't replace header and can't login with Google.

@viktor44 viktor44 added this to the 2.7.1 milestone Jan 19, 2024
@viktor44 viktor44 requested a review from magne4000 January 19, 2024 00:32
@magne4000
Copy link
Member

Here are some places you could check. My guess is that there is some part of the current code that only handles defaultSession, and would need to be adapted for all sessions.

  • export class SessionServiceImpl extends SessionService implements RPC.Interface<SessionService> {
    private session?: Electron.Session;
    constructor(uuid?: string, options?: SessionOptions) {
    super(uuid, { ready: false });
    this.initSession(options).then(session => {
    this.session = session;
    this.ready();
    });
    }
    async getUserAgent(): Promise<string> {
    const session = await this.getSession();
    return session.getUserAgent();
    }
    async getCookies(filter: Electron.CookiesGetFilter): Promise<Electron.Cookie[]> {
    const session = await this.getSession();
    return session.cookies.get(filter);
    }
    private async initSession(options?: SessionOptions): Promise<Electron.Session> {
    if (options && options.partition) {
    return Electron.session.fromPartition(options.partition);
    }
    return waitDefaultSession();
    }
    private async getSession(): Promise<Electron.Session> {
    await this.whenReady();
    return this.session!;
    }
    }
  • private async addOnRequestCompletedObserver(observer: RPC.ObserverNode<DownloadServiceObserver>) {
    await app.whenReady();
    const { defaultSession } = electronSession;
    if (defaultSession && observer.onRequestCompleted) {
    defaultSession.webRequest.onCompleted((details) => {
    if (isAttachmentAsMainframe(details)) {
    observer.onRequestCompleted!({
    ...details,
    hasEmptyHistory: hasEmptyHistory(details.webContentsId),
    });
    }
    });
    return () => defaultSession.webRequest.onCompleted(null as any);
    }
    return () => { };
    }
  • const defaultUserAgent = session.defaultSession?.getUserAgent();
  • const cookiesForUrl = async (url: string) => session.defaultSession!.cookies.get({ url });
  • session.defaultSession!.once('will-download', () => resolve(true));
  • getUserAgent(): Promise<string> {
    return services.defaultSession.getUserAgent();
    }
    async getCookies(manifestURL: string): Promise<any> {
    const state = this.store.getState();
    const installedApplications = getApplicationsByManifestURL(state, manifestURL);
    const tabs: RecursiveImmutableList<StationTab[]> = installedApplications
    .toList()
    .map(application => getTabsForApplication(state, getApplicationId(application)))
    .flatten(1) as any;
    const hostnames: string[] = tabs
    .map(getTabURL)
    .filter(Boolean)
    .map((tabUrl: string) => parseURL(tabUrl).hostname)
    .toJS() as any;
    const cookiesByHostname = await Promise.all(
    uniq(hostnames)
    .map((hostname: string) => {
    return services.defaultSession.getCookies({ domain: hostname });
    }),
    • If after investigation it appears that this is where it seems to be missing something, let me know. We will need to implement SessionService for all sessions, and not only for defaultSession.

@viktor44
Copy link
Collaborator Author

As I can see the SessionService is not widely used across the code and I don't see how it can helps here.
I'm talking about clean Electron event handlers.

One of them:

   session.webRequest.onBeforeRequest(
       (details: OnBeforeRequestListenerDetails, callback: (response: CallbackResponse) => void) => {
         console.log('AAAAAA', details.url, ' ||| ', session.getStoragePath());
         callback({ cancel: false });
       }
   );

is called as expected for each session (default and partitioned).

But this one:

  session.webRequest.onBeforeSendHeaders(
      (details: OnBeforeSendHeadersListenerDetails, callback: (beforeSendResponse: BeforeSendResponse) => void) => {
        details.requestHeaders['User-Agent'] = getUserAgentForApp(details.url, session.getUserAgent());

is called for default session only.

@viktor44
Copy link
Collaborator Author

viktor44 commented Jan 19, 2024

@magne4000 would you mind if I merge the PR?
The bug is quite critical and I want to release v2.7.1 because of it

@magne4000
Copy link
Member

@viktor44 ok, but if there are some new things critical breaking, we should reverse to always use defaultSession until a proper fix is found.

Have you managed to reproduce the issue with a fresh Electron boilerplate? Because it indeed could be an Electron issue in itself, but we need to be sure.

@viktor44 viktor44 merged commit 0a028c3 into getstation:main Jan 19, 2024
3 checks passed
@viktor44
Copy link
Collaborator Author

but if there are some new things critical breaking, we should reverse to always use defaultSession until a proper fix is found.

Agree. But IMO not now.
I've checked our top 30 apps and they work fine.

Have you managed to reproduce the issue with a fresh Electron boilerplate? Because it indeed could be an Electron issue in itself, but we need to be sure.

Not yet.

@viktor44
Copy link
Collaborator Author

Have you managed to reproduce the issue with a fresh Electron boilerplate? Because it indeed could be an Electron issue in itself, but we need to be sure.

I've created a small test and everything is working fine. The bug does not exists in the clean Electron. 😢
So the problem is somewhere in our code. That's a bad news. I have no idea how to trace it.

@magne4000
Copy link
Member

A few things to check:

  • We already had similar issues with cookies. Perhaps a comnbination of settings on the Window (or a missing one) could be the culprit.
  • enhanceWebRequest from electron-better-web-request could also break things, as it has not been updated for 5 years now.

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.

None yet

2 participants