Skip to content

Commit

Permalink
improvement: better handling of signin errors 🌱 #1173
Browse files Browse the repository at this point in the history
  • Loading branch information
olemp committed Jan 31, 2024
1 parent 3432e6b commit f9dc84f
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 24 deletions.
19 changes: 19 additions & 0 deletions client/components/Markdown/Markdown.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React, { FC } from 'react'
import ReactMarkdown from 'react-markdown'
import rehypeRaw from 'rehype-raw'
import rehypeSanitize from 'rehype-sanitize'
import { IMarkdownProps } from './types'

/**
* Renders markdown text as HTML using ReactMarkdown with plugins
* `rehypeRaw` and `rehypeSanitize`.
*
* @param props - The props for the Markdown component.
*/
export const Markdown: FC<IMarkdownProps> = (props) => {
return (
<ReactMarkdown rehypePlugins={[rehypeRaw, rehypeSanitize]}>
{props.text}
</ReactMarkdown>
)
}
2 changes: 2 additions & 0 deletions client/components/Markdown/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './Markdown'
export * from './types'
3 changes: 3 additions & 0 deletions client/components/Markdown/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface IMarkdownProps {
text: string
}
1 change: 1 addition & 0 deletions client/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@ export * from './SubText'
export * from './Toast'
export * from './UserColumn'
export * from './UserMessage'
export * from './Markdown'
5 changes: 3 additions & 2 deletions client/i18n/en-GB.json
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@
"googleSignInText": "Sign in with Google",
"activeDirectory": "Azure AD",
"google": "Google",
"signInDisabledMessage": "Sign in is currently disabled. Please hold on.",
"signInDisabledText": "Sign in is currently disabled. Please hold on.",
"error": "Error",
"uiThemeLabel": "Theme",
"light-theme": "Light theme",
Expand Down Expand Up @@ -547,7 +547,8 @@
"projectTagCopiedToClipboard": "The project tag **{{tag}}** was copied to the clipboard.",
"outlookCategoryError": "There was an error creating the category in Outlook. If you just created it, it might take some moments before it shows up here.",
"tagFieldLabel": "Tag",
"ignoredHours": "Ignored hours"
"ignoredHours": "Ignored hours",
"signInDisabledMessage": "We'"
},
"navigation": {
"TimesheetPage": "Timesheet",
Expand Down
2 changes: 1 addition & 1 deletion client/i18n/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@
"googleSignInText": "Logg på med Google",
"activeDirectory": "Azure AD",
"google": "Google",
"signInDisabledMessage": "Pålogging er for øyeblikket deaktivert. \nVennligst vent.",
"signInDisabledText": "Pålogging er for øyeblikket deaktivert. \nVennligst vent.",
"error": "Feilmelding",
"uiThemeLabel": "Fargetema",
"dark-theme": "Mørkt tema",
Expand Down
2 changes: 1 addition & 1 deletion client/i18n/nn.json
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@
"googleSignInText": "Logg på med Google",
"activeDirectory": "Azure AD",
"google": "Google",
"signInDisabledMessage": "Pålogging er for øyeblikket deaktivert. \nVennligst vent.",
"signInDisabledText": "Pålogging er for øyeblikket deaktivert. \nVennligst vent.",
"error": "Feilmelding",
"uiThemeLabel": "Fargetema",
"dark-theme": "Mørkt tema",
Expand Down
19 changes: 6 additions & 13 deletions client/pages/Home/Home.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Button } from '@fluentui/react-components'
import { UserMessage } from 'components'
import { Logo } from 'components/Logo'
import { PageComponent } from 'pages/types'
import React from 'react'
import { useTranslation } from 'react-i18next'
import { Redirect } from 'react-router-dom'
import _ from 'underscore'
import styles from './Home.module.scss'
import { LoginError } from './LoginError'
import { useAuthProviders } from './useAuthProviders'
import { useHome } from './useHome'

