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: database for default configuration not initializing properly #1094

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kosciolek
Copy link
Contributor

@kosciolek kosciolek commented Jul 19, 2023

Databases for all configuration names are created here: https://github.com/AxaFrance/react-oidc/blob/3e535c94dc57d59956498bfa0ca9536106039bda/packages/react/service_worker/OidcServiceWorker.ts#L288

Before a database is created, there's a check whether it already exists: https://github.com/AxaFrance/react-oidc/blob/3e535c94dc57d59956498bfa0ca9536106039bda/packages/react/service_worker/OidcServiceWorker.ts#L282

But the default database exists since the start: https://github.com/AxaFrance/react-oidc/blob/3e535c94dc57d59956498bfa0ca9536106039bda/packages/react/service_worker/OidcServiceWorker.ts#L41

Which means it won't get properly initialized, so if you do something like this in TrustedDomains.js:

const trustedDomains = {
    default: { domains: [ /* some domains */], showAccessToken: true }
}

the database for default won't show the access token anyway, since it's already initialized to hide the access token:
https://github.com/AxaFrance/react-oidc/blob/3e535c94dc57d59956498bfa0ca9536106039bda/packages/react/service_worker/OidcServiceWorker.ts#L50

I'm not 100% sure that it won't break something else (it seems not to!), but perhaps a maintainer could take a look.

@guillaume-chervet
Copy link
Contributor

Thank you so much @kosciolek !
I may wait another big pull_request merge before your.
It may create some conflicts because all files will move.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants