Communities

Writing
Writing
Codidact Meta
Codidact Meta
The Great Outdoors
The Great Outdoors
Photography & Video
Photography & Video
Scientific Speculation
Scientific Speculation
Cooking
Cooking
Electrical Engineering
Electrical Engineering
Judaism
Judaism
Languages & Linguistics
Languages & Linguistics
Software Development
Software Development
Mathematics
Mathematics
Christianity
Christianity
Code Golf
Code Golf
Music
Music
Physics
Physics
Linux Systems
Linux Systems
Power Users
Power Users
Tabletop RPGs
Tabletop RPGs
Community Proposals
Community Proposals
tag:snake search within a tag
answers:0 unanswered questions
user:xxxx search by author id
score:0.5 posts with 0.5+ score
"snake oil" exact phrase
votes:4 posts with 4+ votes
created:<1w created < 1 week ago
post_type:xxxx type of post
Search help
Notifications
Mark all as read See all your notifications »
Code Reviews

Welcome to Software Development on Codidact!

Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.

Comments on Setting the authentication token in an Angular application for generated API clients

Parent

Setting the authentication token in an Angular application for generated API clients

+2
−0

This is a post of mine from Code Review Stack Exchange which did not get an answer yet.

I am developing an Angular application that consumes an external REST API. I am using OpenAPI generator (TypeScript template) to generate the API clients.

The happy flow is the following:

  • call a method that requires a username + password and get a token
  • set the token on a configuration object
  • update the generated instance API client with that configuration (which should also receive a base path)
  • use the instance API client to call a method

DataLakeSecurityClientCustomizationService

This takes care of initializing the API clients and setting the token when this is refreshed.

@Injectable({providedIn: "root"})
export class DataLakeSecurityClientCustomizationService {

    constructor(
        private readonly defaultService: DefaultService,
        private readonly securityTagsService: SecurityTagsService,
        /* other generated API client service instances will be here */) {
    }

    init() {
        this.defaultService.configuration.basePath = environment.dataLakeSecurityApiBasePath;
        this.securityTagsService.configuration.basePath = environment.dataLakeSecurityApiBasePath;
    }

    setToken(token: string) {
        this.securityTagsService.configuration.accessToken = token;
        // all other used services will get the token here
    }
}

app.component.ts

  private initDataLakeExtAuth() {
    this.dataLakeCustService.init();

    // trying to get a token ASAP to use for subsequent calls
    this.store.dispatch(RequestNewTokenAction());
  }

  ngOnInit(): void {
    this.initDataLakeExtAuth();
  }

Authentication effect

This handles getting the token which is happening outside of reducer's pure functions. I am using ngrx/store framework (Angular 8 version).

@Injectable()
export class DataLakeExtAuthEffects {

    constructor(
        private readonly actions$: Actions,
        private readonly dataLakeExtAuthService: DataLakeExtAuthService,
        private readonly store: Store<AppState>,
        private readonly logger: LoggingService,
        private readonly dataLakeSecurityClientCustomizationService: DataLakeSecurityClientCustomizationService
    ) { }

    getNewToken$ = createEffect(() => this.actions$.pipe(
        ofType(RequestNewTokenAction),
        switchMap(_ => {
            this.store.dispatch(LoadingStartedAction({ message: "Performing security level authentication ..."}));

            return this.dataLakeExtAuthService.getLoginToken().pipe(
                tap(() => this.store.dispatch(LoadingEndedAction())),
                catchError(err => {
                    this.store.dispatch(LoadingEndedAction());
                    this.logger.logErrorMessage("Error getting a Data Lake Security View token: " + err);

                    this.store.dispatch(NewTokenLoadCancelledAction());
                    return of<string>();
                }));
            }),
        map((token: string) => {
            this.dataLakeSecurityClientCustomizationService.setToken(token);

            return NewTokenLoadedAction({token});
        }))
    );
}

The authentication service for the external REST API

@Injectable({providedIn: "root"})
export class DataLakeExtAuthService {

    credentials = DataLakeCredentials;

    constructor(private readonly defaultService: DefaultService,
                private readonly logger: LoggingService) {
    }

    getLoginToken(): Observable<string> {
        const token$ = this.defaultService.loginTokenPost(this.credentials.username, this.credentials.password)
            .pipe(
                map((payload: { access_token: string }) => payload.access_token),
                tap((token: string) => {
                    this.logger.logTrace("Received token: ", token);
                })
            );
        return token$;
    }
}

