feat: request grant confirmation for config updates#39
Conversation
| url: string, | ||
| opClient?: AuthenticatedClient | ||
| ) { | ||
| opClient = opClient ? opClient : await createClient() |
There was a problem hiding this comment.
Nit:
| opClient = opClient ? opClient : await createClient() | |
| opClient ??= await createClient() |
But previous suggestion of caching client might be better. Then don't need to pass the client around as params and can reuse existing one.
Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>
sidvishnoi
left a comment
sidvishnoi
left a comment
There was a problem hiding this comment.
Haven't tested locally, but code looks fine.
| const { getSession, commitSession, destroySession } = | ||
| createCookieSessionStorage({ | ||
| cookie: { | ||
| name: 'wmtools-session', |
There was a problem hiding this comment.
do you think we should add a maxAge or delete the wmtools-session cookie when the session/tab closes?
basically asking the user for a new grant confirmation over a period of time or after the tab closes.
only session gets cleared after a session but not this
There was a problem hiding this comment.
sadly there is no way of detecting when tab closes (browser event onunload fires even when navigating to a different domain, so it would remove the cookie everytime we go to IDP to confirm the grant)
the only solution I see is add maxAge, just need to decide how long that should be ? 10, 30 minutes ? 1h ?
sidvishnoi
deleted the
al/13-request-grant-confirmation-for-config-updates
branch
3 participants
Changes proposed