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

feat: Returns an error on join request with no display name. #13546

Merged
merged 5 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions react/features/base/connection/actions.any.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import _ from 'lodash';

import { IReduxState, IStore } from '../../app/types';
import { setPrejoinDisplayNameRequired } from '../../prejoin/actions.any';
import { conferenceLeft, conferenceWillLeave } from '../conference/actions';
import { getCurrentConference } from '../conference/functions';
import JitsiMeetJS, { JitsiConnectionEvents } from '../lib-jitsi-meet';
Expand Down Expand Up @@ -232,15 +231,6 @@ export function _connectInternal(id?: string, password?: string) {
JitsiConnectionEvents.CONNECTION_FAILED,
_onConnectionFailed);

/**
* Marks the display name for the prejoin screen as required.
* This can happen if a user tries to join a room with lobby enabled.
*/
connection.addEventListener(
JitsiConnectionEvents.DISPLAY_NAME_REQUIRED,
() => dispatch(setPrejoinDisplayNameRequired())
);

/**
* Unsubscribe the connection instance from
* {@code CONNECTION_DISCONNECTED} and {@code CONNECTION_FAILED} events.
Expand Down
2 changes: 1 addition & 1 deletion react/features/lobby/actionTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ export const SET_PASSWORD_JOIN_FAILED = 'SET_PASSWORD_JOIN_FAILED';
/**
* Action type to remove chattingWithModerator field
*/
export const REMOVE_LOBBY_CHAT_WITH_MODERATOR = 'REMOVE_LOBBY_CHAT_WITH_MODERATOR';
export const REMOVE_LOBBY_CHAT_WITH_MODERATOR = 'REMOVE_LOBBY_CHAT_WITH_MODERATOR';
14 changes: 14 additions & 0 deletions react/features/lobby/actions.any.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { LOBBY_CHAT_MESSAGE } from '../chat/constants';
import { handleLobbyMessageReceived } from '../chat/middleware';
import { hideNotification, showNotification } from '../notifications/actions';
import { LOBBY_NOTIFICATION_ID } from '../notifications/constants';
import { joinConference } from '../prejoin/actions';

import {
KNOCKING_PARTICIPANT_ARRIVED_OR_UPDATED,
Expand Down Expand Up @@ -205,6 +206,18 @@ export function startKnocking() {
return async (dispatch: IStore['dispatch'], getState: IStore['getState']) => {
const state = getState();
const { membersOnly } = state['features/base/conference'];

if (!membersOnly) {

// no membersOnly, this means we got lobby screen shown as someone
// tried to join a conference that has lobby enabled without setting display name
// join conference should trigger the lobby/member_only path after setting the display name
// this is possible only for web, where we can join without a prejoin screen
dispatch(joinConference());

return;
}

const localParticipant = getLocalParticipant(state);

dispatch(conferenceWillJoin(membersOnly));
Expand Down Expand Up @@ -406,3 +419,4 @@ export function setLobbyMessageListener() {
});
};
}

