Manifest V3 update. #80
Conversation
|
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. 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... |
There was a problem hiding this comment.
Please adjust the keyboard-shortcut:
"_execute_browser_action" to "_execute_action"
|
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! |
|
My sincerest condolences @kellenmurphy ! First things first! |
|
Hello @kellenmurphy 👋 How are you? Do you think you'll find the time picking up this issue again? |
|
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 @kellenmurphy<https://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 GitHub<#80 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHHM43YIFPNKCURY4FIFFVLX4V6HDANCNFSM55AWDX5Q>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
kellenmurphy
force-pushed
the
main
branch
2 times, most recently
from
46da198
to
441bf66
Compare
- manifest version updated to 3 - remove webRequestBlocking permissions - background scripts are now service_workers - browser_action has been rolled into generic action - fix keyboard shortcut per @khlr's [comment](simplesamlphp#80 (comment))
- manifest version updated to 3 - remove webRequestBlocking permissions - background scripts are now service_workers - browser_action has been rolled into generic action - fix keyboard shortcut per @khlr's [comment](simplesamlphp#80 (comment))
- manifest version updated to 3 - remove webRequestBlocking permissions - background scripts are now service_workers - browser_action has been rolled into generic action - fix keyboard shortcut per simplesamlphp#80 (comment) from @khlr's
|
@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. |
|
@khlr I inadvertently requested review. Disregard. Mis-click. See my last message this isn't ready to review yet. |
- manifest version updated to 3 - remove webRequestBlocking permissions - background scripts are now service_workers - browser_action has been rolled into generic action - fix keyboard shortcut per simplesamlphp#80 (comment) from @khlr's
|
It's back to where you were after my earlier rebase-mistake. |
I took a stab at Issue #79. For the most part this was a straightforward, following the Chrome Migration Path.
Changes
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.
about:configoption (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. 😉
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: Feature/convert to web extension #23 (comment)
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.