IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 841 forks source link

window.opener missing? #1225

Closed chris-kruining closed 3 years ago

chris-kruining commented 4 years ago

I am hesitant to create this issue because I have not changed any code and this issue arose just this morning.

In order to pinpoint the issue I enabled debug logging, this is when I spotted that the callback page says: PopupWindow.notifyOpener: no window.opener. Can't complete notification. as a warning in the popup.

which in turn I believe results in: PopupWindow.error: Popup window closed as an error on the main page.

I'm pretty sure chrome is improving the same-origin-policy and there for there is no opener on the window anymore.

Can you confirm this issue? otherwise I'll assume it is a bug in my code somewhere

brockallen commented 4 years ago

First I've heard of it, so if you learn more let us know.

chris-kruining commented 4 years ago

found it :: https://www.infoq.com/news/2020/09/coop-coep-cross-origin-isolation/

I have COEP enabled

chris-kruining commented 4 years ago

but I believe it should still be allowed to communicate between the popup and page via postMessage, but correct me if I am wrong

chris-kruining commented 4 years ago

correction COOP is the source of the issue not COEP, but bot are enabled

chris-kruining commented 4 years ago

weird, if I disable the headers the issue still persists

chris-kruining commented 4 years ago

Okay, I've been working on this for a couple of days now, and as far as I can see, this should have never worked in the first place due to cross-origin-policy.

to give some context. I am building a client side app in js which runs on https://unifyned.app. this app talks to an api on https://unifyned.cloud. Both have the oidc authority running on https://unifyned.cloud (this is going to be another domain in the future, but I doubt that's relevant).

So to authenticate I call

const UserMananager = new Promise(async (res, rev) => {
    if((globalThis instanceof Window) === false)
    {
        return rev();
    }

    const script = document.createElement('script');
    script.onload = () => {
        Oidc.Log.logger = console;
        Oidc.Log.level = Oidc.Log.DEBUG;

        res(new Oidc.UserManager({
            authority: 'https://unifyned.cloud',
            client_id: 'Shell.Web',
            response_type: 'code',
            scope: 'openid profile cms security',
            redirect_uri: 'https://unifyned.app/callback.html',
            post_logout_redirect_uri : 'https://unifyned.app',

            popupWindowFeatures: 'scrollbars=no,resizable=no,status=no,location=no,toolbar=no,menubar=no,width=360,height=600,left=300,top=300',

            automaticSilentRenew: false,
            validateSubOnSilentRenew: true,
            monitorAnonymousSession : false,
            filterProtocolClaims: true,
            loadUserInfo: true,
            revokeAccessTokenOnSignout : true,
        }));
    }
    script.onerror = rev;
    script.crossOrigin = 'cors';
    script.src = 'https://cdn.jsdelivr.net/npm/oidc-client@1.10.1/dist/oidc-client.min.js';

    document.head.appendChild(script);
});

const userManager = await UserMananager;
const user = await userManager.signinPopup();

which opens an popup to https://unifyned.cloud, this this has the opener, however as soon as https://unifyned.cloud redirects back to https://unifyned.app/callback.html this is lost due to the cross-origin policy

I'm tempted to start a pull request to migrate this logic to the postMessage mechanism. But I'd like to know if you:

(this is all in assumption that the window.opener is only used to that the callback for completion may be called)

chris-kruining commented 4 years ago

another mechanism, but I don't know if this will work is to use the hash part of the url, this stays client side, but I do not know if it will survive redirects

chris-kruining commented 4 years ago

@brockallen ?

chris-kruining commented 4 years ago

well. weird as weird goes, I have forked the lib and loaded it locally, issue is gone. Then reverted back to load from cdn, issue still gone. can't reproduce it anymore...

I'd still advocate for the migration to postMessage but the issue itself seems to be resolved

brockallen commented 4 years ago

Sorry, I'm swamped so I didn't have the time to try to repo your issue. I guess it's good the issue is resolved, but I don't like mysteries either :/

brockallen commented 3 years ago

Any update on this?

chris-kruining commented 3 years ago

not from my end. I never encountered this issue again, and I could reproduce it at the time either. so, as far as I am concerned this can be closed