UI: Fix OIDC login in fullscreen #19071
Conversation
| return this.handleOIDCError(); | ||
| } | ||
| if (event.data.source === 'oidc-callback') { | ||
| if (event.data.source === 'oidc-callback' && event.isTrusted && event.origin === thisWindow.origin) { |
There was a problem hiding this comment.
While I'm in here, pulled in the change from ##18521
| later(() => { | ||
| this.closeWindow(oidcWindow); | ||
| }, WAIT_TIME); |
There was a problem hiding this comment.
I'm just wondering why this pause was needed in the first place and if there might be a case where removing it will cause a regression? Or was the timeout the problem in the first place?
There was a problem hiding this comment.
Also, if we don't need it then the WAIT_TIME variable can also be removed.
There was a problem hiding this comment.
The WAIT_TIME variable is used other places 👍
My suspicion is that we wanted to close the window no matter the outcome of the next request, so wrapping this in a later loop prevented duplicative code. I tested that the window closes if the adapter method errors out and tried to handle all cases.
Resolves Issue #17584
In a browser fullscreen mode, there was a race condition where the new tab triggered close before the OIDC flow was finished.
Behavior before
Locally we increased the timeout, which is a bad experience but shows that the core issue was the close being triggered too soon:
With the fix, we manually call close window after the OIDC succeeds, or when canceling login:
