-
Notifications
You must be signed in to change notification settings - Fork 4k
fix: prevent un-authorized access to workspace pages #20215
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
Changes from all commits
35502ce
deb9d20
e69af66
bbbfea6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import _ from 'underscore'; | ||
| import {CONST} from 'expensify-common/lib/CONST'; | ||
| import Text from './Text'; | ||
| import TextLink from './TextLink'; | ||
| import styles from '../styles/styles'; | ||
|
|
||
| const propTypes = { | ||
| text: PropTypes.string.isRequired, | ||
| style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), | ||
| }; | ||
|
|
||
| const defaultProps = { | ||
| style: [], | ||
| }; | ||
|
|
||
| /* | ||
| * This is a "utility component", that does this: | ||
| * - Checks if a text contains any email. If it does, render it as a mailto: link | ||
| * - Else just render it inside `Text` component | ||
| */ | ||
|
|
||
| const AutoEmailLink = (props) => ( | ||
| <Text style={props.style}> | ||
| {_.map(props.text.split(CONST.REG_EXP.EXTRACT_EMAIL), (str, index) => { | ||
| if (CONST.REG_EXP.EMAIL.test(str)) { | ||
| return ( | ||
| <TextLink | ||
| key={`${index}-${str}`} | ||
| href={`mailto:${str}`} | ||
| style={styles.link} | ||
| > | ||
| {str} | ||
| </TextLink> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <Text | ||
| style={props.style} | ||
| key={`${index}-${str}`} | ||
| > | ||
| {str} | ||
| </Text> | ||
| ); | ||
| })} | ||
| </Text> | ||
| ); | ||
|
|
||
| AutoEmailLink.displayName = 'AutoEmailLink'; | ||
| AutoEmailLink.propTypes = propTypes; | ||
| AutoEmailLink.defaultProps = defaultProps; | ||
| export default AutoEmailLink; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import Text from '../Text'; | |
| import themeColors from '../../styles/themes/default'; | ||
| import TextLink from '../TextLink'; | ||
| import Navigation from '../../libs/Navigation/Navigation'; | ||
| import AutoEmailLink from '../AutoEmailLink'; | ||
|
|
||
| const propTypes = { | ||
| /** Expensicon for the page */ | ||
|
|
@@ -56,7 +57,10 @@ const BlockingView = (props) => ( | |
| height={props.iconHeight} | ||
| /> | ||
| <Text style={[styles.notFoundTextHeader]}>{props.title}</Text> | ||
| <Text style={[styles.textAlignCenter]}>{props.subtitle}</Text> | ||
| <AutoEmailLink | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @allroundexperts what is the need for this change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message contains an email which needs to be shown as a link.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pardon me for my ignorance, but can you please share the context of why this was needed? I cannot find it in the issue comments?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neither I can find the screenshots/test steps related to that.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mananjadhav If you look at the English translation message, it contains concierge email address. We need a way to show it as a hyperlink.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool. Thanks for clarifying.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on the comment, I think it would be better to add a description at the top of the file.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handled. |
||
| style={[styles.textAlignCenter]} | ||
| text={props.subtitle} | ||
| /> | ||
| {props.shouldShowLink ? ( | ||
| <TextLink | ||
| onPress={props.onLinkPress} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1073,6 +1073,7 @@ export default { | |
| growlMessageOnDeleteError: 'This workspace cannot be deleted right now because reports are actively being processed', | ||
| unavailable: 'Unavailable workspace', | ||
| memberNotFound: 'Member not found. To invite a new member to the workspace, please use the Invite button above.', | ||
| notAuthorized: `You do not have access to this page. Are you trying to join the workspace? Please reach out to the owner of this workspace so they can add you as a member! Something else? Reach out to ${CONST.EMAIL.CONCIERGE}`, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@youssef-lr @sonialiap Looking at the test steps, if the user B is already a part of the workspace, does this statement make sense?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay this could be a generic message, that shows up even for the user who is not a part of the workspace, we can keep it as is. |
||
| goToRoom: ({roomName}) => `Go to ${roomName} room`, | ||
| }, | ||
| emptyWorkspace: { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAB: Remove this blank space