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: handle new login endpoints [LIBS-600] #846

Merged
merged 13 commits into from
Apr 29, 2024
Merged
28 changes: 23 additions & 5 deletions adapter/i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1)\n"
"POT-Creation-Date: 2024-03-14T15:31:40.141Z\n"
"PO-Revision-Date: 2024-03-14T15:31:40.141Z\n"
"POT-Creation-Date: 2024-04-24T13:58:13.591Z\n"
"PO-Revision-Date: 2024-04-24T13:58:13.591Z\n"

msgid "Save your data"
msgstr "Save your data"
Expand Down Expand Up @@ -48,6 +48,9 @@ msgstr "Something went wrong"
msgid "Redirect to safe login mode"
msgstr "Redirect to safe login mode"

msgid "Redirect to safe login mode"
msgstr "Redirect to safe login mode"

msgid "Hide technical details"
msgstr "Hide technical details"

Expand All @@ -60,9 +63,27 @@ msgstr "The following information may be requested by technical support."
msgid "Copy technical details to clipboard"
msgstr "Copy technical details to clipboard"

msgid "Signing in..."
msgstr "Signing in..."

msgid "Sign in"
msgstr "Sign in"

msgid "Going to app..."
msgstr "Going to app..."

msgid "Go to app"
msgstr "Go to app"

msgid "Please sign in"
msgstr "Please sign in"

msgid "Specify server"
msgstr "Specify server"

msgid "Could not log in"
msgstr "Could not log in"

msgid "Server"
msgstr "Server"

Expand All @@ -71,6 +92,3 @@ msgstr "Username"

msgid "Password"
msgstr "Password"

msgid "Sign in"
msgstr "Sign in"
157 changes: 119 additions & 38 deletions adapter/src/components/LoginModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,57 +6,133 @@ import {
ModalActions,
Button,
InputField,
NoticeBox,
} from '@dhis2/ui'
import PropTypes from 'prop-types'
import React, { useState } from 'react'
import i18n from '../locales/index.js'
import { post } from '../utils/api.js'
import { get, post, postJSON } from '../utils/api.js'
import { styles } from './styles/LoginModal.style.js'

// Check if base URL is set statically as an env var (typical in production)
const staticUrl = process.env.REACT_APP_DHIS2_BASE_URL

