feat(nav): make filing button in main nav auth-aware#2758
Conversation
| link.href = `/filing/${config.defaultDocsPeriod}/` | ||
| return link | ||
| }) | ||
| } No newline at end of file |
There was a problem hiding this comment.
The Filing link in the main nav uses this helper function to set its href to /filing/<current-filing-period>. It's unnecessary because our router redirects /filing to the current filing period so it's much cleaner to just have the nav link to /filing.
billhimmelsbach
left a comment
billhimmelsbach
left a comment
There was a problem hiding this comment.
Looking great!
HMDA's sign out flow is weird, and maybe we should just get rid of this whole confirmation page anyway and go straight to login.gov, but it does have this weird "Filing Home" link that still exhibits the old behavior?
Check the user's auth state and if they're logged in, send them to the institutions screen.
id_token_hint was replaced with client_id in #2322 for unknown reasons. Reverting this change causes the "Logout" button in the filing app header to take the user directly to the login.gov logout screen instead of an unnecessary keycloak interstitial page.
contolini
force-pushed
the
5535-filing-nav
branch
from
38e2a9a to
577c636
Compare
@billhimmelsbach I removed this interstitial page via 577c636 per our chat. |
billhimmelsbach
left a comment
billhimmelsbach
left a comment
There was a problem hiding this comment.
Heyo! It's looking good, but there might be a bug here that causes some issues?
When you call store.dispatch in refresh() and forceRefreshToken(), store is actually not defined in that function's scope. The other functions that call it use getStore() first to access it. So if you're logged into the filing app and one of these functions triggers: it'll error, hit the catch block with login(), and then bring ya back to the filing home.
You can test it out by idling on the profile page for awhile (refresh()), or by updating your profile (forceRefreshToken()). You'll see ReferenceError: store is not defined in the console then be taken back to the filing home. I put in some code suggestions where I'd put the getStore() in.
Co-authored-by: Bill Himmelsbach <whimmels@gmail.com>
Co-authored-by: Bill Himmelsbach <whimmels@gmail.com>
Yuuuuup nice catch! Accepting your code suggestions fixes things. Thanks! |
billhimmelsbach
left a comment
billhimmelsbach
left a comment
There was a problem hiding this comment.
Looks good now! Thanks for the work on it!
2 participants
When a user visits
/filing, check their auth state and send them to either the filing "home" screen (the login page) or the institutions screen that lists their institutions.This logic was already in the app but was incomplete. Currently,
HomeContainer.jsxchecks the app's state for the presence of an open ID connect value, which would indicate that the user has authenticated. The user is sent to the login screen if that oidc value isnull. However, it's alwaysnullbecause there's no code anywhere in the codebase that defines it. There's aUSER_FOUNDaction type that defines it but it's not used anywhere. This PR dispatches that action at the appropriate times and includes the user's keycloak info as the payload.Changes
USER_FOUND(viauserFoundaction creator) when the filing app checks forkeycloak.authenticatedand it'strue.USER_SIGNED_OUT(viauserSignedOutaction creator) when authentication is not found by the filing app.HomeContainercomponent is the user's oidc property set?<Link>to keep the entire nav within react router.Testing
Filingand you're not logged in, does it show the login.gov page? After logging in, try clicking around the site and then clicking theFilingnav item. Does it take you directly to your institutions screen?Notes
oidcvalue will be cleared and the user will have to click the login.gov button again when visiting the filing page.