-
Notifications
You must be signed in to change notification settings - Fork 2
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 to support interoperation with kbase-ui (react edition) [URO-193] #198
base: main
Are you sure you want to change the base?
Conversation
…193] I've not broken out all of the changes. They are mostly interdependent.
missed in when copying files
supports same-origin better
…efactor to separate send and receive uuidv4 channel ids [URO-193]
simplified, added additional test
use cases are tricky to support
Recent changes include:
|
timer inaccuracy bites again!
this prevents the browser from caching the state of the Europa when navigating away. The caching behavior makes it difficult to keep Europa and kbase-ui in sync. kbase-ui has a matching change to prevent it's index.html from being cached. It becomes an issue with links that navigate away from Europa in the same window - e.g. login - and the user then uses the back button to return to Europa. This configuration forces Europa to be reloaded, which then reloads kbase-ui.
Latest changes:
In navigation scenarios that involve navigating away from Europa, but within the same window, browser back/forward behavior will by default attempt to restore the page from a memory cache. This can lead to strange failure conditions relating to Europa and kbase-ui being restored in different states. That is, both the iframe and the main window are restored, but if their states are not precisely as they were when they were navigated away from, they do not play well together. The simplest solution, implemented in a change in the nginx config file, is to separate the root location (i.e. index.html, i.e. the web app!) differently from static resources. The root location has browser caching disabled with "no-store". Disk and proxy caching behavior should be looked at also, but is out of scope here. The end effect is that when navigating back to a Europa url via the back/forward buttons should cause Europa to be loaded from scratch with that url.
This change was in changes from yesterday, but bears more explanation. This is actually a return to the original design, but I changed it due to problems I didn't have time to resolve. Since the key to ensuring private communication is using a uuid (channel id) to filter messages, it is best if each channel always has a unique channel id. How does communication start then? A pair of channel ids are generated by Europa and passed to kbase-ui as search params. kbase-ui picks those up and sets up its channels correspondingly. kbase-ui ignores regular search params, instead embedding them in the fragment identifier (hash). This is part of the URL standard, although perhaps unusual. The previous incarnation used a stable, static channel id for the initial communication, relying upon the initial exchange of messages to switch over to unique channnel ids. |
@dauglyon would it be possible to begin asking questions or asking for changes? I realize there is a lot to go through, even with having pared out some of the changes into separate PRs, but perhaps we could approach the review incrementally as well. Alternatively, or perhaps concurrently (and it may help review), we could deploy it on CI. CI right now has the matching version of kbase-ui deployed at legacy2.ci.kbase.us. I regularly evaluate the local Europa with the deployed kbase-ui to ensure they are kept in sync. I'll probably spend the remainder of this afternoon reviewing it myself, probably adding clarifying comments, perhaps working on documentation - as it has been a few days since I looked at it and I'll have a fresh set of eyes on it. |
Changes to improve kbase-ui support
Index of changes
Here we present the sets of changes, along with (hopefully the full set of) files affected.
Summary of Changes
Refactoring of Legacy Support
- Decompose legacy support into separate
files
- Refactor to channels
- Refactor hooks
- Refactor constant values
- Refactor bare legacy path
- Fallback handling and message for PageNotFound page
Regressions
- Parameters in legacy urls
- Redirect after authentication
- Narrative Loader
Issues
- Full login/logout responsibility
- Add navigation for ORCID Link
- Token coerced to uppercase
Other Improvements
- Navigation for orcidlink
- Support legacy on subpath
- Note on deploy build script
Summary of Changes
Generally all changes made to Europa are in support of improving legacy support, and adapting to a new version of kbase-ui which has itself been specifically refactored to exist in an environment hosted by Europa.
(This version of kbase-ui had been hosted on CI for the year preceding the launch of Europa, but has since been heavily refactored to be hosted by Europa.)
Some of the Europa changes necessarily touch code outside of legacy support. The biggest set of such changes are in the auth/hooks module, as there were several tricky areas around login, logout, and legacy startup that were problematic with the existing design.
Refactoring Legacy Support
Decompose legacy support into separate files
When trying to understand and refactor the legacy implementation, I eventually decided to split the functionality into separate files. This resulted in the primary implementation files:
Legacy.tsx
, the entrypoint component to legacy support; target of navigation to an legacy endpointIFrameWrapper.tsx
, creates the iframe for kbase-ui and orchestrates startup and monitoring of kbase-uiKBaseUIConnection.ts
, creates the runtime window-message based communication between Europa and kbase-uiAs I developed this and solved various problems, this ended up being a fairly stable and understandable division of labor.
Files affected:
features/legacy/
Legacy.tsx
IFrameWrapper.tsx
LoadingOverlay.tsx
Monitor.ts
ErrorAlert
FallbackNotFound.tsx
ReceiveChannel.ts
SendChannel.ts
TimeoutMonitor.ts
messages.ts
constants.tx
README.md
Refactor to channels
Having implemented window channel synchronization with kbase-ui and iframes and now Narrative service widgets, I think it is a good idea if the techniques involved are as similar as possible.
The challenges involved are very similar.
To this end, I repurposed a pair of classes I’ve used elsewhere,
SendChannel.ts
andReceiveChannel.ts
. They provide a channel-based implementation on top of window messages.The “channel” concept is that by constraining window message data by a common, enforced structure, and a unique identifier shared between Europa and kbase-ui and embedded in that structure, messages are naturally filtered. By encasing this logic in the classes, the implementation of message sending and receiving is simplified, and the logic flow of the application itself is easier to understand.
The main ideas are:
It is notable that kbase-ui did not originally utilize this design, but rather used a single "WindowChannel" class. This is because kbase-ui plugins always operate on the same domain as the kbase-ui host, and therefore messages may be
sent and received between code in the parent and iframe window but utilizing the same window for messages (either parent or iframe). This does not work cross-origin, though, as is the case when kbase-ui is hosted on a subdomain.
But this aspect of the split channel design may be moot at some point in the future, as this set of changes also includes the ability to run kbase-ui on a sub-path rather than sub-domain. (more on this elsewhere.)
Files involved:
features/legacy/
IFrameWrapper.tsx
SendChannel.ts
ReceiveChannel.ts
Communication Design
One of the hearts of the integration is communication between Europa and kbase-ui. The low level api for this communication is DOM window messaging. Within the code, window messaging is abstracted into sending and receiving channels. These channels are used to send and subscribe to messages. It is the exchange of these messages which forms the communication.
Messages sent from Europa to kbase-ui
europa.connect
eruopa.authnavigate
europa.authenticated
europa.deauthenicated
europa.navigate
Messages sent from kbase-ui to Europa
kbase-ui.connect
kbase-ui.connected
kbase-ui.loggedin
kbase-ui.logout
kbase-ui.set-title
kbase-ui.navigated
kbase-ui.redirect
Startup
When Europa needs to invoke a kbase-ui endpoint, it first needs to mount the kbase-ui web app inside an iframe.
Before mounting kbase-ui, Europa sets up a channel for listening to kbase-ui messages. When kbase-ui starts up it sends a message
kbase-ui.connect
along with a channel id. From this point forward, all communication must reference this channel id. This message signifies that kbase-ui has started successfully and is ready to start communicating with Europa.After Europa receives the
kbase-ui.connect
message, it sends aeuropa.connect
message. This message has no payload, but indicates to kbase-ui that Europa is now ready for communication.The final message in this dance is kbase-ui sending
kbase-ui.connected
to indicate that it is fully connected on this channel.First message:
europa.authnavigate
As soon as the connection is established, Europa sends the
europa.authnavigate
message. This message contains a navigation instruction (destination url, essentially) and authentication information. The bundling of these two messages means that authentication and navigation can be coordinated well, avoiding the ui jitter that would result if they were processed separately.For example, after connecting, Europa may send:
Which would cause kbase-ui to authenticate with token “FOO” and then navigate to the user profile page for
kbaseuitest
.Login
Login is a dance between Europa and kbase-ui. Kbase-ui performs the login and obtains a token, but then sends that to Europa which then authenticates the token and sets the app state appropriately.
/legacy/auth2/login
kbase-ui.loggedin
with a payload of token, token expiration, and navigation instructions.europa.authenticated
message with the token and navigation instruction./narratives
Logout
Logout integration is limited, but it is still a dance.
kbase-ui.logout
message.europa.deauthenticated
message to be sent to kbase-ui./legacy/auth2/signedout
Authentication
The integration provides for authentication notification to be sent to kbase-ui. This supports sign-in, as described above, as well as authentication via a cookie change, which can occur with multiple Europa windows open.
europa.authenticated
message to kbase-ui. This message contains the auth token and optionally a location to navigate to after authentication./narratives
.Navigation
Refactor Hooks
As a modern React app, Europa makes extensive use of 3rd party and custom hooks. These hooks are key to the proper operation of legacy support.
Hooks and state changes are used to detect and propagate changes to kbase-ui. This was one of the more fraught areas of the integration, as event detection through changes in state can be quite tricky with hooks with large dependencies. I’ve found the splitting hooks into smaller ones with more limited concerns is easier to debug and understand.
useTokenCookie
was refactored touseInitializeAuthFromCookie
,useSyncAuthStateFromCookie
anduseSyncCookieFromAuthState
useTryAuthFromToken
refactored touseAuthenticateFromToken
useTokenCookie
was invoked only in the top level app,App.tsx
, so a change toApp.tsx
is to invokeuseInitializeAuthFromCookie
,useSyncAuthStateFromCookie
anduseSyncCookieFromAuthState
.useTryAuthFromToken
was used in the legacy support as well asuseTokenCookie
, butuseAuthenticateFromToken
is only used by legacy support. This change helps focus its purpose.useTokenCookie
refactored touseInitializeAuthFromCookie
,useSyncAuthStateFromCookie
anduseSyncCookieFromAuthState
At the top level of the app, in
App.tsx
theuseTokenCookie
hook was called. This hook was responsible for both syncing auth from the primary auth cookie (kbase_session
), clearing the auth cookie in case of an auth query error (potentially an invalid token), and removing app authentication in the case of an empty auth cookie.This is was quite long and complex, over 100 lines long, with many effects. It also mixes two concerns - syncing app auth state to the current cookie and also setting the cookie to auth state!
To facilitate changes to this hook, and to keep my sanity, I split it into three hooks, each named after their primary responsibility.
useInitializeAuthFromCookie
is responsible for ensuring that application authorization state reflects the browser token when the app first starts up. This situation is distinct from the similar task after app startup, because we do not compare the token in the cookie to the token in the auth state, as there is none.useSyncAuthStateFromCookie
is responsible for ensuring the running app auth state reflects any changes to the auth cookie.useSyncCookieFromAuthState
does the converse, ensuring that the auth cookie reflects the current auth state and changes to it.Some functionality from
useTryAuthFromToken
was brought directly intouseSyncAuthStateFromCookie
. This was both because it was problematic to share theuseTryAuthFromToken
hook in two quite different contexts - the legacy support needs a function callback from the hook, and syncing auth state from the cookie does not.useTryAuthFromToken
refactored touseAuthenticateFromToken
Other than the name change, to clarify the purpose, the hook operates differently.
Since the purpose of this hook is to set app auth state based on a token sent as a message from kbase-ui, it is more direct to have this hook return a function for this purpose rather than to rely upon a more indirect approach like the legacy support setting a state variable which is fed into the hook as an argument, and using change detection in the hook to determine if the token has actually changed.
Refactor constant values
To assist in understanding the code, and ensuring that constant values are consistent, I set up a simple solution, a TypeScript module containing exported constants.
Files affected:
features/legacy/contants.ts
app/Routes.tsx
app/Routes.test.tsx
Refactor bare legacy path
The bare legacy path (e.g.
https://ci.kbase.us/legacy
) was causing kbase-ui to be invoked with an empty path. This led to a redirect to/narratives
by kbase-ui. So kbase-ui needed to be loaded, only to navigate back to Europa to display the Narratives Navigator.To improve UX and reduce code complexity, this use case is now handled directly by Europa.
Files affected:
Routes.tsx
Fallback handling and message for PageNotFound page
The "fallback" route seems to have been original designed to handle unhandled paths within kbase-ui. Why not have a simple "LegacyNotFound" component instead? Well, originally the fallback was used to show the "narratives navigator" if kbase-ui was invoked with no path - if kbase-ui were invoked with no path it would redirect to the
/narratives
path.A related change described above, is to have Europa handle the empty legacy path by redirecting to narrative navigators itself. Thus kbase-ui will never see a navigation without a (hash) path.
In addition, in situations in which kbase-ui should route to the narratives navigator, specifically #dashboard and #narratives, kbase-ui will navigate to the default path, https://ENV.kbase.us/narratives in Europa, thus never invoking the
fallback
route.Rather, the fallback route is only invoked, by kbase-ui, in the case of an unsupported (not found) hash path. Since all that is needed is for the fallback path to support a "page not found", that is all it does.
In order to communicate which "page was not found", the (hash) path that was not handled by kbase-ui is passed as in the remainder of the path after
fallback/
and will be utilized in the message displayed byFallbackNotFound
. The latter is a wrapper aroundNotFound
, merely interpreting the current route match, and forming a message for the user.Files involved:
app/Routes.tsx
: fallback/* catchall route handled by FallbackNotFoundfeatures/legacy/FallbackNotFound.tsx
: based on the current catchall param, forms a message for the PageNotFound componentfeatures/layout/PageNotFound.tsx
- modified to show a messagefeatures/layout/PageNotFound.tsx
features/legacy/FallbackNotFound
Regressions
In this section we describe important functionality or user-facing features of kbase-ui that were found absent in Europa. This is not all of the regressions, just the ones which seemed necessary or important to fix.
Parameters in legacy urls
Generally, several locations within kbase-ui which utilize search parameters failed initially because Europa maintains a whitelist of supported query parameters.
If a url is encountered with an unsupported query parameter, it is simply ignored and will probably result in a "page not found”, or some functionality missing, such as not navigating to a target tab, or not propagating a search field. So, as I encountered unsupported search parameters, I updated the
ParamsClass
inparamsSlice.ts
.Examples of functionality disabled by missing params include:
Files affected:
features/params/paramsSlice.ts
Redirect after authentication
A relatively important aspect of authentication UX is that when a user's browser hits an auth-protected resource or a user directly invokes sign-in via the sign-in button, the auth process will capture the current location, and upon
successful authentication, navigate to the original location. This requires the requested URL (or url fragment) to be captured when authentication is detected, to be retained during the authentication process, and restored after authentication is concluded.
This was not supported in the Europa/kbase-ui integration.
Two basic features are required to support this:
The
The solution was to implement an optional post-authentication callback in order, if involved in authentication, can send a navigation message to kbase-ui after authentication has been resolved.
Europa implements this behavior in the following manner:
In addition, to prevent capturing the location of certain locations for which it makes no sense, a blacklist is implemented in the login ui
TopBar.tsx
.Files affected:
features/legacy/Legacy.tsx
features/legacy/IFrameWrapper.tsx
features/auth/hooks.ts
features/layout/TopBar.tsx
Narrative Loader
kbase-ui provided a special
narrative-loader.html
file for safely loading a Narrative when a user does not have an active Narrative container.This functionality is not in the Europa codebase (although it is provided in deployments through other means.)
When a user first launches a Narrative after a period of inactivity (including for the first time), their Narrative container has not yet been launched. The Narrative container launch process can take several 10s of seconds to complete and become available. After the Narrative container is available, an individual Narrative will start almost instantly (although it takes several seconds to load - a different issue.)
During the Narrative container launch process, an attempt to open a Narrative will result in a browser error. Therefore, when the Narrative service detects that a user’s Narrative container needs to be launched, it will redirect the browser to a special url
https://ENV.kbase.us#load-narrative.html?n=NARRATIVE_ID
, where:ENV
is the KBase deployment environmentNARRATIVE_ID
is the Narrative’s workspace idNote that this is a standalone page. This is not strictly necessary, but is the way the Narrative is currently coded to redirect.
In the legacy kbase-ui, this page is loosely integrated into the kbase-ui web app. It is able to load modules from the kbase-ui source tree, just as kbase-ui itself does.
The responsibility of the narrative loader is to attempt to load a resource (the narrative config file) from within the user’s Narrative container, repeatedly attempting to load the resource until it succeeds or a timeout period has elapsed. The resource fetch will return an error initially because the Narrative container is not yet loaded. When the Narrative container loads and the Narrative http server is available, the resource will be successfully returned.
In the updated kbase-ui, this functionality has been modified. It is no longer natural to host
narrative-loader.html
within the kbase-ui web app, as the web app itself is bundled, and the source modules are not available.So the narrative loader was redesigned to simply redirect to a location within kbase-ui -
/legacy/load-narrative?n=NARRRATIVE_ID
.This new location, integrated into kbase-ui, performs the same tasks as
load-narrative.html
and the modules it imports do.In order to support this, a simple html file
load-narrative.html
must be available within the Europa public directory.I’m not sure how this functionality was ported over in the current Europa release. It may be an nginx proxy trick to catch the single endpoint
/load-narrative.html
and route it to the legacy kbase-ui. If this is the case, it will no longer be supported, as that version of kbase-ui is EOL.Issues
Full login/logout responsibility
In the version of kbase-ui utilized in the current version, kbase-ui still conducts some auth session management tasks. This is both a duplication of responsibility, duplication of code, a set of race conditions, and provides opportunity for auth status to become out of sync between the two runtimes.
To address this, all session management code was removed from kbase-ui, and a small amount added to Europa.
After the changes, kbase-ui is still responsible for sign-in, sign-out, and sign-up. It communications the outcome of these processes to Europa. Europa, in turn, is responsible for managing the auth session - browser cookies and it's own runtime state - and communicating changes in this state to kbase-ui.
An example may clarify. Here is the outline of sign-in:
kbase-ui.loggedin
message, along with the Login Token acquired.europa.authenticated
message, with the auth token and optional navigation path (next request).This design prevents kbase-ui from being controlling auth state, while leaving it in charge of acquiring authentication from the auth service. kbase-ui relies upon Europa to inform it that the user's session has been authenticated. This will allow Europa to take over the auth flow at a later time, even when kbase-ui's code for handling sign-in, out, and signup is removed.
Files affected:
KBaseUIConnection.tsx
IFrameWrapper.tsx
europa.authenticated
europa.deauthenticated
kbase-ui.loggedin
to handle post-sign-in actions for Europakbase-out.logout
to handle a logout request from kbase-uiLegacy.tsx
features/auth/hooks.ts
kbase-ui.loggedin
above)Navigation for ORCID Link
ORCID Link was not present in the menu system, so it was added.
Code affected:
features/layout/TopBar.tsx
Navigation Coerced to Uppercase
I have never seen the KBase auth token manipulated in any way (other than being copied.) It has always been clear that the auth token is to be taken as completely opaque, as provided by the auth service api. In fact, when we transitioned from our original auth to auth2, this was one of the elements of the manifesto - we used to parse the token and extract info out of it, but with auth2 the token is opaque.
All we should do with the token is consider it to be present or absent.
I’ve removed that code when I encountered it in
auth/hooks.ts
.Other Improvements
Support legacy on subpath
The current version of Europa/kbase-ui integration calls for kbase-ui to run on a subdomain with the hostname "legacy".
Thus:
https://legacy.ci.kbase.us#foo
However, this is problematic:
Thus, we can use kbase-ui on a path, as in
https://ci.kbase.us/legacy#foo
This is enabled through the configuration environment variable
REACT_APP_KBASE_LEGACY_BASE_PATH
and the configuration file (config.json
) property keylegacy_base_path
. The implementation is located in the new functionlegacyBaseURL
located insrc/features/legacy/utils.ts
.Files affected:
config.json
scripts/build_deploy.sh
src/features/legacy/utils.ts
Note on deploy build script
The deploy build script was altered to support a base path for the URL to the kbase-ui instance.
This is preparatory for being able to move kbase-ui/legacy support from a subdomain to a simple subpath. This effort is discussed above.
Here I just want to note that in order to support this environment variable within the
build_deploy.sh
script, the field separator needed to be changed from a space to a pipe character.The impact is that the configuration string was modified to separate fields by the pipe
|
character, and theIFS
environment variable (Internal Field Separator) likewise needed to be changed to the pipe character.Without this change, an empty environment variable would corrupt the result of the
read
command.