[typescript-angular2] Allow lazy oauth token refresh (#6486)

[alexrashed]

Unfortunately it was a pain to update the previous PR / feature-branch, so here's a fresh branch / PR for ticket #6486 replacing PR #6493.

PR checklist

• Read the contribution guidelines.
• Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
• Filed the PR against the correct branch: master for non-breaking changes and 3.0.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Fixes #6486.
Replaces #6493.

• Add authentication scheme name and necessary scopes as parameters for the accessToken function in the Configuration as seen in [Typescript-Fetch] Support additionalproperties, Enum, Auth and more. #6130
• accessToken function now returns an Observable<string> (allows lazy renewal using the refreshToken)
• Generated ts-ng2, ts-ng4 and the ts-ng security client (the security test client seems like it hasn't been generated for a while)

[alexrashed]

Looks like the automated Travis build container didn't get enough resources (job exceeded the time limit). The builds worked fine from my forked repo / feature branch (same code):

• Travis CI
• Circle CI

[wing328]

@alexrashed I'll restart the CI jobs later to give it another try.

cc @TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx for review.

[wing328]

@alexrashed when you've time, please rebase on the latest master to resolve the merge conflicts.

[alexrashed]

Again the automated Travis build failed, but the one coupled to my forked repo succeeded: https://travis-ci.org/alexrashed/swagger-codegen/builds/280310264

[wing328]

@alexrashed no worry. I'll restart the Travis job.

cc @TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx for review.

[sebastianhaas]

Thanks for the PR, it looks really promising. I'm going to have to check it out and try locally though. Looking forward to using this!

[kenisteward]

@sebastianhaas I looked over this and it seems good. I also haven't tried it locally but I'll try to verify as well with you. from the other PR he made we had a convo about it as I thought it wasn't needed but this changes only brings benefit to the client so I backed down :)

[alexrashed]

I rebased the changes once again and also added a pom file to automatically build the Typescript Angular 4.3 (HttpClient) changes with the samples profile.

[kenisteward]

This should work with lower level angular right? I didn't think there was anything specific to 4.3

…

On Wed, Oct 11, 2017, 3:18 AM Alexander Rashed ***@***.***> wrote: I rebased the changes once again and also added a pom file to automatically build the Typescript Angular 4.3 (HttpClient) changes with the samples profile. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMPLtUJNWIeM9Z9EOQ_kr7LJJMlENWLKks5srGu_gaJpZM4PWhVB> .

[alexrashed]

Of course. But recent changes on the master differ in handling the header depending on the new option for using the new HttpClient. So I rebased my changes, adopted them to correctly set the header fields depending on the option and also added a pom file for the previous changes for HttpClient (which weren't done by me, but I realized that the sample client is not built yet when executing the sample maven profile).

[kenisteward]

Okay thanks and good catch

…

On Wed, Oct 11, 2017, 7:22 AM Alexander Rashed ***@***.***> wrote: Of course. But recent changes on the master differ in handling the header depending on the new option for using the new HttpClient. So I rebased my changes, adopted them to correctly set the header fields depending on the option and also added a pom file for the previous changes for HttpClient (which weren't done by me, but I realized that the sample client is not built yet when executing the sample maven profile). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMPLtVlH8OGPJo9O9dSWloVI2e8aZVw0ks5srKT2gaJpZM4PWhVB> .

[bedag-moo]

This seems rather complicated: Why must we extend the code generator, when the HttpClient makes it easy to add a token to all requests by means of an HttpInterceptor?

My app does not even use the accessToken feature of the code generator, but simply declare

@Injectable()
export class OpenIdConnectHttpInterceptor implements HttpInterceptor {

  constructor(private oauthService: OAuthService) {}

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const headers = req.headers.set("Authorization", "Bearer " + this.oauthService.getAccessToken());
    return next.handle(req.clone({headers}));
  }
}

and register it with

  providers: [
    {provide: HTTP_INTERCEPTORS, useClass: OpenIdConnectHttpInterceptor, multi: true},
  ]

It is also worth noting that the above solution supports repeated subscription on the Observable. For instance, permissions = every5Minutes.switchMap(() => permissionsService.getPermissions()).retry(3) would send each request with a current token, while your implementation bakes the token into the observable, causing each request to be sent with the original token.

That is, idiomatic functional reactive programming is only possible if access tokens are evaluated upon subscription rather than being baked into Observables. This is trivial to achieve with an HttpInterceptor, and hard to achieve in the code generator.

I'd therefore prefer Configuration.accessToken to be deprecated rather than extending its implementation once again to cover more (but not yet all) usecases.

[alexrashed]

Interceptor is a nice pattern / feature which solves the less complex configurations fairly elegant. But an interceptor can't access some data needed for more advanced OAuth swagger configs, f.e. the different scopes for each method.

Also the token isn't static if you assign it a function.
This function is evaluated for every request and therefore is equally powerful as the interceptor in terms of the execution flow.

But I do agree that it gets rather complicated, but I would question the importance of supporting the old http client in a new release (without having a closer look at the PR / issue threads).

[kenisteward]

Adrian I'm with you here but Alex made fine arguments for this feature at the time so I gave in. I would love for @tufu @sebastianhaas to chime in on deprecation of the access token as well since 4.3 let's you handle all this.

…

On Thu, Oct 12, 2017, 6:06 PM Alexander Rashed ***@***.***> wrote: Also the token isn't static if you assign it a function. This function is evaluated for every request and therefore is equally powerful as the interceptor in terms of the execution flow. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMPLtX_5rJUmiTAWCv84ZNeLRiq46Gioks5sro10gaJpZM4PWhVB> .

[bedag-moo]

@alexrashed: The current PR does not support variable scopes per method either, does it? And the interceptor can inspect the entire request, and pick different scopes accordingly. This why I'd make the interceptor part of the application, so developers can easily modify it to suit their needs.

Also the token isn't static if you assign it a function.

I don't think you understood the point I was making. Yes, every invocation of the generated api service method gets a fresh token, but in both Http and HttpClient, a request is sent whenever the returned Observable is subscribed to. This can easily happen more than once when employing RXJS operators. Consider the following code:

currentPermissions = permissionService.getPermissions().repeatWhen(() => every5Minutes);

Here, an Observable is created once, but a request is sent every 5 minutes.

[alexrashed]

The PR does support variable scopes per method. It's pretty much the same as implemented for the typescript-fetch client with PR #6130 (but with observables).
And a single interceptor mapping requests to methods / scopes really doesn't seem to be very clean to me...

The latter however is true, the observable is only created once.

[bedag-moo]

Ah, I didn't notice the arguments added to Configuration.accessToken. Ok, that can be useful and justifies keeping this concern in the code generator. However, I really need token passing to work with RXJS operators, so I could not use this PR in the present form.

[alexrashed]

Since there hasn't been any progress and this PR is heavily outdated, I'm closing it.