8 changes: 7 additions & 1 deletion react/features/lobby/components/AbstractLobbyScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ export interface IProps {
*/
_deviceStatusVisible: boolean;

/**
* Indicates whether the message that display name is required is shown.
*/
_isDisplayNameRequiredActive: boolean;

/**
* True if moderator initiated a chat session with the participant.
*/
Expand Down Expand Up @@ -435,7 +440,7 @@ export function _mapStateToProps(state: IReduxState) {
const participantId = localParticipant?.id;
const inviteEnabledFlag = getFeatureFlag(state, INVITE_ENABLED, true);
const { disableInviteFunctions } = state['features/base/config'];
const { knocking, passwordJoinFailed } = state['features/lobby'];
const { isDisplayNameRequiredError, knocking, passwordJoinFailed } = state['features/lobby'];
const { iAmSipGateway } = state['features/base/config'];
const { disableLobbyPassword } = getSecurityUiConfig(state);
const showCopyUrlButton = inviteEnabledFlag || !disableInviteFunctions;
Expand All @@ -445,6 +450,7 @@ export function _mapStateToProps(state: IReduxState) {

return {
_deviceStatusVisible: deviceStatusVisible,
_isDisplayNameRequiredActive: Boolean(isDisplayNameRequiredError),
_knocking: knocking,
_lobbyChatMessages: messages,
_lobbyMessageRecipient: lobbyMessageRecipient?.name,
Expand Down
25 changes: 17 additions & 8 deletions react/features/lobby/components/web/LobbyScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,25 @@ class LobbyScreen extends AbstractLobbyScreen<IProps> {
*/
_renderParticipantInfo() {
const { displayName } = this.state;
const { t } = this.props;
const { _isDisplayNameRequiredActive, t } = this.props;
const showError = _isDisplayNameRequiredActive && !displayName;

return (
<Input
className = 'lobby-prejoin-input'
id = 'lobby-name-field'
onChange = { this._onChangeDisplayName }
placeholder = { t('lobby.nameField') }
testId = 'lobby.nameField'
value = { displayName } />
<>
<Input
autoFocus = { true }
className = 'lobby-prejoin-input'
error = { showError }
id = 'lobby-name-field'
onChange = { this._onChangeDisplayName }
placeholder = { t('lobby.nameField') }
testId = 'lobby.nameField'
value = { displayName } />

{ showError && <div
className = 'lobby-prejoin-error'
data-testid = 'lobby.errorMessage'>{t('prejoin.errorMissingName')}</div>}
</>
);
}

Expand Down
19 changes: 17 additions & 2 deletions react/features/lobby/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
import { INotificationProps } from '../notifications/types';
import { open as openParticipantsPane } from '../participants-pane/actions';
import { getParticipantsPaneOpen } from '../participants-pane/functions';
import { shouldAutoKnock } from '../prejoin/functions';
import { isPrejoinPageVisible, shouldAutoKnock } from '../prejoin/functions';

import {
KNOCKING_PARTICIPANT_ARRIVED_OR_UPDATED,
Expand Down Expand Up @@ -267,6 +267,8 @@ function _conferenceFailed({ dispatch, getState }: IStore, next: Function, actio
const state = getState();
const { membersOnly } = state['features/base/conference'];
const nonFirstFailure = Boolean(membersOnly);
const { isDisplayNameRequiredError } = state['features/lobby'];
const { prejoinConfig } = state['features/base/config'];

if (error.name === JitsiConferenceErrors.MEMBERS_ONLY_ERROR) {
if (typeof error.recoverable === 'undefined') {
Expand All @@ -277,7 +279,8 @@ function _conferenceFailed({ dispatch, getState }: IStore, next: Function, actio

dispatch(openLobbyScreen());

if (shouldAutoKnock(state)) {
// if there was an error about display name and pre-join is not enabled
if (shouldAutoKnock(state) || (isDisplayNameRequiredError && !prejoinConfig?.enabled)) {
dispatch(startKnocking());
}

Expand All @@ -288,6 +291,18 @@ function _conferenceFailed({ dispatch, getState }: IStore, next: Function, actio

dispatch(setPasswordJoinFailed(nonFirstFailure));

return result;
} else if (error.name === JitsiConferenceErrors.DISPLAY_NAME_REQUIRED) {
const [ isLobbyEnabled ] = error.params;

const result = next(action);

// if the error is due to required display name because lobby is enabled for the room
// if not showing the prejoin page then show lobby UI
if (isLobbyEnabled && !isPrejoinPageVisible(state)) {
dispatch(openLobbyScreen());
}

return result;
}

Expand Down
25 changes: 24 additions & 1 deletion react/features/lobby/reducer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { CONFERENCE_JOINED, CONFERENCE_LEFT, SET_PASSWORD } from '../base/conference/actionTypes';
import {
CONFERENCE_FAILED,
CONFERENCE_JOINED,
CONFERENCE_LEFT,
SET_PASSWORD
} from '../base/conference/actionTypes';
import { JitsiConferenceErrors } from '../base/lib-jitsi-meet';
import ReducerRegistry from '../base/redux/ReducerRegistry';

import {
Expand All @@ -14,6 +20,7 @@ import {
import { IKnockingParticipant } from './types';

const DEFAULT_STATE = {
isDisplayNameRequiredError: false,
knocking: false,
knockingParticipants: [],
lobbyEnabled: false,
Expand All @@ -22,6 +29,12 @@ const DEFAULT_STATE = {
};

export interface ILobbyState {

/**
* A conference error when we tried to join into a room with no display name
* when lobby is enabled in the room.
*/
isDisplayNameRequiredError: boolean;
knocking: boolean;
knockingParticipants: IKnockingParticipant[];
lobbyEnabled: boolean;
Expand All @@ -39,6 +52,16 @@ export interface ILobbyState {
*/
ReducerRegistry.register<ILobbyState>('features/lobby', (state = DEFAULT_STATE, action): ILobbyState => {
switch (action.type) {
case CONFERENCE_FAILED: {
if (action.error.name === JitsiConferenceErrors.DISPLAY_NAME_REQUIRED) {
return {
...state,
isDisplayNameRequiredError: true
};
}

return state;
}
case CONFERENCE_JOINED:
case CONFERENCE_LEFT:
return {
Expand Down
5 changes: 0 additions & 5 deletions react/features/prejoin/actionTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ export const SET_DEVICE_STATUS = 'SET_DEVICE_STATUS';
*/
export const SET_SKIP_PREJOIN_RELOAD = 'SET_SKIP_PREJOIN_RELOAD';

/**
* Action type used to set the mandatory stance of the prejoin display name.
*/
export const SET_PREJOIN_DISPLAY_NAME_REQUIRED = 'SET_PREJOIN_DISPLAY_NAME_REQUIRED';

/**
* Action type to set the country to dial out to.
*/
Expand Down
14 changes: 0 additions & 14 deletions react/features/prejoin/actions.any.ts

This file was deleted.

14 changes: 14 additions & 0 deletions react/features/prejoin/actions.native.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { IStore } from '../app/types';

/**
* Action used to start the conference.
*
* @param {Object} options - The config options that override the default ones (if any).
* @param {boolean} _ignoreJoiningInProgress - If true we won't check the joiningInProgress flag.
* @returns {Function}
*/
export function joinConference(options?: Object, _ignoreJoiningInProgress = false) {
// eslint-disable-next-line @typescript-eslint/no-empty-function
return async function(_dispatch: IStore['dispatch'], _getState: IStore['getState']) {
};
}
2 changes: 0 additions & 2 deletions react/features/prejoin/actions.web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ const STATUS_REQ_FREQUENCY = 2000;
*/
const STATUS_REQ_CAP = 45;

export * from './actions.any';

/**
* Polls for status change after dial out.
* Changes dialog message based on response, closes the dialog if there is an error,
Expand Down
2 changes: 1 addition & 1 deletion react/features/prejoin/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function isDeviceStatusVisible(state: IReduxState): boolean {
* @returns {boolean}
*/
export function isDisplayNameRequired(state: IReduxState): boolean {
return Boolean(state['features/prejoin']?.isDisplayNameRequired
return Boolean(state['features/lobby']?.isDisplayNameRequiredError
|| state['features/base/config']?.requireDisplayName);
}

Expand Down
10 changes: 0 additions & 10 deletions react/features/prejoin/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
SET_JOIN_BY_PHONE_DIALOG_VISIBLITY,
SET_PRECALL_TEST_RESULTS,
SET_PREJOIN_DEVICE_ERRORS,
SET_PREJOIN_DISPLAY_NAME_REQUIRED,
SET_PREJOIN_PAGE_VISIBILITY,
SET_SKIP_PREJOIN_RELOAD
} from './actionTypes';
Expand All @@ -26,7 +25,6 @@ const DEFAULT_STATE = {
},
dialOutNumber: '',
dialOutStatus: 'prejoin.dialing',
isDisplayNameRequired: false,
name: '',
rawError: '',
showPrejoin: true,
Expand All @@ -45,7 +43,6 @@ export interface IPrejoinState {
};
dialOutNumber: string;
dialOutStatus: string;
isDisplayNameRequired: boolean;
joiningInProgress?: boolean;
name: string;
precallTestResults?: {
Expand Down Expand Up @@ -143,13 +140,6 @@ ReducerRegistry.register<IPrejoinState>(
};
}

case SET_PREJOIN_DISPLAY_NAME_REQUIRED: {
return {
...state,
isDisplayNameRequired: true
};
}

default:
return state;
}
Expand Down
11 changes: 11 additions & 0 deletions resources/prosody-plugins/mod_muc_lobby_rooms.lua
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,17 @@ process_host_module(main_muc_component_config, function(host_module, host)
end
end

-- Check for display name if missing return an error
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be a separate module? This functionality is not exclusive to lobby is it?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for lobby, we are reporting this through jiconop when lobby is enabled so we can make the display name required.

reply:tag('feature', { var = DISPLAY_NAME_REQUIRED_FEATURE }):up();

Now when dropping initial connection on opening pre-join we are adding this if someone tries to join without a display name when lobby is enabled to fail and set the UI that display name is required, so the participant enters a dispalyname and retries.

Copy link
Member Author

Choose a reason for hiding this comment

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

But this does not work if prejoin is not enabled ... the tests catch that failing case ... need to look into

local displayName = stanza:get_child_text('nick', 'http://jabber.org/protocol/nick');
if not displayName or #displayName == 0 then
local reply = st.error_reply(stanza, 'modify', 'not-acceptable');
reply.tags[1].attr.code = '406';
reply:tag('displayname-required', { xmlns = 'http://jitsi.org/jitmeet', lobby = 'true' }):up():up();

event.origin.send(reply:tag('x', {xmlns = MUC_NS}));
return true;
end

-- we want to add the custom lobbyroom field to fill in the lobby room jid
local invitee = event.stanza.attr.from;
local affiliation = room:get_affiliation(invitee);
Expand Down