Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle HTTP HEAD requests by setting response status to 200 OK #6390

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

KD23243
Copy link
Contributor

@KD23243 KD23243 commented Jan 27, 2025

Root Cause

The issue arises when a user with a Microsoft Outlook email address, having "Office 365 Advanced Threat Protection" enabled, logs in using a magic link. After receiving the magic link email and clicking the sign-in button, they encounter an error.

Related Issues

Previous Behavior

This issue is caused by the SafeLink protection in Outlook/Office 365, which sends a HEAD request to the original link before redirecting to the browser. WSO2 IS processes the login flow in response to the HEAD request, completing the account login process prematurely. As a result, the magic link becomes invalid, and the user sees an error message when trying to sign in.

Before.mov

Changes in this Pull Request

This PR introduces a method override to handle HTTP HEAD requests in the CommonAuthenticationServlet class. The server will now respond with a status code of 200 OK to HEAD requests, allowing clients to check resource availability without fetching the full content.

Current Behavior

With the new handling of HTTP HEAD requests, when a user clicks the magic link in the email, the server will respond with a 200 OK status for the HEAD request before redirecting the user to the sign-in page. This prevents premature completion of the login flow, ensuring that the magic link remains valid and the user can successfully sign in without encountering the error.

After.mov

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.95%. Comparing base (3992a69) to head (8efdcaf).
Report is 225 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6390      +/-   ##
============================================
- Coverage     46.36%   45.95%   -0.41%     
- Complexity    14770    15040     +270     
============================================
  Files          1714     1731      +17     
  Lines        103938   106554    +2616     
  Branches      18630    19307     +677     
============================================
+ Hits          48195    48972     +777     
- Misses        48852    50572    +1720     
- Partials       6891     7010     +119     
Flag Coverage Δ
unit 30.34% <100.00%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KD23243 KD23243 marked this pull request as ready for review January 27, 2025 15:10
@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/13002694350


commonAuthenticationServlet.doHead(request, response);

verify(response).setStatus(HttpServletResponse.SC_OK);
Copy link
Contributor

@piraveena piraveena Jan 28, 2025

Choose a reason for hiding this comment

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

what are we trying to do here? Are you trying to set 200OK status in the servelet response? Ideally the response should have 200 OK and we need to verify it

Copy link
Contributor

Choose a reason for hiding this comment

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

can you check this

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/13002694350
Status: success

Copy link

@jenkins-is-staging jenkins-is-staging left a comment

Choose a reason for hiding this comment

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

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/13002694350

piraveena
piraveena previously approved these changes Jan 28, 2025
@RushanNanayakkara RushanNanayakkara merged commit e85fc71 into wso2:master Jan 31, 2025
5 checks passed
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.

4 participants