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.
Post History
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...
Answer
#3: Post edited
- **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);- }),
- 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.
- **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.
#2: Post edited
- **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 intercepters 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 of course 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);
- }),
- 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.
- **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);
- }),
- 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.
#1: Initial revision
**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 intercepters 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 of course 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); }), 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.