Skip to content

Add end-to-end tests for force_access_check_with_provider#1507

Open
nooreldeenmansour wants to merge 1 commit intomainfrom
e2e-force-provider
Open

Add end-to-end tests for force_access_check_with_provider#1507
nooreldeenmansour wants to merge 1 commit intomainfrom
e2e-force-provider

Conversation

@nooreldeenmansour
Copy link
Copy Markdown
Member

Adds two 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 offline behavior

UDENG-9529
UDENG-9528

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
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.05%. Comparing base (5809e03) to head (6142b66).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nooreldeenmansour nooreldeenmansour marked this pull request as ready for review May 7, 2026 04:53
Copy link
Copy Markdown
Contributor

@adombeck adombeck left a comment

Choose a reason for hiding this comment

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

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.

@nooreldeenmansour
Copy link
Copy Markdown
Member Author

The first test which needs device auth could create a snapshot afterwards, and all subsequent tests which need it could restore that snapshot

That would indeed be a great improvement, it would make the tests less flaky, and reduce the total CI time as well.

Comment on lines +176 to +177
SSH.Execute sudo iptables -I OUTPUT -p tcp --dport 443 -j REJECT
SSH.Execute sudo ip6tables -I OUTPUT -p tcp --dport 443 -j REJECT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

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.

3 participants