Skip to content

Conversation

@yash-rajpal
Copy link
Member

@yash-rajpal yash-rajpal commented Aug 28, 2025

Proposed changes (including videos or screenshots)

Login using iframe authentication was broken in release 7.8.0 and this issue was introduced by a refactor #36022

Issue(s)

Fixes #36638

Steps to test or reproduce

Configure iframe auth and ensure login works

Further comments

SUP-853

Removing headers on this API call because it was causing CORS errors. Our specs don't ask for allowed headers on origin, either we ask our customers to allow headers on their CORS config or we omit them on our side on this API call.
image

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Aug 28, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 7.11.0, but it targets 7.10.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2025

🦋 Changeset detected

Latest commit: f9aff63

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 39 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Major
@rocket.chat/ui-client Major
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Major
@rocket.chat/ui-voip Major
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@yash-rajpal yash-rajpal marked this pull request as ready for review August 28, 2025 13:42
@yash-rajpal yash-rajpal requested a review from a team as a code owner August 28, 2025 13:42
@jessicaschelly
Copy link
Member

Is it possible to add tests?

@yash-rajpal yash-rajpal added this to the 7.11.0 milestone Aug 28, 2025
@yash-rajpal yash-rajpal added the stat: QA assured Means it has been tested and approved by a company insider label Aug 28, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Aug 28, 2025
@yash-rajpal
Copy link
Member Author

yash-rajpal commented Aug 28, 2025

I would very much like to add tests for this critical feature, but we have some challenges to solve to write end to end tests for this. This feature relies on some external integrations and APIs, I'll try to find a way to have this in our code for TEST_MODE or will figure out some other way.

Since this a critical bug fix which needs to be back-ported to previous releases, my suggestion is to move forward now without tests and I'll open a task in our backlog for this.
wdyt @jessicaschelly ?

@codecov
Copy link

codecov bot commented Aug 28, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.15%. Comparing base (92e30b4) to head (f9aff63).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #36820      +/-   ##
===========================================
+ Coverage    66.13%   66.15%   +0.02%     
===========================================
  Files         3303     3303              
  Lines       110994   110998       +4     
  Branches     21043    21053      +10     
===========================================
+ Hits         73402    73434      +32     
+ Misses       34919    34892      -27     
+ Partials      2673     2672       -1     
Flag Coverage Δ
e2e 57.28% <80.00%> (-0.01%) ⬇️
unit 71.53% <ø> (+0.05%) ⬆️

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

🚀 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.

Copy link
Contributor

@tiagoevanp tiagoevanp left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit 3825afa into develop Aug 28, 2025
48 checks passed
@kodiakhq kodiakhq bot deleted the fix/iframe-login branch August 28, 2025 16:38
@scuciatto
Copy link
Member

/patch

dionisio-bot bot pushed a commit that referenced this pull request Sep 17, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 17, 2025

Pull request #36962 added to Project: "Patch 7.10.1"

@scuciatto
Copy link
Member

/backport 7.9.4

dionisio-bot bot pushed a commit that referenced this pull request Sep 17, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 17, 2025

Pull request #36963 added to Project: "Patch 7.9.4"

@scuciatto
Copy link
Member

/backport 7.8.5

dionisio-bot bot pushed a commit that referenced this pull request Sep 17, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 17, 2025

Pull request #36964 added to Project: "Patch 7.8.5"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Iframe Login Integration not Loading anymore

5 participants