Skip to content

Conversation

@mbentley
Copy link

@mbentley mbentley commented Oct 3, 2025

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

Skip the reload because it will cancel POST requests which breaks SSO/SAML auth

Motivation and Context

This fixes #1973

Screenshots

n/a

Checklist

  • My pull request is properly named
  • The changes respect the code style of the project (pnpm prepare-code)
  • pnpm test passes
  • I tested/previewed my changes locally

Release Notes

Fixes SSO/SAML authentication issues

@mbentley
Copy link
Author

mbentley commented Oct 4, 2025

I posted something similar in the issue and forgot to include it when I submitted the PR- I have no idea if this will break other stuff and in full transparency, I used Claude Code to figure out why this was happening.

I took a look at the code was originally added in 2ad39ff just to try to understand why it was added in the first place. I did test to see if TickTick authentication still works and it does so unless it was more than the auth, it seems to be good.

I had Claude do some analysis of why this shouldn't break things (as if the excessive emoji use didn’t give it away).

What that commit did:
Added chromeless user agent for Google login (removes Chrome version)
Added the forwardingHack parameter with loadURL() to reload the page
This was needed because TickTick requires Chrome version, but Google blocks it
The problem with the original approach:
The loadURL() reload was a workaround to apply the user agent change
It worked for simple GET requests
But it breaks POST requests (like SAML SSO) by canceling and reloading them as GET
Why Our Fix is Safe
What changed in Electron since 2021: The commit is from June 2021. Modern Electron (which Ferdium now uses) applies user agent changes immediately to ongoing navigation without needing a reload. Our fix:
✅ Still sets this.webview.userAgent = this.userAgent (line 90)
✅ Chromeless user agent still applies to Google accounts
✅ TickTick still gets the chromeless user agent it needs
❌ No more loadURL() that cancels POST requests
✅ SSO/SAML now works (as you confirmed!)

If further changes need to be made, I can take a look but this honestly isn’t my area of expertise.

@mbentley
Copy link
Author

mbentley commented Oct 4, 2025

I took at shot at simplifying things. I left it as it's own commit for now in case it needs to be backed out.

@Alphrag
Copy link
Member

Alphrag commented Oct 5, 2025

@mbentley Thanks a lot for looking into this and raising this PR. At first glance it seems very legitimate, but I would need to have a closer look at it to check it it could have side effects. In the meantime, can you just make sure to run the build+lint locally and push the fixes so that the builds can work through it?

@kris7t If you have a bit of time, I'd be happy to have your input on why was this implemented in the first place (since it seems this was you who made this), and if you think such a logic is still necessary? Thanks

@mbentley
Copy link
Author

mbentley commented Oct 5, 2025

Ah sorry about that, I was working from my laptop without all of the required tools, only using containers so the husky hooks weren't executing. Fixed the formatting and squashed the commits.

@Alphrag Alphrag requested a review from SpecialAro October 5, 2025 21:30
@kris7t
Copy link
Member

kris7t commented Oct 8, 2025

This was implemented I think because Google had some validation for the User-Agent string (they really don't like people log in to Google with Electron apps directly and they'd prefer we used OAuth) that this forwarding hack worked around. In the meantime, Google probably updated its validation, and now it seems Electron is working differently, so we can't use the same hack for handling forwarding...

Note that using OAuth (as required by Google) is impossible here, because that would only give Ferdium access to the user's Google data, but it wouldn't give the browser session inside Ferdium a valid Google session cookie (this separation of privileges is the point of OAuth). SSO with Google requires a valid Google session cookie inside the browser container.

@mbentley
Copy link
Author

I just wanted to check back in for any feedback on anything that potentially should be done differently or if this is good as is. I was reminded of this annoyance today, having to manually execute the workaround for SSO 😉

Copy link
Member

@SpecialAro SpecialAro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this but LGTM!

@Alphrag
Copy link
Member

Alphrag commented Nov 23, 2025

