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: identity Provider init() should be triggered async #1415

Conversation

tbouliere-datasolution
Copy link
Contributor

ICMIdentityProvider can trigger a token refresh during the init phase.

this.oAuthServiceConfigured$
.pipe(
whenTruthy(),
take(1),
switchMap(() => merge(this.apiTokenService.restore$(), this.configService.setupRefreshTokenMechanism$()))
)
.subscribe(noop);
}

In the case of optimized SSR rendering, all ngrx storage conditions will be met when the IdentityProviderFactory constructor is called.
As a side effect, token refresh call an http request.
This http request is then intercepted by identity-provider

intercept(req: HttpRequest<unknown>, next: HttpHandler): Observable<HttpEvent<unknown>> {
if (req.url.startsWith(this.appFacade.icmBaseUrl)) {
return this.identityProviderFactory.getInstance()?.intercept(req, next) ?? next.handle(req);
}
return next.handle(req);
}

In the worst case, if all of this chain of effect can be trigger before the constructor initialization and identityProviderFactory will be undefined

Error performing ${grantType} flow TypeError: this.identityProviderFactory.getInstance is not a function
    intercept identity-provider.interceptor.ts:15
    handle Angular
    intercept icm-error-mapper.interceptor.ts:100
    Angular 7
    RxJS 25
    Angular 3
    fetchToken user.service.ts:118
    n/this.fetchAnonymousUserToken$</< user.effects.ts:118
    RxJS 9
    stateSubscription ngrx-store.mjs:486
    RxJS 24
    next ngrx-store.mjs:206
    dispatch ngrx-store.mjs:529
    restore$/< api-token.service.ts:201
    RxJS 51
angular-oauth2-oidc.mjs:1600:28
    fetchTokenUsingGrant Angular
    RxJS 30
    Angular 3
    fetchToken user.service.ts:118
    n/this.fetchAnonymousUserToken$</< user.effects.ts:118
    RxJS 9
    stateSubscription ngrx-store.mjs:486
    RxJS 24
    next ngrx-store.mjs:206
    dispatch ngrx-store.mjs:529
    restore$/< api-token.service.ts:201
    RxJS 82
    init icm.identity-provider.ts:68
    m identity-provider.factory.ts:43
    RxJS 43
TypeError: this.identityProviderFactory.getInstance is not a function
    intercept identity-provider.interceptor.ts:15
    handle Angular
    intercept icm-error-mapper.interceptor.ts:100
    Angular 7
    RxJS 25
    Angular 3
    fetchToken user.service.ts:118
    n/this.fetchAnonymousUserToken$</< user.effects.ts:118
    RxJS 9
    stateSubscription ngrx-store.mjs:486
    RxJS 24
    next ngrx-store.mjs:206
    dispatch ngrx-store.mjs:529
    restore$/< api-token.service.ts:201
    RxJS 51
default-error-handler.ts:19:3
    handleError default-error-handler.ts:19
    handleError default-error-handler.ts:18
    _1 ngrx-effects.mjs:171
    RxJS 6
    Angular 32
    Xi login-status.component.html:32
    Angular 9
    qi login-status.component.html:13
    Angular 49
    n jsonp chunk loading:75
    <anonyme> common.2a5ab1defa44345e.js:1

PR Type

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:

What Is the Current Behavior?

Issue Number: Closes #

What Is the New Behavior?

Does this PR Introduce a Breaking Change?

[ ] Yes
[ ] No

Other Information

@Eisie96 Eisie96 self-requested a review April 14, 2023 12:24
Copy link
Contributor

@Eisie96 Eisie96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to add a inlinine documentation, to make clear, why the setTimeout() function is needed for identity provider factory. Unfortunately i have no permission to push commits onto your given branch. Could you give me permissions to do that or add a inline documentation yourself? :)

@tbouliere-datasolution
Copy link
Contributor Author

@Eisie96 Eisie96 merged commit 61c4b0f into intershop:develop Apr 18, 2023
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.

2 participants