Add end-to-end tests for force_access_check_with_provider#1507
Add end-to-end tests for force_access_check_with_provider#1507nooreldeenmansour wants to merge 1 commit intomainfrom
force_access_check_with_provider#1507Conversation
Adds tests to verify the `force_access_check_with_provider` functionality: - Allows login with local password when the identity provider is reachable. - Denies login when the identity provider is unreachable. Also adds a new keyword to block outbound HTTPS using iptables to simulate an offline provider.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1507 +/- ##
==========================================
+ Coverage 86.19% 87.05% +0.85%
==========================================
Files 113 93 -20
Lines 7556 6443 -1113
Branches 111 111
==========================================
- Hits 6513 5609 -904
+ Misses 985 778 -207
+ Partials 58 56 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
adombeck
left a comment
There was a problem hiding this comment.
Not super happy that we keep adding small tests which all start with the time consuming and flaky device auth, but in this case I see how it makes sense to have separate test cases, given how the conditions (broker settings and network connectivity) are different in each.
As I also mentioned recently to @denisonbarbosa, adding support for creating snapshots during the tests would help: The first test which needs device auth could create a snapshot afterwards, and all subsequent tests which need it could restore that snapshot. I think Denison wanted to create a Jira ticket for that.
That would indeed be a great improvement, it would make the tests less flaky, and reduce the total CI time as well. |
| SSH.Execute sudo iptables -I OUTPUT -p tcp --dport 443 -j REJECT | ||
| SSH.Execute sudo ip6tables -I OUTPUT -p tcp --dport 443 -j REJECT |
There was a problem hiding this comment.
Couldn't instead use static resolver routes so that we can block only the requests going to a well know domain name rather than all the https traffic?
There was a problem hiding this comment.
Maybe, but Is there a specific test scenario where blocking only a single domain would be beneficial for our test plan? Since these tests run across multiple brokers (google and entra currently, and maybe we could replace google with the generic broker; perhaps keycloak), blocking all HTTPS feels simpler and more generic. It also gets automatically reverted after each test, each test starts on a clean snapshot. So there's no risk of it affecting subsequent ones. WDYT?
There was a problem hiding this comment.
I agree with Noor, it's not worth spending time on blocking only the traffic to the IdP because the e2e-test doesn't require any network traffic to work, and the VM is reset when the next test is started.
Adds two tests to verify the
force_access_check_with_providerfunctionality:Also adds a new keyword to block outbound HTTPS using
iptablesto simulate offline behaviorUDENG-9529
UDENG-9528