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

fix(authentication): removed old LoginDialog.js file, fixed redirection to the external auth and created actions.any.js #9049

Merged
merged 9 commits into from Apr 22, 2021

Conversation

Calinteodor
Copy link
Contributor

@Calinteodor Calinteodor commented Apr 20, 2021

No description provided.

@Calinteodor Calinteodor changed the title fix(authentication) login dialog closed when connection established fix(authentication) Apr 21, 2021
Copy link
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Why do we have some actions with _ ? Usually those are "private". If we are using them outside of the feature itself, we may want to make them public.

@@ -3,11 +3,14 @@
import Logger from 'jitsi-meet-logger';

import { openConnection } from '../../../connection';
import { externalAuthDialog } from '../../../react/features/authentication/actions';
import { _openLoginDialog } from '../../../react/features/authentication/actions.native';
Copy link
Member

Choose a reason for hiding this comment

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

Why are we importing things from native?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created actions.any.js for actions used on both web and mobile.

@Calinteodor Calinteodor changed the title fix(authentication) fix(authentication) removed old LoginDialog.js file, created actions.any for authentication react feature and fixed redirection to the external auth Apr 21, 2021
@Calinteodor Calinteodor changed the title fix(authentication) removed old LoginDialog.js file, created actions.any for authentication react feature and fixed redirection to the external auth fix(authentication): removed old LoginDialog.js file, fixed redirection to the external auth and created actions.any.js Apr 21, 2021
@SimonMaenaut
Copy link

SimonMaenaut commented Apr 21, 2021

We tested this fix on our instance with Shibboleth (external authentification).

The login at the beginning of the conference ("Waiting for the host ..." -> "I am the host") works and redirects to the login service. However the login button in the settings under profile (when the conference is running) does nothing and closes the dialog box.

Screenshot from 2021-04-22 00-48-00

@@ -10,8 +10,8 @@ import { Dialog } from '../../../base/dialog';
import { translate, translateToHTML } from '../../../base/i18n';
import { JitsiConnectionErrors } from '../../../base/lib-jitsi-meet';
import { connect as reduxConnect } from '../../../base/redux';
import { authenticateAndUpgradeRole } from '../../actions.native';
import { cancelLogin, hideLoginDialog } from '../../actions.web';
import { authenticateAndUpgradeRole } from '../../actions.any';
Copy link
Member

Choose a reason for hiding this comment

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

actions.web and actions.native should re-export everything from actions.any, so you only import from one of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -43,7 +45,7 @@ function doExternalAuth(room, lockPassword) {
getUrl = room.getExternalAuthUrl(true);
}
getUrl.then(url => {
externalAuthWindow = LoginDialog.showExternalAuthDialog(
externalAuthWindow = openAuthDialog(
Copy link
Member

Choose a reason for hiding this comment

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

Will this really work? Currently that will directly open the popup to the given URL, but with your change the WaitForOwnerDialog will be open instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted the change.

@Calinteodor Calinteodor force-pushed the authentication-login-dialog-fix branch from 6a89103 to 30e5432 Compare April 22, 2021 14:11
@saghul saghul merged commit 98658f5 into jitsi:master Apr 22, 2021
psi-4ward pushed a commit to psi-4ward/jitsi-meet that referenced this pull request Apr 30, 2021
…on to the external auth and created actions.any.js (jitsi#9049)

* fix(authentication) login dialog now closes when connection is established

* fix(authentication) fixed shibboleth auth

* fix(authentication) renamed authenticateExternal func to authenticate and updated its logic

* fix(authentication)removed logindialog.js and created actions.any

* fix(authentication) removed focus from externalauthwindow

* fix(authentication) removed private sign from some actions and added openLoginDialog to actions.any

* fix(authentication) exported all from actions.any

* fix(authentication) reverted change regarding externalAuth

* fix(authentication) fixed indentation
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.

None yet

3 participants