Expand All @@ -17,7 +17,7 @@ import { useHome } from './useHome'
*/
export const Home: PageComponent = () => {
const { t } = useTranslation()
const { error, subscription, redirectPage } = useHome()
const { loginError, subscription, redirectPage } = useHome()
const providers = useAuthProviders()

if (redirectPage) {
Expand All @@ -27,20 +27,13 @@ export const Home: PageComponent = () => {
return (
<div className={Home.className}>
<Logo showMotto={true} dropShadow={true} />
{error && (
<UserMessage
className={styles.error}
intent='error'
text={[`#### ${error.name} ####`, error.message].join('\n\n')}
/>
{loginError && (
<LoginError text={loginError.name} message={loginError.message} />
)}
{_.isEmpty(Object.keys(providers)) && (
<UserMessage
intent='warning'
text={t('common.signInDisabledMessage')}
/>
<LoginError text={t('common.signInDisabledText')} />
)}
{!subscription && !error && (
{!subscription && !loginError && (
<div className={styles.signIn}>
{Object.keys(providers).map((key) => (
<Button
Expand Down
10 changes: 10 additions & 0 deletions client/pages/Home/LoginError/LoginError.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
.loginError {
margin: 15px 0 0 0;
display: flex;
flex-direction: column;
align-items: center;

.text {
margin-bottom: 5px;
}
}
30 changes: 30 additions & 0 deletions client/pages/Home/LoginError/LoginError.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Markdown } from 'components'
import React from 'react'
import { StyledComponent } from 'types'
import { ILoginErrorProps } from './types'
import styles from './LoginError.module.scss'
import { Button, Caption1 } from '@fluentui/react-components'
import { getFluentIcon } from 'utils'

export const LoginError: StyledComponent<ILoginErrorProps> = (props) => {
return (
<div className={LoginError.className}>
<h3 className={styles.text}>{props.text}</h3>
<Caption1 hidden={!props.message}>
<Markdown text={props.message} />
</Caption1>

<Button
style={{ marginTop: '1rem' }}
icon={getFluentIcon('Dismiss')}
appearance='subtle'
onClick={() => window.location.replace(window.location.origin)}
>
Dismiss
</Button>
</div>
)
}

LoginError.displayName = 'LoginError'
LoginError.className = styles.loginError
2 changes: 2 additions & 0 deletions client/pages/Home/LoginError/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './LoginError'
export * from './types'
4 changes: 4 additions & 0 deletions client/pages/Home/LoginError/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface ILoginErrorProps {
text: string
message?: string
}
7 changes: 4 additions & 3 deletions client/pages/Home/useHome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export function useHome() {
const { user, subscription } = useAppContext()
const location = useLocation<{ prevPath: string }>()
const urlSearchParameters = new URLSearchParams(document.location.search)
const error =
const loginError: Error =
urlSearchParameters.get('error') &&
JSON.parse(atob(urlSearchParameters.get('error')))
const redirectPage =
Expand All @@ -17,9 +17,10 @@ export function useHome() {
location.state?.prevPath === undefined
? user.startPage
: null

return {
error,
loginError,
subscription,
redirectPage
} as const
}
}
19 changes: 17 additions & 2 deletions server/middleware/passport/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,29 @@ export class SigninError extends Error {
}
}

/**
* No OID found error
*/
export const NO_OID_FOUND = new SigninError(
'0f8fc199',
'Sorry to break it to you..',
'... but an error occured attempting to sign you in.',
'BlockedSite'
)

/**
* Tenant not enrolled error
*/
export const TENANT_NOT_ENROLLED = new SigninError(
'de72e4da',
'Your company is not enrolled in did',
'Please contact<a href="mailto:[email protected]">[email protected]</a> for more information.',
'We\'re currently accepting new pilot customers. Please contact <a href="mailto:[email protected]">[email protected]</a> for more information.',
'Phone'
)

/**
* User not enrolled error
*/
export const USER_NOT_ENROLLED = new SigninError(
'cee991f0',
'I promised to keep it a secret...',
Expand All @@ -49,13 +58,19 @@ export const USER_NOT_ENROLLED = new SigninError(
'Sad'
)

export const SIGNIN_FAILED = new SigninError(
/**
* Generic sign in failed error
*/
export const GENERIC_SIGNIN_FAILED = new SigninError(
'e0666582',
'An error occured signing you in',
'Sorry, we were not able to sign you in right now, and we are not really sure why!<br/><br/> It can help to clear your browser cache.',
'Dislike'
)

/**
* User account disabled error
*/
export const USER_ACCOUNT_DISABLED = new SigninError(
'e0666582',
'An error occured signing you in',
Expand Down
8 changes: 6 additions & 2 deletions server/routes/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import { NextFunction, Request, Response, Router } from 'express'
import passport from 'passport'
import _ from 'underscore'
import url from 'url'
import { SigninError, SIGNIN_FAILED } from '../middleware/passport/errors'
import {
SigninError,
GENERIC_SIGNIN_FAILED
} from '../middleware/passport/errors'
import { environment } from '../utils'
const auth = Router()

Expand Down Expand Up @@ -40,7 +43,8 @@ export const authCallbackHandler =
(request: Request, response: Response, next: NextFunction) => {
passport.authenticate(strategy, (error: Error, user: Express.User) => {
if (error || !user) {
const _error = error instanceof SigninError ? error : SIGNIN_FAILED
const _error =
error instanceof SigninError ? error : GENERIC_SIGNIN_FAILED
return response.redirect(
url.format({
pathname: '/',
Expand Down

0 comments on commit f9dc84f

Please sign in to comment.