Just chipping in on that: I am afraid the removal of the chromelessUserAgent variable might have some unexpected side-effects. The reason why I am saying this is because the userAgent(..) method is called in other parts of the code, and in such cases, it will never receive the userAgentWithoutChromeVersion value with the proposed changes. So my question is then: are we sure that the will-navigate event will be fired every time we actually care about changing this user agent, and will it impact any other calls throughout the app?

I haven't been able to test it fully since I do not have access to the type of accounts having the problem, but using this fix hasn't affected anything on my side anyway. So if my remark above if of no concern, then I'm happy for this to be merged.

@mbentley
Copy link
Author

Sorry for the AI generated analysis - someone will want to double check these points but this is what Claude found:

"The userAgent(..) method is called in other parts of the code, and in such cases, it will never receive the userAgentWithoutChromeVersion value"

Places the userAgent getter is called:

File Line Code Purpose
ServiceWebview.tsx 145 useragent={service.userAgent} Initial webview HTML attribute
Service.ts 397 return this.userAgentModel.userAgent Passthrough getter
TodosScreen.tsx 40 userAgent={todosStore.userAgent} Initial todos webview attribute
todos/store.ts 76 return this.userAgentModel.userAgent Passthrough getter
  1. These look to be initial webview creation. The useragent attribute on <ElectronWebView> sets the user agent when the webview is first mounted. Google SSO happens during runtime navigation, not at mount time.

  2. The old code also set this.webview.userAgent directly. The getter was only used to compute the value:

    // OLD CODE
    this.chromelessUserAgent = true;
    this.webview.userAgent = this.userAgent;  // <-- Direct assignment to webview
  3. The fix does the exact same thing, just more directly:

    // NEW CODE
    this.webview.userAgent =
      this.serviceUserAgentPref || this.userAgentWithoutChromeVersion;
  4. The getter never "pushed" the value anywhere. MobX reactivity doesn't automatically update the webview's user agent property when chromelessUserAgent changes. The old code explicitly called this.webview.userAgent = this.userAgent after setting the boolean—the change just removes the intermediate state variable.


"Are we sure that the will-navigate event will be fired every time we actually care about changing this user agent?"

The old code relied on the exact same events

// OLD CODE
_addWebviewEvents(webview: ElectronWebView): void {
  this._willNavigateListener = event => this._handleNavigate(event.url, true);
  webview.addEventListener('will-navigate', this._willNavigateListener);

  this._didNavigateListener = event => this._handleNavigate(event.url);
  webview.addEventListener('did-navigate', this._didNavigateListener);
}

My code uses the exact same events

// NEW CODE
this._willNavigateListener = event => this._handleNavigate(event.url);
webview.addEventListener('will-navigate', this._willNavigateListener);

this._didNavigateListener = event => this._handleNavigate(event.url);
webview.addEventListener('did-navigate', this._didNavigateListener);

The Google SSO flow is a series of redirects. Every redirect triggers will-navigate before the request is sent, allowing the user agent to be set in time. The did-navigate event acts as a backup to catch any missed navigations.


Summary

Aspect Old Code New Code Change?
Events used will-navigate, did-navigate will-navigate, did-navigate No change
How user agent is applied this.webview.userAgent = this.userAgent this.webview.userAgent = <value> Same mechanism
Where getter is used Initial webview creation only Initial webview creation only No functional change

The concern stems from thinking the getter was doing something special. It wasn't—the getter was just a computed property. The actual user agent change always happened via direct assignment to this.webview.userAgent, which this fix preserves. The only behavioral change is removing the loadURL() call on will-navigate, which was causing the original bug (it cancelled POST requests needed for SAML authentication).

@Randomness0x
Copy link

Randomness0x commented Dec 3, 2025

I just tested this commit and it works for me for using Google workspace in combination with keycloak.
For Claude which also didn't work redirects me to an external browser, so this still fails. Even I set the option to open the link within Ferdium itself. But luckily Claude has a workaround by using a code sent to my e-mail.

Edit: Now running for a week and still no issues

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.

Google Mail not loading (Company login)

6 participants