export const LoginModal = ({ appName, baseUrl }) => {
const getUseNewLoginAPI = async (server) => {
try {
// if loginConfig is available, the instance can use new endpoints
await get(`${server}/api/loginConfig`)
return true
} catch (e) {
// if loginConfig is not available, the instance must use old endpoints
return false
}
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
}

const loginWithNewEndpoints = async ({
server,
username,
password,
setError,
setIsLoggingIn,
}) => {
try {
await postJSON(
`${server}/api/auth/login`,
JSON.stringify({
username: encodeURIComponent(username),
password: encodeURIComponent(password),
})
)
window.location.reload()
} catch (e) {
setError(e)
setIsLoggingIn(false)
}
}

const loginWithOldEndpoints = async ({ server, username, password }) => {
try {
await post(
`${server}/dhis-web-commons-security/login.action`,
`j_username=${encodeURIComponent(
username
)}&j_password=${encodeURIComponent(password)}`
)
} catch (e) {
console.error(e)
} finally {
window.location.reload()
}
}

export const LoginModal = ({ appName, baseUrl, loginApp = false }) => {
const [server, setServer] = useState(baseUrl || '')
const [username, setUsername] = useState('')
const [password, setPassword] = useState('')
const [isDirty, setIsDirty] = useState(false)
const [error, setError] = useState(null)
const [isLoggingIn, setIsLoggingIn] = useState(false)

const isValid = (val) => val && val.length >= 2
const getSignInButtonText = ({ loginApp, isLoggingIn }) => {
if (!loginApp) {
return isLoggingIn ? i18n.t('Signing in...') : i18n.t('Sign in')
}
return isLoggingIn ? i18n.t('Going to app...') : i18n.t('Go to app')
}

const onSubmit = async (e) => {
e.preventDefault()
setIsDirty(true)
if (isValid(server) && isValid(username) && isValid(password)) {
if (
isValid(server) &&
((isValid(username) && isValid(password)) || loginApp)
) {
setIsLoggingIn(true)
if (!staticUrl) {
// keep the localStorage value here -- it's still used in some
// obscure cases, like in the cypress network shim
window.localStorage.DHIS2_BASE_URL = server
await setBaseUrlByAppName({ appName, baseUrl: server })
if (loginApp) {
window.location.reload()
}
}
try {
await post(
`${server}/dhis-web-commons-security/login.action`,
`j_username=${encodeURIComponent(
username
)}&j_password=${encodeURIComponent(password)}`
)
} catch (e) {
console.log(
'TODO: This will always error and cancel the request until we get a real login endpoint!'
)
}

// TODO: Hacky solution... this shouldn't require a reload
window.location.reload()
const useNewLoginAPI = await getUseNewLoginAPI(server)
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

if (useNewLoginAPI) {
loginWithNewEndpoints({
server,
username,
password,
setError,
setIsLoggingIn,
})
} else {
loginWithOldEndpoints({ server, username, password })
}
}
}

return (
<Modal open small dataTest="dhis2-adapter-loginmodal">
<style jsx>{styles}</style>
<form onSubmit={onSubmit}>
<ModalTitle>{i18n.t('Please sign in')}</ModalTitle>
<ModalTitle>
{!loginApp
? i18n.t('Please sign in')
: i18n.t('Specify server')}
</ModalTitle>

<ModalContent>
{error && (
<div className="errorNotification">
<NoticeBox error title={i18n.t('Could not log in')}>
{error?.message || error?.details?.message}
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
</NoticeBox>
</div>
)}
{!staticUrl && (
<InputField
dataTest="dhis2-adapter-loginserver"
Expand All @@ -68,35 +144,39 @@ export const LoginModal = ({ appName, baseUrl }) => {
onChange={(input) => setServer(input.value)}
/>
)}
{!loginApp && (
<>
<InputField
dataTest="dhis2-adapter-loginname"
error={isDirty && !isValid(username)}
label={i18n.t('Username')}
name="j_username"
type="text"
value={username}
onChange={(input) => setUsername(input.value)}
/>

<InputField
dataTest="dhis2-adapter-loginname"
error={isDirty && !isValid(username)}
label={i18n.t('Username')}
name="j_username"
type="text"
value={username}
onChange={(input) => setUsername(input.value)}
/>

<InputField
dataTest="dhis2-adapter-loginpassword"
error={isDirty && !isValid(password)}
label={i18n.t('Password')}
name="j_password"
type="password"
value={password}
onChange={(input) => setPassword(input.value)}
/>
<InputField
dataTest="dhis2-adapter-loginpassword"
error={isDirty && !isValid(password)}
label={i18n.t('Password')}
name="j_password"
type="password"
value={password}
onChange={(input) => setPassword(input.value)}
/>
</>
)}
</ModalContent>

<ModalActions>
<Button
primary
dataTest="dhis2-adapter-loginsubmit"
type="submit"
disabled={isLoggingIn}
>
{i18n.t('Sign in')}
{getSignInButtonText({ loginApp, isLoggingIn })}
</Button>
</ModalActions>
</form>
Expand All @@ -106,4 +186,5 @@ export const LoginModal = ({ appName, baseUrl }) => {
LoginModal.propTypes = {
appName: PropTypes.string,
baseUrl: PropTypes.string,
loginApp: PropTypes.bool,
}
17 changes: 13 additions & 4 deletions adapter/src/components/ServerVersionProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const ServerVersionProvider = ({
parentAlertsAdd,
showAlertsInPlugin,
loginApp,
setLoginBaseUrl,
children,
}) => {
const offlineInterface = useOfflineInterface()
Expand Down Expand Up @@ -149,6 +150,10 @@ export const ServerVersionProvider = ({
}
}, [appName, baseUrl, loginApp])

useEffect(() => {
setLoginBaseUrl(baseUrl)
}, [setLoginBaseUrl, baseUrl])

useEffect(() => {
if (pwaEnabled) {
offlineInterface.ready.then(() => {
Expand All @@ -159,10 +164,12 @@ export const ServerVersionProvider = ({

// This needs to come before 'loading' case to show modal at correct times
if (systemInfoState.error || baseUrlState.error) {
return !loginApp ? (
<LoginModal appName={appName} baseUrl={baseUrl} />
) : (
<p>Specify DHIS2_BASE_URL environment variable</p>
return (
<LoginModal
appName={appName}
baseUrl={baseUrl}
loginApp={loginApp}
/>
)
}

Expand Down Expand Up @@ -192,6 +199,7 @@ export const ServerVersionProvider = ({
plugin={plugin}
parentAlertsAdd={parentAlertsAdd}
showAlertsInPlugin={showAlertsInPlugin}
skipApiVersion={loginApp ? true : false}
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
>
{children}
</Provider>
Expand All @@ -207,6 +215,7 @@ ServerVersionProvider.propTypes = {
parentAlertsAdd: PropTypes.func,
plugin: PropTypes.bool,
pwaEnabled: PropTypes.bool,
setLoginBaseUrl: PropTypes.func,
showAlertsInPlugin: PropTypes.bool,
url: PropTypes.string,
}
7 changes: 7 additions & 0 deletions adapter/src/components/styles/LoginModal.style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import css from 'styled-jsx/css'

export const styles = css`
.errorNotification {
margin-block: 8px;
}
`
9 changes: 7 additions & 2 deletions adapter/src/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { checkForSWUpdateAndReload } from '@dhis2/pwa'
import PropTypes from 'prop-types'
import React from 'react'
import React, { useState } from 'react'
import { AppWrapper } from './components/AppWrapper.js'
import { ErrorBoundary } from './components/ErrorBoundary.js'
import { LoginAppWrapper } from './components/LoginAppWrapper.js'
Expand All @@ -23,6 +23,7 @@ const AppAdapter = ({
loginApp,
children,
}) => {
const [loginBaseUrl, setLoginBaseUrl] = useState(url)
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to pass the base url (logic in ServerVersionProvider) to LoginAppWrapper in order to display the correct redirection for safe mode login.

The easiest option would be to just move the LoginAppWrapper component into ServerVersionProvider, but I thought maybe the current code structure was preferable in terms of displaying where the wrapper was? So hence this use of state variable to track the updates from ServerVersionProvider in this parent component.

Copy link
Member

@Birkbjo Birkbjo Apr 25, 2024

Choose a reason for hiding this comment

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

Wait, why can't we just read from the server-version context in LoginAppWrapper, since it's a child of that provider?
EDIT: Ah...nvm, that's not exposed from that provider.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Birkbjo : we can use useConfig within LoginAppWrapper to get the baseUrl. I didn't want to do that initially because I thought it was kind of confusing to use the hook from app-runtime there, but we're also using useConfig in the ConnectedHeaderBar component, so I guess using useConfig in this case would be more consistent. I think my hesitation was just that within the directory we have some components that have access to the context and some that do not, so you have to pay attention to where these components are in the hierarchy.

I've updated to use useConfig in LoginAppWrapper. 🙏

if (loginApp) {
return (
<ErrorBoundary
Expand All @@ -42,8 +43,11 @@ const AppAdapter = ({
pwaEnabled={pwaEnabled}
loginApp={loginApp}
plugin={false}
setLoginBaseUrl={setLoginBaseUrl}
>
<LoginAppWrapper url={url}>{children}</LoginAppWrapper>
<LoginAppWrapper url={loginBaseUrl}>
{children}
</LoginAppWrapper>
</ServerVersionProvider>
</ErrorBoundary>
)
Expand All @@ -65,6 +69,7 @@ const AppAdapter = ({
plugin={plugin}
parentAlertsAdd={parentAlertsAdd}
showAlertsInPlugin={showAlertsInPlugin}
setLoginBaseUrl={() => {}}
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
>
<AppWrapper
plugin={plugin}
Expand Down
9 changes: 9 additions & 0 deletions adapter/src/utils/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,12 @@ export const post = (url, body) =>
'Content-Type': 'application/x-www-form-urlencoded',
},
})

export const postJSON = (url, body) =>
request(url, {
method: 'POST',
body,
headers: {
'Content-Type': 'application/json',
},
})
Loading