swagger-ui

swagger-ui copied to clipboard

Motivation and Context

Sometimes window.opener might be null (there's multiple issues about that). Currently this results in a blank page without any indication of what might have gone wrong.

Related: #8315, #3227 Fixes #8030, fixes #6150

Description

This PR adds a fallback: if window.opener is null, it will display a one-liner to run on a Swagger UI page (using devtools) in a prompt(). Not too elegant, but given this is an edge case I'm not sure if it warrants more complex UI.

How Has This Been Tested?

I've checked the added snippet independently by running it in devtools and it works as intended.

I'll be grateful for pointers to how to cover it with unit / integrations tests :-)

Checklist

My PR contains...

• [ ] No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• [ ] Dependency changes (any modification to dependencies in package.json)
• [x] Bug fixes (non-breaking change which fixes an issue)
• [ ] Improvements (misc. changes to existing features)
• [ ] Features (non-breaking change which adds functionality)

My changes...

• [ ] are breaking changes to a public API (config options, System API, major UI change, etc).
• [ ] are breaking changes to a private API (Redux, component props, utility functions, etc.).
• [ ] are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• [x] are not breaking changes.

Documentation

• [x] My changes do not require a change to the project documentation.
• [ ] My changes require a change to the project documentation.
• [ ] If yes to above: I have updated the documentation accordingly.

Automated tests

• [ ] My changes can not or do not need to be tested.
• [ ] My changes can and should be tested by unit and/or integration tests.
• [ ] If yes to above: I have added tests to cover my changes.
• [ ] If yes to above: I have taken care to cover edge cases in my tests.
• [ ] All new and existing tests passed.

I think that window.postMessage could be used to communicate with the original opener instead of doing it manually (by using the oauth2 state to check the origin).

I think that window.postMessage could be used to communicate with the original opener instead of doing it manually (by using the oauth2 state to check the origin).

@nicolashenry Could you elaborate? I'm interested in getting this fixed and am willing to contribute.

@nicolashenry This is how it usually works already, yeah. The problem is, when window.opener is null we can't use postMessage, so we need some kind of a fallback mechanism for that – this is what I'm trying to achieve here.

@ftsell Thank you so much for the review! Fixed both issues.

I looked into using postMessage() for this usecase and while I agree that it is probably the better way for cross-tab communication in general the postMessage() function is also located on a window object. This means that it also requires a reference to the opener which is precisely what we're missing :/

An alternative I found is to use a BroadcastChannel but that would broadcast authentication related information through the whole browser which is also not optimal.

Yes, I was thinking about BroadcastChannel.postMessage and not Window.postMessage. Sorry for the mistake. Creating a unique identifier (a "nonce" for example) in the oauth "state" should be enough to differentiate which window is the origin of the call.

@nicolashenry Sounds like a good idea. Wanna try implement it?

@notpushkin I made this quickly : https://github.com/swagger-api/swagger-ui/commit/940ebe0cff45885d4eaa36f5a0ee7624bc7b20bc, I have not tested it yet but I will try to turn this into a PR this week if it works.

@nicolashenry Have you tested/made a PR for your fix?

Are there plans to include the fix in a release? I ran into the same problem and this solution from @nicolashenry works fine for me.