jeroenheijmans/sample-angular-oauth2-oidc-with-auth-guards

Can the sample assume the callback url is /callback instead of /index.html?

Closed this issue · 3 comments

Thanks for making this. I realize this sample can't be the end all and be all for all users. However, when I was integrating CoreModule into my existing angular app this morning into my app, a big issue was realizing I needed an explicit route for /callback because I don't have a default route like your sample does. The sample worked with my endpoint because of the default route.

I've done Azure AD Oauth a few times, but this is my first rodeo with OAuth that doesn't come with its own library. My junior coworkers who have not were lost.

Anyway, I understand everything is a trade off, but perhaps if the sample got rid of the default route, set an explicit callback (and I guess change the sample IdentityServer to send the callback to /callback and then there was a note "if your callback is / then just move this code to AppComponent"

Let me know what you think and perhaps in early June I could make a PR doing this.

Good to hear the sample was helpful!

If I understand you correctly you'd prefer to change:

redirectUri: window.location.origin + '/index.html',

Into:

  redirectUri: window.location.origin + '/callback',

right? And I guess there should then also be a route at /callback, but what should it point to?

Unless I'm missing something, I prefer to have a slight change into this option, which needs no special route or page:

  redirectUri: window.location.origin + '/',

I think nearly all apps will have a route for /, and bootstrapping logic (such as with oauth2/oidc) is often sensible to do at startup anyways.

Do I understand correctly your app had no route for / and thus using the sample implementation didn't work?

Hope all of this makes sense?

@jeroenheijmans correct. I'll dig into this a little more and do some testing to see if / works.

Hey @zippy1981, while I was doing some other updates I figured to pick this one up as well. It was super straightforward, and the e2e tests give some additional confidence that all is well. I'm inclined to merge as is. Did you ever get to test it in your setup? Run into any problems?