fix: handle new login endpoints [LIBS-600] #846
Conversation
# [10.6.0-alpha.1](v10.5.0...v10.6.0-alpha.1) (2024-03-05) ### Features * updates for login_app type [LIBS-405] ([#831](#831)) ([0cc2fbd](0cc2fbd))
# [10.6.0-alpha.2](v10.6.0-alpha.1...v10.6.0-alpha.2) (2024-03-20) ### Features * add redirect to safe login mode for login apps ([#835](#835)) ([ba26ada](ba26ada))
# [10.6.0-alpha.3](v10.6.0-alpha.2...v10.6.0-alpha.3) (2024-03-21) ### Features * enable publishing login_app apps ([93093ca](93093ca))
adapter/src/index.js
Outdated
| @@ -23,6 +23,7 @@ const AppAdapter = ({ | |||
| loginApp, | |||
| children, | |||
| }) => { | |||
| const [loginBaseUrl, setLoginBaseUrl] = useState(url) | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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. 🙏
## [11.2.1](v11.2.0...v11.2.1) (2024-04-29) ### Bug Fixes * handle new login endpoints [LIBS-600] ([#846](#846)) ([4512825](4512825))
|
🎉 This PR is included in version 11.2.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
See https://dhis2.atlassian.net/browse/LIBS-600
Summary
This change fixes the app-platform login modal to:
Detecting version
If a server is v41 or greater, api/loginConfig is accessible with the version. Therefore, this PR assumes that if a request to api/loginConfig succeeds that the server is v41+. This logic, obviously, has some limitations because the request could fail for other reasons (e.g. server is down), but this is not in general likely.
UI Improvements
General
I've disabled the log in button when login is in progress.
Normal apps
When using api/auth/logic, we have a REST api and will get a 400-range error back if authentication fails. This means that we can catch errors and display them back to the user. When using the struts endpoint, even successful authentication errors out. As a consequence, I've added error display if the server is detected as v41+ and have otherwise used the previous approach of refreshing the window (for v40 or less).
Note that you can successfully authenticate (regardless of version), and have CORS errors that are only detected on reload (note that this appears like the older version workflow: you authenticate, the app reloads, and then the cors error leads to the login modal to display again). I have not made any modifications to specifically catch CORS errors and display them.
v42:
v39:
Login app
I have added some handling for the case where you are have a login app. Previously, running the login app required that you specify the
DHIS2_BASE_URLenvironment variable. This adds a bit more refactoring to the code, but I think it's worth it as it makes login app development less finicky and also would be relevant if we introduce "public" app types in the future.Code notes
I thought about doing a bit more a refactor (e.g. to get rid of need for page refreshing with v41+, but then I felt that was a bit complicated and not worth the effort at the moment).