Expose the auth0-angular classes for extension points

[alexmach]

Describe the problem you'd like to have solved

Allow for the auth0-angular library to be more extensible/configurable for larger projects.

Describe the ideal solution

• Expose required classes/angular structures to allow for an implementer to customize the usage of the library.
• Expose a testing suite that utilizes the newly exposed classes and other Angular tokens.
• Expose a secondary entry point that contains the mocked classes and test module.

Alternatives and current work-arounds

The only alternative has been to re-implement large chunks of the library.

Additional context

I work on a series of applications that are currently in the process of implementing auth0. The library works well when auth0 is enabled and there is an SPA configured correctly.

Since the team I am on works in many UI applications that all share the same common library which holds our current authentication mechanism we have to be able to implement this authentication upgrade iteratively. We try to avoid long lived branches when possible.

We currently have a solution to do this; however, it was with a bit of effort to achieve this and the recent 1.6.0 release was not backwards compatible with 1.5.1 classes which is why I am now requesting this feature.

Much of the desire is to be able to dynamically change the dependency injection for the Auth0Client and AuthService when our legacy authentication is enabled to be able to take advantage of the AuthHttpInterceptor.

I do not mind submitting a PR with changes if this is something people would want.

As one last note, the auth0 community answer for testing when using this library leaves a lot to be desired. Ideally this solution will meet that need as well. https://community.auth0.com/t/how-to-unit-test-authservice-in-angular-sdk/52569/6

[stevehobbsdev]

Thanks for the feedback @alexmach. We're always looking to improve developer's experiences of our libraries.

Can you elaborate on:

the recent 1.6.0 release was not backwards compatible with 1.5.1 classes

[alexmach]

Certainly! This library definitely was a huge benefit to getting our auth0 implementation going but these changes just caused a little panic until I was able to track down why it was no longer behaving as expected.

The change that was breaking was the constructor change in #180/projects/auth0-angular/src/lib/auth.interceptor.ts

Breaking aspects:

1. Removal of AuthService from the constructor
2. Addition of the InjectionToken Auth0ClientService to the constructor
3. Addition of the AuthState to the constructor

This was a breaking change for how we expected Angular dependency injection to function with the library.

We were left needing two implement the Auth0Client but unable use Angular dependency injection to implement a fake one that conformed to our current authentication because the InjectionToken Auth0ClientService is not publicly exposed. Since Auth0ClientService isn't publicly exposed we would have had to replicate the classes used for the module which would have been been more work for adding the tests needed. We would also have been required to add the @auth0/auth0-spa-js dependency to our internal library.

I would suggest the following changes to make this kind of move easier for people:

• Any class or injection token required for the module be exposed in the public api
• Moving the getAccessTokenSilently method added to the AuthHttpInterceptor to the AuthService but with a new name or giving it additional options to specify the error reporting behavior
• Keep the AuthHttpInterceptor only dependent upon AuthService

Making these changes would also allow for people implementing this library to override any aspect of the module needed including for their tests.

[frederikprijck]

Moving the getAccessTokenSilently from the Interceptor to the AuthService is something we explicitly didn't do as we do not want to expose this method to our users through a public API. Therefor the interceptor stopped using AuthService and uses Auth0Client directly now, which is on purpose and I believe we might want to keep this as is, but happy to discuss further.

Exposing the Auth0ClientService InjectionToken should be fine from our side and should allow you to provide a different instance, could you elaborate why doing that solely wouldn't solve your problem here ?

As a aide note, I am not sure if you are talking about inheriting our services, but personally I would recommend using composition instead, as that would avoid breaking your code when our internal dependencies change, making your application a bit more resilient on it self here.

[alexmach]

Yes we are using composition and have two implementations of the same service one that depends on the Auth0 library and one that depends on our legacy logins.

The biggest challenge with not having the getAccessTokenSilently method from the Interceptor in the AuthService is that one now has to implement multiple classes or have the same instance injected for two different types/tokens. On top of that requiring someone to implement Auth0ClientService requires them to add the dependency to @auth0/auth0-spa-js since those are not exposed as well through the library. Exposing those directly through the library could solve that though.

Out of curiosity why is it desired not to publicly expose that version of the getAccessTokenSilently?

To properly be able to setup the AuthHttpInterceptor it would also require AuthState to be publicly exposed.

[frederikprijck]

Would you be able to provide us with a small example application of what the actual problem with the current version is? Doing so would allow us to better understand what is going on and see how we can resolve that.

Out of curiosity why is it desired not to publicly expose that version of the getAccessTokenSilently?

The method in the interceptor is for internal use in our interceptor only. Generally, a user shouldn't need to call getAccessTokenSilently, and if they ever need to they should use the one on AuthService, not the one inside of the interceptor.

To properly be able to setup the AuthHttpInterceptor it would also require AuthState to be publicly exposed.

Not exposing AuthState publicly is an issue for sure and something I will resolve, together with exposing Auth0ClientService.

[alexmach]

Sorry it took so long to get together.

Main branch contains the 1.5.1 where the interceptor will work.
https://github.com/alexmach/auth0-angular-sample

latest-auth0-angular branch will contain the 1.6.0 branch that prevents the use of the AuthHttpInterceptor
https://github.com/alexmach/auth0-angular-sample/tree/latest-auth0-angular

This is not intended to be our long term solution but just how we were able to manage our transition.

The main reason I would like to see the method getAccessTokenSilently moved into the AuthService is that it would be ideal to only have to know about the classes from this library rather than know about this library and the library this one depends upon.

[frederikprijck]

Thanks for the sample, I haven't been able to actually see the issue with the interceptor as I can't see any HTTP call being involved in the sample.

Would u be able to give some more information with what you mean here (like what do u mean with it isn't getting injected?): https://github.com/alexmach/auth0-angular-sample/blob/latest-auth0-angular/src/app/auth/sample-auth.module.ts#L75

If I understand the repository correctly, you are using two situations where one uses Auth0 and the other one doesn't use Auth0?
And for the one that doesn't use Auth0, you are swapping the Auth0Service and Auth0Guard to use a different instance.

The first thing that comes to mind here is that this sounds like something you might want to solve by introducing your own wrapper around Auth0Service rather than just replacing it. This would avoid code like this: https://github.com/alexmach/auth0-angular-sample/blob/latest-auth0-angular/src/app/auth/token.service.ts#L17-L25.

However, keep in mind our SDK is built to only support Auth0. I know that using your solution is replacing our services in such a way that you are not using the core part of our SDK with InfoLink, instead, you are relying on using our InjectionTokens to provide other instances to ensure it works with things other than Auth0. However, some parts are still being used, such as the Interceptor (which is turning to be problematic now).

This is not intended to be our long-term solution but just how we were able to manage our transition.

Even though I see why these changes make things complicated to do what you are doing, I do not think our SDK should focus on supporting a use case that wants to use our Interceptor with something other than Auth0.

If the interceptor is the problem, I would assume you can get around by creating a different Interceptor for the situation where u are using InfoLink, just as you are doing with all other parts of our SDK. However, I would still advise using a somewhat different approach that does not force you to swap the implementations or our InjectionTokens.

public class MyInterceptor {
  constructor(
    @Inject(AUTH0_ENABLED) private auth0Enabled: boolean, 
    private authHttpInterceptor: AuthHttpInterceptor,
    private legacyAuthHttpInterceptor: LegacyAuthHttpInterceptor
  )
  
  intercept(
    req: HttpRequest<any>,
    next: HttpHandler
  ): Observable<HttpEvent<any>> {
    return this.auth0Enabled ? this.authHttpInterceptor.intercept(req, next) : this.legacyAuthHttpInterceptor.intercept(req, next);
  }
}

Doing the above will ensure our Interceptor isn't used when not using Auth0, which is something we don't explicitly support anyway so it would be safer to not rely on our interceptor when not using Auth0.

Also, as you are re-providing so many things of our SDK already, I believe adding something like this could work as well:

{
      provide: Auth0ClientService,
      useFactory: (enabled:boolean, config: AuthClientConfig) => enabled ? Auth0ClientFactory.createClient(config) : LegacyClientFactory.createClient(),
      deps:[AUTH0_ENABLED, AuthClientConfig],
}

export class LegacyClientFactory {
  createClient() {
    return {
      getTokenSilently: () => {
        return Promise.resolve(sessionStorage.getItem('token'));
      }
    }
  }
}

Where in the above case, you probably only need to implement the getTokenSilently method.
I don't believe the above is ideal and I wouldn't really recommend doing so, but as it's not something we want to be supporting by default it should be acceptable for specific situations like yours.

Note: In order to achieve the above, we do need to export the Auth0ClientService InjectionToken.
On top of that, exporting AuthState is also something I believe we should be doing

To come back to the testing:

As one last note, the auth0 community answer for testing when using this library leaves a lot to be desired. Ideally, this solution will meet that need as well. https://community.auth0.com/t/how-to-unit-test-authservice-in-angular-sdk/52569/6

I do not think that unit testing your application should include our Interceptor. Unit Tests for your application should not try and test our internals, so I can't see why you would want to include the interceptor in your Unit Tests, and if u do I would provide a mock implementation.

[alexmach]

I do understand what we have done isn't quite ideal or how things were intended to be used, I wasn't really happy with how it went either. I am advocating for a few small changes to make a transition easier.

I did start out by writing the wrapper. The main reason I stopped and went with an approach similar to what I sent is that I wanted to reduce differences between the two implementations.

Would u be able to give some more information with what you mean here (like what do u mean with it isn't getting injected?): https://github.com/alexmach/auth0-angular-sample/blob/latest-auth0-angular/src/app/auth/sample-auth.module.ts#L75
I meant that because of the change between 1.5.1 and 1.6.0 the interceptor is injected with Auth0ClientService directly instead of the AuthService from auth0. That is what caused the library to not function the same way for me. Again, I realize this library was not intended for my use case. However, with a few small adjustments this library could support my use case.

Yes we would need Auth0ClientService exposed as well.

As far as testing is concerned the way it was mainly needed because we have a shared library across multiple applications and we configure the Auth0 library in our common library which is used within our tests. If we do not override some of the auth0 library it will attempt to initiate the login flow.

[frederikprijck]

I am advocating for a few small changes to make a transition easier.

The solution being brought up here is introducing a public API with a method we think no user should ever be calling manually. That isn't a small change in terms of surface area and will confuse more people than it helps.

I did start out by writing the wrapper. The main reason I stopped and went with an approach similar to what I sent is that I wanted to reduce differences between the two implementations.

How to implement those things is up to the application side, and I don't believe an SDK should be making changes only to make the implementation easier / more consistent when mixing with other / legacy authentication systems.

If we do not override some of the auth0 library it will attempt to initiate the login flow.

Yes, which I believe is fine. But regarding the Auth0Client from SPAJS, as long as u don't use the Interceptor (which u don't need in a test) and u ensure u mock the Auth0Service, you should be fine (as in, you shouldn't need to mock Auth0Client or Auth0ClientService).

I will make some changes to expose both Auth0ClientService and AuthState, which should allow you to achieve what you want, even though it might require some changes on your side.

[alexmach]

Thanks!

I do understand not wanting to make certain changes.

I appreciate the additional changes to expose more.

My comment about the testing was mainly about it not being completely obvious how it should be setup and it was causing some of our tests issues at first and could very well have just been the pain of initially implementing the library.

I would add that making the transition period easier should be a feature of this library. If it is easier to implement it could help save other people time as well.

I realize it should be a short period of time for a transition and that is how it will be for us. After we have enabled auth0 all of our legacy code will be removed and we will be using the default configurations and this will no longer be a concern for us.

[frederikprijck]

I opened a PR, feel free to have a look and let me know if this is or isn't helping you.