Fix Google SSO/SAML authentication; fixes #1973
Pre-flight Checklist
Please ensure you've completed all of the following.
- [X] I have read the Contributing Guidelines for this project.
- [X] I agree to follow the Code of Conduct that this project adheres to.
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
- [X] My pull request is properly named
- [X] The changes respect the code style of the project (
pnpm prepare-code) - [X]
pnpm testpasses - [X] I tested/previewed my changes locally
Release Notes
Fixes SSO/SAML authentication issues
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 https://github.com/ferdium/ferdium-app/commit/2ad39ffb1cb0d0e5f79d6948f798ca79ed73c76c 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.
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.
@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
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.
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.
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 😉
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.
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 |
-
These look to be initial webview creation. The
useragentattribute on<ElectronWebView>sets the user agent when the webview is first mounted. Google SSO happens during runtime navigation, not at mount time. -
The old code also set
this.webview.userAgentdirectly. The getter was only used to compute the value:// OLD CODE this.chromelessUserAgent = true; this.webview.userAgent = this.userAgent; // <-- Direct assignment to webview -
The fix does the exact same thing, just more directly:
// NEW CODE this.webview.userAgent = this.serviceUserAgentPref || this.userAgentWithoutChromeVersion; -
The getter never "pushed" the value anywhere. MobX reactivity doesn't automatically update the webview's user agent property when
chromelessUserAgentchanges. The old code explicitly calledthis.webview.userAgent = this.userAgentafter 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).