-
Notifications
You must be signed in to change notification settings - Fork 431
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
ServerSide Rendering #36
Comments
Is should be but I have tested it. For the server side rendering you need to replace the sessionStorage with for example cookiestorage. |
This is supported with the setStorage function which needs to be called before the setupModule |
this.oidcSecurityService.setStorage(cookieStorage); |
If this doesn't work, you would have to edit the following class: |
I will add this feature, test, example unless you would like to do it |
@damienbod You don't need to store the token in a cookie for Server side rendering. You only need to not manipulate the DOM and browser objects like cookies and storage. I added the support for SSR with this commit: 29a4901 |
@robisim74 I removed this by mistake. I had to remove the provider from the constructor, but removed too much. Thanks for the info. I'll add it back today. |
@robisim74 fixed in version 1.1.4, thanks for the comment |
@damienbod no problem. But we should also test the library in the Mark Pieszak app: we still have also a lot of console that maybe could create problems if the logging is enabled. |
hi, console are not problem, probably navigation are (window.location). For managing this problem, I think best way is create INavService and provide 2 implementation (server & client) |
@Crazyht You are right, also window.location could be a problem. @damienbod Before creating a new service, we should test if we can detecting it: after the bootstrap, it would not be a problem anymore. |
You can detect client or server side by using PLATFORM_ID available by DI. |
I'd love to help make this happen. I created a test repo to try out the current state of things that I can update to help validate any changes that get made. It looks like @Crazyht was correct, because I get errors related to |
@Crazyht @astegmaier @robisim74 super thanks. The test repo is great. TODO
I would really like to get this working as well. Greetings Damien |
@damienbod I think I can help answer some of those questions:
|
@damienbod I pushed a new branch for SSR: https://github.com/damienbod/angular-auth-oidc-client/tree/SSR Thanks to the test repo of @astegmaier I was able to narrow critical points, and to include them in PLATFORM_ID as suggested by @Crazyht with this commit: f473bb6#diff-3551bf70e40e99dfec0656111f37bf52R67 The problems are in oidcSecuritySilentRenew and in oidcSecurityCheckSession, where you have made extensive use of the DOM: so for the moment I think the only solution is to bypass them: in the end this is a client library and the important thing is that it works when the browser is running. To test the branch, download it and run:
Then you can install it in the test repo:
|
usage of location is problem too. For read you can get information with request abstraction like Mark implement it. For write is more complex because no standard practice is provided by Angular, Little service can use window can client and ??? something on server, no standard way, probably something like transfer data to caller (.NET or nodejs) and server response with HttpRedirect like he want. In my opinion your lib must provide interface which can be override/implement by project. You probably can't find perfect implementation for SSR usable on Node or .NET (without lot of doc on how tweak your server side code). Tank for your reactivity, I test this branch tomorrow:) |
That was fast--thanks! I'll try to test it out on my end to see if it's working. As far as whether to bypass functionality in
@robisim74 do you think your changes as-is will work with these scenarios? I'm happy to help brainstorm /test/look deeper if you think not. |
@Crazyht @robisim74 @astegmaier Really big thanks! I'll test it against browser app. As soon as I get the OK from ye, I'll make the npm package. Greetings Damien |
@astegmaier As said, this library was thinked for the client, not for the server, and its name is angular-auth-oidc-client. However it is important that it does not generate errors during the SSR. If you need actions during the SSR, you should work server side: in theory the server knows and can store the token, it can decode the token, it knows if the token is still valid to know if the user is authenticated; but these things should be developed on the server side.
|
@robisim74 interfaces sound good. We need to maybe rethink the whole silent renew. Have also interesting issues about this in this repo: https://github.com/damienbod/AspNet5IdentityServerAngularImplicitFlow/issues |
I see. Even if the norm is specified like this, I think it does not specify language, right? So we could use Angular Renderer2 class, which allows you to work on the DOM without incurring any errors. |
yes. We can implement silent renew anyway we want. check session needs to follow the norm, but if we use Renderer2, it does not matter. Just the iframe,message is defined, I'll release 1.1.5 as it is, and the interface for the storage and Renderer2 refactor in the next, later versions? |
I'd rather wait for the test result of @astegmaier and @Crazyht for 1.1.5, and yes, then we can rewrite the classes. |
I'll setup the test in this branch: I'll add you as admin, will wait for feedback anyway. @Crazyht @astegmaier |
@robisim74 repo is running with own STS. The window item causes problems. I'll replace the npm package with the src code from the npm. |
@damienbod I took a look at your branch, and I like the changes - I merged them back into my repo, and made a few more to try to hook things up end-to-end. (Was your intention that we both work together on your branch? If so, let me know, and we can move there). My initial results are that the changes @robisim74 made do fix the errors for the initial repo, but that when I try to do a more complete implementation, things break once again. Specifically, when I added an ngOnInit method to my controller receive the auth token (see home.component.ts):
I get a new exception:
Another weird thing I'm seeing: it looks like the tokens are still being stored in @robisim74 Sorry this is a bit of a moving target (and thanks for your help so far!), but I suspect we'll uncover more issues as we flesh out a complete implementation of the pre-rendered client (since the current tests exercise only very basic functionality). I think the right thing to do at this point might be to flesh out the test application with pre-rendering turned off (making sure to exercise all the major functionality of |
Would you like to add the CustomStorage and the changes in the common in a pull request, otherwise I will add it, if it's ok with you? |
@Crazyht How do you solve the problem with the window item? |
WIndow still in code, but server don't receive token so don't try refresh :) I will try with cookie-store may be SSR try refresh if token come in cookie. |
You can add CustomStorage no problem |
@damienbod The test app is not configured yet with authentication server: missing login etc. Before change the library, we should do as said by @astegmaier:
And I think that's what @Crazyht is doing in his repository. However for now the library (1.1.5) doesn't break the SSR: but it will be important to provide the ability to overwrite the storage class: #37 |
Sorry I'm late to the party :) Also is the code being wrapped with isPlatformBrowser() and the service grabbed via the @Component({ /* etc */ })
export class ExampleComponent {
private AIService: AppInsightsService;
private isBrowser: boolean;
constructor(@Inject(PLATFORM_ID) private platformId, private injector: Injector) {
this.isBrowser = isPlatformBrowser(this.platformId);
}
ngOnInit() { // <--
if (this.isBrowser) { // <-- only run if isBrowser
this.AIService = <AppInsightsService>this.injector.get(AppInsightsService); // <-- using the Injector, get the Service
this.AIService.trackEvent('Testing', { 'user': 'me' });
}
}
} Not familiar with this library here, but are you trying to ignore it completely during server-renders? That's the idea with the code above. As for Storage on the server, you could just use some D.I. and swap it out for another Class with getItem/setItem methods on it to be used during the render? Anyway hope that helps! |
Hi @MarkPieszak, thank you for your interest! At the moment we applied the logic above in the library, and we have no more errors during the SSR. The issue has now moved on how to communicate the access token between server and client without the SSR process interfering. |
@robisim74 I'm having trouble getting the test app to run. once it runs, I'll add the login logic. The STS server is configured for this. |
Which problems do you have? However now the problem is not the SSR: they are trying to access to the token from node, and this is not possible: Storage and cookies live in the browser. After I'll refactored the storage, they will able to override it with a custom implementation, so that they will access to the token from node. But this is another issue. |
I have runtime errors in the client app, I'll get this running and add a login |
in test version 1.2.0 |
I made a few tweaks to damien's test fork of the test repo so that I can test out the latest changes end-to-end. As far as server runtime errors, things look great. Thanks so much for addressing this! (I had some comments/concerns about the storage interface changes, but I'll post them on on #37). |
Wondering what the status of this is since there hasn't been any activity here in a while. Does this library work with Universal? Thanks! |
@astegmaier has a running example, correct? Greetings Damien |
I didn't get it to work. I have tried to get it to work in my own test repository here: https://github.com/lgoudriaan/oidc-ssr-example All default angular functionalities break after login, does anyone have an idea what I am doing wrong? |
I never actually got it working end-to-end. However, I was able to follow Priyantha Lankapura's excellent example to add and configure the angular-oauth2-oidc package. I tried it out with the new angular-latest Angular-Cli-based templates, and finally got everything working in this repository. I would bet that it would be relatively easy to use the same approach for Damien's library, but I haven't tried it out yet. |
@lgoudriaan @astegmaier Greetings Damien |
I encountered the same issues described by @lgoudriaan after adding angular-auth-oidc-client into a project based originally on aspnetcore-angular2-universal. Traced the issue to the preboot@5.x library. Migrating preboot to version 6.0.0-beta.1 resolved the issue for me. This preboot commit removes an ApplicationRef isStable check that led to preboot never completing on page load if underlying setTimeout or similar existed (as is the case with this library background timer() calls for token refresh). |
Hi, @esoderquist, |
@lgoudriaan can you paste your solution to this with example please? |
@premiumwd set preboot to ^6.0.0-beta.1 then remove all preboot references. After that add the import That's it :) |
@lgoudriaan getting errors. core.es5.js:1084 ERROR Error: No provider for InjectionToken DocumentToken! Unhandled Promise rejection: No provider for InjectionToken DocumentToken! ; Zone: ; Task: Promise.then ; Value: Error: No provider for InjectionToken DocumentToken! |
I can't help you with this error, you can look at the answer of @astegmaier for starter repositories with SSR. Then Install this library, and to fix the server side rendering upgrade preboot to 6.0 using my instructions above. Hope that helps! |
@lgoudriaan no thanks, built static auth file, and updated my authguard. got it working. |
I've an app using
Here is the relevant code executed by node server which I am not related any of these.
|
@DooMachine Is it possible for you toe share your test project with us? |
|
closing this, please open if you still have an issue |
Hi,
I used angular 4 with asp core server rendering (based on https://github.com/MarkPieszak/aspnetcore-angular2-universal). This library is compliant with server side rendering?
If not what you advise for ?
Thank
The text was updated successfully, but these errors were encountered: