akveo / nebular

:boom: Customizable Angular UI Library based on Eva Design System :new_moon_with_face::sparkles:Dark Mode
https://akveo.github.io/nebular
MIT License
8.06k stars 1.51k forks source link

Adding dynamic values for auth module forRoot() configuration #1125

Open jjgriff93 opened 5 years ago

jjgriff93 commented 5 years ago

Issue type

I'm submitting a ... (check one with "x")

Issue description

Current behaviour: Hi team, happy new year!

We're currently implementing a continuous integration/deployment pipeline for the Angular app from our local dev to Azure test and production web app slots, and we've hit an Ahead of Time compilation challenge.

With JIT compilation we've been defining the callback URL for the auth configuration using document.getElementsByTagName('base')[0].href + 'auth/callback' - which has meant between different deployment slots, the URL has been updated dynamically. However, this method does not work with AoT compilation which we need to now enable in our production & test scenarios, as the window element doesn't exist when it compiles.

We've implemented a configuration API in our ASP.NET Core server that's called when the Angular app is initialised utilising the APPINITIALIZER injection token (https://elanderson.net/2018/05/pass-asp-net-core-appsettings-values-to-angular-via-an-api-call/); this works great for most of the configuration we provide throughout the app as when components are initialised we can get config values from the ConfigService, and this means our server can provide different values depending on the hosting environment, without having to rebuild the app.

The problem we've got is because Nebular auth config is provided in a module decorator as static values:

...NbAuthModule.forRoot({
          strategies: [
            AzureADB2CAuthStrategy.setup({
              name: 'AzureADB2C',
              clientId: environment.azureADB2C.clientId,
              clientSecret: '',
              authorize: {
                endpoint: environment.azureADB2C.endpoint,
                responseType: 'id_token',
                scope: environment.azureADB2C.scope,
                redirectUri: environment.azureADB2C.callbackURL, // was document.getElementsByTagName('base')[0].href + 'auth/callback'
                params: {
                  'p': environment.azureADB2C.policyName
                }
              },
              token: {
                class: AuthAzureToken,
              },
              redirect: {
                success: '/dashboard/overview',
              },
            }),
          ],

And any attempt I've tried to inject the configuration into this that was retrieved by APPINITIALIZER configservice has failed; it seems that the decorators are precompiled and therefore any dynamic values retrieved at runtime can't be inserted into this, always returning null.

We're having to stick with using the static const environment files at the moment and do separate builds for each environment with the different callback URLs and other auth config hardcoded.

Expected behaviour: We would like to be able to provide the Nebular auth configuration dynamically at runtime in a different method than a static forRoot declaration - providing all of these sensitive values in the angular application code doesn't seem to be very production ready or secure. Or any other suggestions of ways around this would be welcome!

Steps to reproduce: Use the ngx-admin template, serve it from a dotnet core server then follow https://elanderson.net/2018/05/pass-asp-net-core-appsettings-values-to-angular-via-an-api-call/ - trying to take the configuration retrieved from the APPINITIALIZER and supplant it into ngAuthModule.forRoot({ ... })

Angular, Nebular

Angular 7.1.4
Nebular 3.0.1
visyone commented 5 years ago

Hi, I also run into this probelm. Waiting for an official solution, there is a workaround?

I have notice that in auth.service.ts file there is a protected method getStrategy(strategyName: string) that return an NbAuthStrategy object and in that there is a setOptions(options: any) method. Making getStrategy(strategyName: string) public could be enough?

Thanks in advance.

jjgriff93 commented 5 years ago

@visyone the only workaround ive done at the moment is to use the angular built in environment.ts files and add configurations for each different build so the redirectURI is updated depending on whether it's ran in dev locally or test and prod web apps - however not an ideal solution as it needs rebuilding each time

visyone commented 5 years ago

@jjgriff93 I also follow the workaround but I need a dynamic configuration for avoid rebuilding. In my spare time I'm trying to implement it.

jjgriff93 commented 5 years ago

Same, please let me know if you have any luck. @nnixa would welcome some thoughts here :)

yggg commented 5 years ago

@nnixaa ping

nnixaa commented 5 years ago

As of now, we can simply inject a particular strategy service, and then use setOptions method to dynamically post the configuration, something like this:

constructor(authStrategy: NbOAuth2AuthStrategy) {
    this.loadConfig().subscribe((config) => {
      authStrategy.setOptions(config)
    });
}

I believe we need to make the getStrategy auth.service method public to improve this.

myleshk commented 5 years ago

Any update on this? Same problem encountered. And @nnixaa setOptions does not change the options of NbPasswordAuthStrategy for me. I tried to put your code in app.component.ts, and several places else, no luck.

nnixaa commented 5 years ago

@myleshk make sure you have only one instance of NbPasswordAuthStrategy in your app, by registering NbAuthModule.forRoot only in the app module. Else post a reproducible to https://stackblitz.com/github/akveo/nebular-seed so that we can check. Thanks.

myleshk commented 5 years ago

@nnixaa Hi I was not sure where to inject the strategy service and call setOptions as you suggested. I'm currently using 2 actions namely login & refresh_token, and tried to inject it in app.comonent.ts, and it does not work. See https://stackblitz.com/edit/nebular-dynamic-auth-api-fail

So finally I have to build a CustomAuthComponent extending NbAuthComponent, inject NbPasswordAuthStrategy and call setOptions there. Also, I have to repeat the whole config there instead of just set baseEndpoint. See https://stackblitz.com/edit/nebular-dynamic-auth-api

Please let me know if you have any better solutions.

visyone commented 5 years ago

@myleshk thanks for your hint. I follow your implementations and now I can use dynamic values for strategy options. Would be great if this configuration can be achieved in AppComponent

jjgriff93 commented 5 years ago

Thanks @nnixaa and @myleshk for sharing some insights here - we redirect to our oauth provider on router guard when initially loading the app if the user isn't authenticated, so I assume I'd need to setOptions in the router-guard and overwrite the ones set in the module. I can imagine this will get quite fragmented if we need to overwrite the app.module.ts forRoot() configuration each time with dynamic values at the component level (there are various places where a user could sign in/out etc.), so would be much more workable if we can declare these dynamic variables forRoot() globally in the app module

stoto34 commented 5 years ago

Hi !

I'm trying to change clientId and clientSecret by using your solution @jjgriff93 . But, there is a problem when I'm trying to log in : clientSecret and clientId properties are not sent in POST request. Any idea ? Thank very much !

Edit : after some rest, I figured out the problem. It was my fault, I didn't declare some variable :)

jjgriff93 commented 5 years ago

Glad to hear you got it resolved @stoto34 :)

ZQ-jhon commented 5 years ago

Hi !

I'm trying to change clientId and clientSecret by using your solution @jjgriff93 . But, there is a problem when I'm trying to log in : clientSecret and clientId properties are not sent in POST request. Any idea ? Thank very much !

Edit : after some rest, I figured out the problem. It was my fault, I didn't declare some variable :)

you need config your NbPasswordAuthStrategyOptions , seem like:

  ...NbAuthModule.forRoot({
        strategies: [
            NbPasswordAuthStrategy.setup({
                name: 'email',
                baseEndpoint: `https://${baseEndpoint}`,
                login: {
                    endpoint: '/token',
                    method: 'post', // set it to 'GET', but it's not security
                    // rememberMe: true,
                    redirect: {
                        success: '/',
                        failure: null,
                    },
                    defaultErrors: ['try again plz'],
                    defaultMessages: ['logging now'],
                },
                token: {
                    class: NbAuthJWTToken,
                    key: 'value',
                },
                register: {},
            }),
        ],
        forms: {},
    }).providers,

good luck~

icesmith commented 5 years ago

Regarding the strategy initialization via setOptions(). I've tried to inject the strategy into the module constructor and call setOptions() there, but without success. The options that are set in the constructor are overriden afterwards somehow. Finally I found out that you should inject NbAuthService as well to the module constructor, because the service initializes the auth strategies during construction. The working solution looks like:

@NgModule({
  imports: [
    ...
    NbAuthModule.forRoot({
      strategies: [
        NbPasswordAuthStrategy.setup({
          name: 'email'
        })
      ],
      ...
    })
  ],
  ...
})
export class AppModule {
  constructor(
    authService: NbAuthService, // force construction of the auth service
    authStrategy: NbPasswordAuthStrategy
  ) {
    this.authStrategy.setOptions({
      name: 'email',
      ... // initialize the strategy here
    });
  }
}
manijak commented 3 years ago

@nnixaa I know it's been 3 years almost since this was posted, but is there any way we can provide these values dynamically today? The examples above do not work in the latest version (v8).

Interesting that this was marked as "Nice to have" but it is actually essential when working with CI tools and have multiple environments. Build once, deploy to all.