manfredsteyer / angular-oauth2-oidc

Support for OAuth 2 and OpenId Connect (OIDC) in Angular.
MIT License
1.88k stars 686 forks source link

Cannot read property 'toLowerCase' of null for event type "user_profile_load_error" #828

Open Hasan-git opened 4 years ago

Hasan-git commented 4 years ago

Trying to integrate with Spotify authentication, but getting this error on login image

jeroenheijmans commented 4 years ago

Sorry to hear you're having issues! But a one liner with just an error screenshot is really unfriendly to the community, since it would take a load of effort and guesswork on our part to help you. Please be mindful and share your research, include minimal steps to reproduce the problem, and other details.

I also recommend checking out other similar issues, you might find an answer there.

alvipeo commented 4 years ago

Same here!

I'm using the lib with Azure B2C and here's the set up:

import { AuthConfig } from "angular-oauth2-oidc";

export const DiscoveryDocumentConfig = {
  url:
    "https://smthg.b2clogin.com/{guid}/v2.0/.well-known/openid-configuration?p=b2c_1_susi",
};

export const authConfig: AuthConfig = {
  redirectUri: window.location.origin + "/signedin",
  silentRefreshRedirectUri: window.location.origin + "/silent-refresh.html",
  useSilentRefresh: true,
  responseType: "token id_token",
  issuer:
    "https://smthg.b2clogin.com/{guid}/v2.0/",
  strictDiscoveryDocumentValidation: false,
  tokenEndpoint:
    "https://smthg.b2clogin.com/smthg.onmicrosoft.com/oauth2/v2.0/token?p=b2c_1_susi",
  loginUrl:
    "https://smthg.b2clogin.com/smthg.onmicrosoft.com/oauth2/v2.0/token?p=b2c_1_susi",
  clientId: "{appId}",
  scope:
    "openid offline_access profile https://smthg.onmicrosoft.com/tmvdev-api/api",
  skipIssuerCheck: true,
  clearHashAfterLogin: true,
  oidc: true,

  sessionChecksEnabled: false,  // seems we need it to make it work
  showDebugInformation: true,
};

I see parsed url console output:

{
  "state": "blahblahblah",
  "access_token": "sometoken",
  "token_type": "Bearer",
  "expires_in": "3600",
  "id_token": "some_id_token"
}

next is OAuthSuccessEvent output from auth.service.ts:

{
  "type": "token_received",
  "info": null
}

and then this error:

core.js:6228 ERROR Error: Uncaught (in promise): TypeError: Cannot read property 'toLowerCase' of null
TypeError: Cannot read property 'toLowerCase' of null
    at HttpXsrfInterceptor.intercept (http.js:2812)
    at HttpInterceptorHandler.handle (http.js:1958)
    at HttpInterceptingHandler.handle (http.js:2895)
    at MergeMapSubscriber.project (http.js:1682)
    at MergeMapSubscriber._tryNext (mergeMap.js:46)
    at MergeMapSubscriber._next (mergeMap.js:36)
    at MergeMapSubscriber.next (Subscriber.js:49)
    at Observable._subscribe (subscribeToArray.js:3)
    at Observable._trySubscribe (Observable.js:42)
    at Observable.subscribe (Observable.js:28)
    at resolvePromise (zone-evergreen.js:798)
    at zone-evergreen.js:705
    at SafeSubscriber._error (angular-oauth2-oidc.js:951)
    at SafeSubscriber.__tryOrUnsub (Subscriber.js:183)
    at SafeSubscriber.error (Subscriber.js:135)
    at Subscriber._error (Subscriber.js:75)
    at Subscriber.error (Subscriber.js:55)
    at MapSubscriber._error (Subscriber.js:75)
    at MapSubscriber.error (Subscriber.js:55)
    at FilterSubscriber._error (Subscriber.js:75)
jeroenheijmans commented 4 years ago

@alvipeo I guess you might have the same symptom, but since OP didn't post any repro or info at all, it's a bit hard to know if it's truly the same cause. Thx for sharing a bit more details.

Looking at your stack trace, it seems like it happens in the HttpInterceptor. There's only one specific point where toLowerCase() is used in the interceptor, suggesting a request to a null URL is being made...?

Could you open up your dev tools, find the code I linked, and put a break point there to see what's going on?

(I half expect the userinfo URL to be missing...)

alvipeo commented 4 years ago

ok, it actually happens here.

alvipeo commented 4 years ago

I'm not sure if this helps but redirectUrl for my B2C auth set up is the following:

https://localhost:4200/signedin#state=M2R-...;%252Fsome-route&access_token=some-access-token&token_type=Bearer&expires_in=3600&id_token=some-token-blahblah

and the hash is not cleared even though I have clearHashAfterLogin: true.

alvipeo commented 4 years ago

I tried to set conditional breakpoints everywhere where I could find .toLowerCase() and it seems that this happens outside of the lib 🤔

any suggestions?

alvipeo commented 4 years ago

ok, so angular team replied that:

HttpRequest.url is declared as non-nullable in TypeScript, so a null value for URLs is not supported. Consequently, it is the responsibility of the caller not to create an HttpRequest with a null value at runtime, which appears to be happening here. As such, closing as not a framework issue.