DefaultService is an automatically generated TypeScript class that actually performs the API call.

I am looking for improvement suggestions (code quality, performance is less relevant here).

Note: I cannot control what code is generated through the OpenAPI generator.

History
Why does this post require attention from curators or moderators?
You might want to add some details to your flag.
Why should this post be closed?

0 comment threads

Post
+2
−0

Do you need to write this yourself?

That is, are you sure there is no library that could do this for you?

After all, the angular services generated by the typescript-angular language binding of the openapi-generator use angular's HttpClient, whose interceptors offer a neat way to decorate all outgoing calls with the token, and many authentication libraries can provide such an interceptor for you. Personally, I used angular-oauth2-oidc to set up authentication for my generated clients.

Poor Naming

DefaultService is really a rather poor name. You could blame the generator, but it's likely taking the name from the OpenAPI spec you're feeding it. Perhaps you could improve that spec to use a more descriptive name?

Data Race when setting the token

What happens if the login is still pending when the first API request is sent? It appears you are then sending the request without a token, which doesn't seem like a very graceful way to handle this. Wouldn't it be better to delay these requests until a token arrives?

Expired Tokens?

Access Tokens generally have a finite validity, but your code doesn't seem to handle this, merrily sending expired tokens to the API server. Perhaps you should renew tokens?

Service-Configuration After Injection

Configuring a service by injecting it and then setting its configuration properties is fragile, because angular does not know that it must create the configuring service before injecting the configured service elsewhere, where it might hence be used before its configuration is set.

The correct way to provide configuration parameters to a service is to inject them into the service to be configured, so it can initialize itself during construction, and thus before it is injected elsewhere. The api services generated by typescript-angular binding support this, you can simply do:

{provide: BASE_PATH, useValue: url}

or

{provide: Configuration, useValue: {baseUrl: url}}

For details, check the constructor parameters of the generated api services.

Is ngrx worth it here?

Nearly half of the code you posted deals with propagating the events of the login flow through the ngrx store. However, aside from being able to follow the login flow with the ngrx dev tools, I don't see a clear benefit of involving ngrx? Are you sure that benefit justifies the added complexity?

How I'd do it

If I couldn't use a library, I'd do something like this:

@Injectable()
class TokenFetcher {
  constructor(private loginService: LoginService, private credentials: DataLakeCredentials) {}

  currentAccessToken = timer(0, tokenLifetime).pipe(
    map(_ => {
      const {username, password} = this.credentials;
      return loginService.login(username, password)
    }),
    map(response => response.access_token),
    shareReplay(1)
  );
}

@Injectable()
class TokenAttacher implements HttpInterceptor {
  constructor(private tokenFetcher: TokenFetcher) {}

  intercept(req: HttpRequest<any>, next: HttpHandler) {
    if (this.isLoginRequest(req)) {
      // the login request doesn't need an access token
      return next.handle(req);
    } else {
      return this.tokenFetcher.currentAccessToken.pipe(
        first(),
        flatMap(token => {
          const request = req.clone({
            headers: req.headers.set(
              'Authorization', 'Bearer ' + token
            )
          });
          return next.handle(request);
        }
      }
    }
  }
}

This ensures outgoing API requests are delayed until an access token is here. If getting the token fails, an API request is not sent, and the api request fails with the token fetching error (you might want to improve on that ...).

BTW, the above code isn't tested; it's just there to give you an idea.

History
Why does this post require attention from curators or moderators?
You might want to add some details to your flag.

1 comment thread

General comments (2 comments)
General comments
Alexei‭ wrote about 4 years ago

Thanks for the comments, they provide great insight. Ref. to usage of ngrx/store: clearly overkill for this code alone, but this code is part of a much larger application where I have used this pattern.

meriton‭ wrote about 4 years ago

I suspected that :-) I was trying to point that ngrx need not be an all-or-nothing affair. If part of your state is SHARI, and thus benefits from the store, other data might not be. And since the token is neither shared, hydrated, necessarily available when reentering routes, retrieved with a side effect, or impacted by actions from other sources, I'd personally have opted for an easier place to keep it.

Skipping 3 deleted comments.