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

Recommended way of oauth2 guarding entire application. #478

Closed
ronnyek opened this issue Nov 26, 2018 · 7 comments
Closed

Recommended way of oauth2 guarding entire application. #478

ronnyek opened this issue Nov 26, 2018 · 7 comments
Labels
more-info-needed Please provide a minimal example (e.g. at stackblitz.com) which demonstrates the issue

Comments

@ronnyek
Copy link

ronnyek commented Nov 26, 2018

Greetings.
I've used this library in a number of angular applications before, but I must be doing something wrong, and am hoping I can get to the bottom of this stuff.

Basically I've got applications I'd like to initialize where it'll check for token in whatever storage, and redirect to login immediately.

Currently I've done things like this:

    this.oauthService.configure(this.securityConfig.oidcConfig);
    this.oauthService.tokenValidationHandler = new JwksValidationHandler();

   console.log('attempting to login');
   this.oauthService.loadDiscoveryDocumentAndTryLogin({
        onLoginError: (params: any) => {
          console.log('Error logging in', params);
        },
        onTokenReceived: (receivedTokens: any) => {
          console.log('Logged in, received tokens: ', receivedTokens);
        }}).catch(
        (err: any) => {
          console.log('could not load discovery doc: ', err);
        });

    });

Very frequently I'll end up at a page that just doesn't login, end up with a white screen and console log that says 'attempting to login'. My Identity server reports no errors, but just never goes further than that.

So I guess my question is, if I decide to not have a explicit login/logout button that calls initImplicitFlow(), how do I best fire my app up in such a way that I redirect immediately if no VALID token has been stored?

I tried checking after that loadDiscoveryDocAndTryLogin for a token, and then calling initImplicitFlow(), which seems to help in some cases, and in other, it causes just constant redirects to identity server and then back.

@jeroenheijmans
Copy link
Collaborator

There's nothing wrong with the code you're showing (though I suggest migrating to a promise-based solution over the object with on... handler functions), but on its own it doesn't tell us much.

You could check my sample implementation; I use a variant of that in production without any of the issues you're experiencing.

As for your direct question:

if I decide to not have a explicit login/logout button that calls initImplicitFlow(), how do I best fire my app up in such a way that I redirect immediately if no VALID token has been stored?

I suggest calling initImplicitFlow() at the right time in the login chain. This is in fact exactly what loadDiscoveryDocumentAndLogin(...) does under the hood, which you could use. But I prefer to explicitly state the entire chain myself for all sorts of edge cases (your specific question is handled by uncommenting this line).

@ronnyek
Copy link
Author

ronnyek commented Nov 27, 2018

Wow. That seems like an aweful lot of work around... that if necessary, should be integrated as part of the library...

I dont know if I am just running buggy old version or what... but in just changing that stuff up there to AndLogin(...) ... sometimes it works, sometimes it doesn't. Sometimes I get a nonce validation problem, sometimes it just says attempting to login (but never actually does anything except fetch jwks and well known configuration and then stop

@ronnyek
Copy link
Author

ronnyek commented Nov 27, 2018

For the record, I was attempting this with angular 5 project using 3.1.4 (I think)... but doesn't really seem to matter. I've also noticed that when it does work.... theres a lot of time wrapped up in that initial discovery, and actually redirecting back.

@jeroenheijmans
Copy link
Collaborator

angular 5 project using 3.1.4 (I think)...

Yeah then the docs probably still at some points lean to the onTokenReceived... style, but I've personally preferred the then(...) style.

That seems like an aweful lot of work around...

It is, and it isn't. You might not need all the stuff from my sample setup, you can pick and choose. The repo was meant to document the most extensive case I could fit in one example.

In a lot of cases, perhaps also yours, just doing loadDiscoveryDocumentAndLogin(...) (so no ...TryLogin(...)) would do the trick. The main difference with the other helper function you were using (the tryLogin version) is that it forces a login when no token is available, exactly what you asked for in the original post I believe?

Note that in either case, to prevent weird pages from showing up in your app (if only for a moment), you should hold back on loading your application until the loadDiscoveryDocumentAndLogin() promise resolves (or rejects). The loading is done asynchronously, so you cannot show your app immediately, in a synchronous manner, if you're using implicit flow to determine state.

Still, I agree that the library could benefit from a little more sugar, which I proposed in #359

@ronnyek
Copy link
Author

ronnyek commented May 14, 2019

I guess I just feel like I want to go implicit flow, and the examples in the repo and readmes dont seem to clearly provide examples of that.

Much of my usage of this library (the more I fiddle with it) I occasionally get weird redirect loops, or places where its not even attempting to redirect to login UI at all. I'm confident its probably mis-configuration on my part... but if there are things we can change to make a developer like myself, less prone to shooting myself in the foot... all the better.

@jeroenheijmans
Copy link
Collaborator

Looking back at this thread, most questions got at least some answer, and apart from that it's about guidance for creating apps actually using the library? I'm tempted to close this issue, unless we could boil things down again to a specific question or feature request?

@jeroenheijmans jeroenheijmans added the more-info-needed Please provide a minimal example (e.g. at stackblitz.com) which demonstrates the issue label Aug 4, 2019
@jeroenheijmans
Copy link
Collaborator

Going to keep things tidy and close this issue for now.

If the original question is still open, please let us know what the precise left over question is. If you have another (potentially related) issue: feel free to open a fresh issue with minimal repro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-info-needed Please provide a minimal example (e.g. at stackblitz.com) which demonstrates the issue
Projects
None yet
Development

No branches or pull requests

2 participants