Where do we go from here?

alvipeo commented 4 years ago

guys, I can make a private repo for you if you need to see what's going on. Can't make it public though.

jeroenheijmans commented 4 years ago

this happens outside of the lib

Oh ehmm well... in that case there's not much we can do?

can make a private repo for you if you need to see what's going on. Can't make it public though

Thx for the offer! But I can't personally commit to diving a lot deeper, esp. if it's possibly outside the library. Hope you understand.

If you could reduce it to a smaller repro that would be great, then maybe some community member can have a look? (Though it's usually just me answering questions here.)

angular team replied [null is not supported]

Yeah, I can imagine. Since we determine some of the URLs runtime (e.g. via config), it might be worth to sprinkle some null checks in the lib, to give folks running into this some more logging.

In fact, I suppose that this is the generic solution? (It doesn't imeediately help you, Alvipeo. I realize that.)

alvipeo commented 4 years ago

ok, the problem is here => https://github.com/manfredsteyer/angular-oauth2-oidc/blob/3b9a6e94ed6fdcae1e473cee6aaa408bed328bd5/projects/lib/src/oauth-service.ts#L721

this.userinfoEndpoint is null, therefore the problem. Let me try to find out why... :/

alvipeo commented 4 years ago

and probably here - https://github.com/manfredsteyer/angular-oauth2-oidc/blob/3b9a6e94ed6fdcae1e473cee6aaa408bed328bd5/projects/lib/src/oauth-service.ts#L526

what is this userInfoEndpoint? Put me in context please so I could dig into it.

alvipeo commented 4 years ago

I found the cause of all this pain :) I took this AuthService from here - https://github.com/jeroenheijmans/sample-angular-oauth2-oidc-with-auth-guards/blob/master/src/app/core/auth.service.ts. This sample works with IdentityServer but I use Azure AD B2C.

Anyway, I commented out this subscription and everything seems to be working now:

    // this.oauthService.events
    //   .pipe(filter((e) => ["token_received"].includes(e.type)))
    //   .subscribe((e) => this.oauthService.loadUserProfile());

Please fix your loadUserProfile() method. It must have a validation against undefined/null.

jeroenheijmans commented 4 years ago

Please fix your loadUserProfile() method. It must have a validation against undefined/null.

Fair enough. Although it seems silly to add it to only that call, and not other usages of url.toLowerCase().

EDIT: By that I mean it's a good idea, but I'd love to see robust error handling for all instances of .toLowerCase(), if we'd make a PR to handle htis.

xuanswe commented 4 years ago

I got the same error but it's just unstable. Sometime work, sometime error:

image

tabuckner commented 4 years ago

Please fix your loadUserProfile() method. It must have a validation against undefined/null.

Fair enough. Although it seems silly to add it to only that call, and not other usages of url.toLowerCase().

Why would adding a check against nullish values be silly--or are you mostly just saying that you'd rather wait for the Angular team to adjust how the HttpClient handles nullish url arguments?

jeroenheijmans commented 4 years ago

@tabuckner Wups, I communicated unclearly!

I meant it would be silly to only add it to the one instance we now found. I think it's a great idea, but only if we apply it to all instances of .toLowerCase() in the service.

Will update my previous comment, to clarify it for others.

Augustpi commented 3 years ago

I've same problem before, @alvipeo you should add the following property into the autConfig

export const authConfig: AuthConfig = {
     //  ...,
    userinfoEndpoint: 'https://localhost:9443/oauth2/userinfo',
}
arunkumar-natesan commented 2 years ago

@Augustpi Thanks your solution worked in my case.

alexcpn commented 1 year ago

I've same problem before, @alvipeo you should add the following property into the autConfig

export const authConfig: AuthConfig = {
     //  ...,
    userinfoEndpoint: 'https://localhost:9443/oauth2/userinfo',
}

For Google OIDC

AuthConfig userinfoEndpoint: 'https://www.googleapis.com/oauth2/v3/userinfo ',

DonJuwe commented 1 year ago

Thanks so much @Augustpi. Works for me as well!

BrunoSGomes commented 1 year ago

@Augustpi thanks, helped me a lot

VFiber commented 1 year ago

I think this behavior is a bug in the library, because the userinfo_endpoint is an optional property both in OIDC Connect Discovery and in the library config as well.

Although the flow is faulty, but the reproducing steps are:

The error itself is not very informative and at first it is not obvious what caused it.

Since the userinfo_endpoint it is optional in the discovery metadata, some IDP's might choose not implement it, therefore in the library a null check and / or an error should be thrown when userinfoEndpoint is not set and the loadUserProfile() has been called.

giovannicandido commented 1 year ago

I got this error in a unstable fashion reading the comments here I also debug the cause to be the lack of userinfoEndpoint. I was using the OauthService in two locations, in one authGuard and in the main component. The authguard calls oauthService.configure but I forgot to call this method in the main component and this causes the null url. The documentation is not clear that we need to call this configure method or better add AuthConfig as a service. Authconfig is also optional service injected.

My solution has to add AuthConfig as a provider with the userinfoEndpoint property. I think there is two possible actions for this issue:

  1. Add a error message in the code that will throw if the config is not setted with instructions on how to config it.
  2. Improve the documentation