Manifest V3 update.

[kellenmurphy]

I took a stab at Issue #79. For the most part this was a straightforward, following the Chrome Migration Path.

Changes

• browserAction has been replaced by a more generic action.
• there are some permissions changes, namely that '<all_urls>' is now a host permission and that we don't need webRequestBlocking (see note below re: the reviseRedirectedRequestMethod issue) raised in #79 by @khlr.
• we now refer the background script as a "service worker" instead of a background script

Notes

• We do not want to merge this now to main, as Manifest V3 is unsupported by Firefox at the moment, though will be out later this year. I think it may make sense to keep this around in a feature branch until then, however, so I'm contributing the code I have now.

  • I checked and even my Firefox Dev Edition (104.0b3), which has an about:config option (extensions.manifestV3.enabled) to "support" MV3, doesn't yet support service workers... so we're kind of dead in the water with MV3 development at the moment.

• Re: the necessity of blocking permissions for reviseRedirectedRequestMethod

  • I took a look at where the requirement for this code came from in the original issue cited within the code, and referencing this StackOverflow post and I have to be completely honest here, try as a I might I was unable to replicate. Obviously, I'm not leveraging the versions of Chrome and FF from back then, but despite throwing many test cases of POSTs followed by GETs I did not see untracked requests within SAML-Tracer.

  • That said, that does not prove that we don't have an issue. The absence of evidence is not the evidence of absence. In particular, either (a) webRequest has changed since that time, or (b) I'm not replicating the issue correctly.

  • Re: (a), I'm not in a position to comment since I've not done this research. :wink:

  • Re: (b), I'm not in a position to comment since I'm the cause of that issue.

  • For what it's worth, I think there's a strong likelihood that I'm not replicating the previous issue correctly... but I will expound on what I did:

    • I tested with the https://wiki.surfnet.nl/ SP, as described in the original issue: https://github.com/SimpleSAMLphp/SAML-tracer/pull/23#issuecomment-344970423

    • I tested with my own Shibboleth and SimpleSAMLphp dev environment and forcing POSTs followed by GETs.

    • I tested with two Shibboleth IDPs that are configured as proxied instances to upstream IDPs... one a "production" quality system I maintain that upstreams to Okta, and another my development proxy instance running in Azure that's got a selectable selection of upstreams (Shibboleth, WSO2 IS, Azure, and Keycloak).

    • At no point with ANY of these was I able to "break" things insofar as non-tracked SAML assertions being logged.

• So while I do not want to say with 100% certainty that "it's working", I would be relatively comfortable stating that I have some degree of confidence that it's working, and that I think the next step is for some others to test to confirm my findings / validate that they can't replicate.

• And lastly, by "replicate" I should also clarify to mean that I cannot get these lines of code to activate under any of the test circumstances described above.

[khlr]

Thank you so much for this PR, @kellenmurphy! I really like your detailed explanations!

Regarding the reviseRedirectedRequestMethod-issue:

IIRC case 1 affected Chrome and Firefox. At least at that time. Since case 2 is/was a FF-only special treatment we can ignore it for the moment. So I focused on case 1. And yep, I can't reproduce it either! But have a try using Firefox (without your MV3-changes). Then you can activate the mentioned lines! Maybe have a try, too. Go here and then select Academisch Medisch Centrum (AMC). If you set a breakpoint within the if, you'll reach that line! But only in FF and not in Chrome...

Possibly Chrome changed "something" in the meantime... So maybe both cases are FF-only speacial treatments by now ¯\_(ツ)_/¯

Hence I think at the moment we can't clarify if altering the method will break something in the future...

[kellenmurphy]

@khlr great insights! I will check this out later this week to see if I can replicate, and I will also update the keyboard-shortcut code.

Thanks!!! I'll have more soon... (my week is very front-heavy).

[kellenmurphy]

Just an update... I'm still planning to work on this, I've just had to be away for a bit due to a death in the family. I'll be circling back to this soon!

[tvdijen]

My sincerest condolences @kellenmurphy ! First things first!

[kellenmurphy]

Thanks @tvdijen...

FWIW I just fixed the keyboard shortcut, but I do still need to do some digging in line with @khlr's comment above. We also obviously need to wait for Firefox to start supporting MV3.

[khlr]

Hello @kellenmurphy 👋

How are you? Do you think you'll find the time picking up this issue again?

[kellenmurphy]

Sure! I can probably get to that next week. 😀

Kellen Murphy @.***

---

From: Jan Köhler @.> Sent: Thursday, September 28, 2023 9:51:45 AM To: simplesamlphp/SAML-tracer @.> Cc: Kellen Murphy @.>; Mention @.> Subject: Re: [simplesamlphp/SAML-tracer] Manifest V3 update. (PR #80)

Hello @kellenmurphyhttps://github.com/kellenmurphy 👋

How are you? Do you think you'll find the time picking up this issue again?

— Reply to this email directly, view it on GitHubhttps://github.com/simplesamlphp/SAML-tracer/pull/80#issuecomment-1739251224, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHHM43YIFPNKCURY4FIFFVLX4V6HDANCNFSM55AWDX5Q. You are receiving this because you were mentioned.Message ID: @.***>

[kellenmurphy]

@khlr I'm picking this back up this week. I've rebased onto simplesamlphp/main to pick up the recently approved PC, and squashed the previous commits to clean up the PR (and amended the commit message text).

I'm planning to carve out some time this week to thoroughly test this... it's been over a year (!) since I last looked at this so I want to vet this thoroughly.

[kellenmurphy]

@khlr I inadvertently requested review. Disregard. Mis-click. See my last message this isn't ready to review yet.

[tvdijen]

It's back to where you were after my earlier rebase-mistake. I've kept a backup in the manifest-backup branch, just in case