Open

Linked account login skips a user check #9113

alexander.nikonov created 15 days ago

8.1.3 / OpenIDServer / Angular

I have noticed that an inactive login can successfully login from "Linked accounts" dialog. To fix this, I have added the following code into OpenIDServer project:

    public class AbxValidateUserStatusHandler : IOpenIddictServerHandler&lt;ProcessSignInContext&gt;, ITransientDependency
    {
        private readonly IdentityUserManager _userManager;
        private readonly IAbxUserRepository _abxUserRepository;
        private readonly IStringLocalizer&lt;CentralToolsResource&gt; _stringLocalizer;
    
        public AbxValidateUserStatusHandler
        (
            IdentityUserManager userManager,
            IStringLocalizer&lt;CentralToolsResource&gt; stringLocalizer,
            IAbxUserRepository abxUserRepository
        )
        {
            _userManager = userManager;
            _stringLocalizer = stringLocalizer;
            _abxUserRepository = abxUserRepository;
        }
    
        public async ValueTask HandleAsync(ProcessSignInContext context)
        {
            var userId = context.Principal?.FindFirst(AbpClaimTypes.UserId)?.Value;
            if (!string.IsNullOrWhiteSpace(userId))
            {
                var user = await _userManager.FindByIdAsync(userId);
    
                var abxUser = await _abxUserRepository.GetAsync(user.Id);
                if (abxUser != null)
                {
                    var dateNow = DateTime.Now.Date;
                    if (abxUser.ValidFrom.HasValue && dateNow &lt; abxUser.ValidFrom.Value)
                    {
                        context.Reject
                        (
                            error: OpenIddictConstants.Errors.AccessDenied,
                            description: _stringLocalizer.GetString(&quot;Login:InvalidDateForUser&quot;)
                        );
                        return;
                    }
                    if (abxUser.ValidTo.HasValue &amp;&amp; dateNow &gt; abxUser.ValidTo.Value)
                    {
                        context.Reject
                        (
                            error: OpenIddictConstants.Errors.AccessDenied,
                            description: _stringLocalizer.GetString("Login:InvalidDateForUser")
                        );
                        return;
                    }
                }
            }
        }
    }

Basically it works. But not very nice: A) the '/connect/token' response returns code 400 and the following JSON:

    {
      "error": "access_denied",
      "error_description": "This user name is not valid anymore. Please contact your system-administrator"
    }

But error_description is ignored and I only see this: I guess this is the reason:

    this.handleLinkLoginError = (err) => {
            this.toaster.error(err.error?.error); // should be err.error?.error_description ... as everywhere
            return of(null);
    };

B) afterwards, the following ABP method returns js error, because res is null:

    linkLogin(input) {
        return this.identityLinkLoginService.linkLogin(input).pipe(catchError(this.handleLinkLoginError), switchMap(res => {
        if (res.tenant_domain) { // res is NULL!

C) my API calls on home page are invoked (they should not, because nothing has changed - i did not switch to another login) - probably i need to add additional check in my HttpInterceptor to stop this:

    intercept(req: HttpRequest&lt;any&gt;, next: HttpHandler): Observable&lt;HttpEvent&lt;any&gt;> {
        if (this.isLoggingOut && !req.url.endsWith('/connect/revocat')) { // too bad we need to hardcode the exact url for logging out
            return EMPTY;
        }
        //another check to handle unsuccessful linked login?

Go to accepted answer

18 Answer(s)

0

liangshiwei created 13 days ago
Support Team Fullstack Developer

Hi,

Is ValidFrom your custom property?

Could you please share a test project with me? I will check it. thanks. shiwei.liang@volosoft.com

alexander.nikonov created 13 days ago

Hi.

Yes, this is a custom property, but it's not very important here. What is important is to be able to invalidate inactive user not only in LoginModel - when a user tries to log in from Login form (it does work as expected) implemented as follows:

[Dependency(ReplaceServices = true)]
[ExposeServices(typeof(LoginModel), typeof(OpenIddictSupportedLoginModel), typeof(AbxLoginModel))]
public class AbxLoginModel : OpenIddictSupportedLoginModel, ITransientDependency
{
    ...

    public override async Task&lt;IActionResult&gt; OnPostAsync(string action)
    {
        AbxLoginModel supportedLoginModel = this;
        var user = await UserManager.FindByNameAsync(LoginInput.UserNameOrEmailAddress) ?? await UserManager.FindByEmailAsync(LoginInput.UserNameOrEmailAddress);

        if (user != null)
        {
            var abxUser = await _abxUserRepository.GetAsync(user.Id);
            if (abxUser != null)
            {
                var dateNow = DateTime.Now.Date;
                if (abxUser.ValidFrom.HasValue && dateNow &lt; abxUser.ValidFrom.Value)
                {
                    Alerts.Danger(_stringLocalizer.GetString(&quot;Login:InvalidDateForUser&quot;));
                    supportedLoginModel.EnableLocalLogin = true;
                    return supportedLoginModel.Page();
                }
                if (abxUser.ValidTo.HasValue &amp;&amp; dateNow &gt; abxUser.ValidTo.Value)
                {
                    Alerts.Danger(_stringLocalizer.GetString("Login:InvalidDateForUser"));
                    supportedLoginModel.EnableLocalLogin = true;
                    return supportedLoginModel.Page();
                }
            }
        }

        var result = await base.OnPostAsync(action);
        return result;
    }
}

but also when a 'Linked accounts' dialog is shown and a user clicks a linked account login href to login as this user.

Unfortunately, I cannot share the project.

liangshiwei created 13 days ago

Support Team Fullstack Developer

Hi,

It could be a problem, we will improve the error handler.

You can try replacing the LinkLoginHandler service to fix it.

@NgModule({
  declarations: [AppComponent],
  ......
  providers: [
    { provide: LinkLoginHandler, useClass: MyLinkLoginHandler } // 
  ],
  bootstrap: [AppComponent]
})
export class AppModule {}

liangshiwei created 13 days ago

Support Team Fullstack Developer


import { ToasterService } from '@abp/ng.theme.shared';
import { HttpErrorResponse } from '@angular/common/http';
import { inject, Injectable } from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';
import { Public } from '@volo/abp.ng.account/public/proxy';
import { IdentityLinkLoginService, LinkLoginHandler } from '@volo/abp.ng.identity/config';
import { from, of, pipe } from 'rxjs';
import { catchError, filter, switchMap, tap } from 'rxjs/operators';

@Injectable({ providedIn: 'root' })
export class MyLinkLoginHandler {

    private router = inject(Router);
    private route = inject(ActivatedRoute);
    private identityLinkLoginService = inject(IdentityLinkLoginService);
    private toaster = inject(ToasterService);

  private handleLinkLoginError = (err: HttpErrorResponse) => {
    this.toaster.error(err.error?.error_description);
    return of(null);
  };

  private listenToQueryParams() {
    this.route.queryParams
      .pipe(
        filter(params => params.handler === 'linkLogin' && (params.linkUserId || params.token)),
        switchMap(
          (
            params: Public.Web.Areas.Account.Controllers.Models.LinkUserLoginInfo & {
              token?: string;
            },
          ) => {
            if (params.token) {
              Object.entries(JSON.parse(params.token)).forEach(([key, value]: [string, string]) => {
                localStorage.setItem(key, value);
              });
            }

            return params.linkUserId
              ? this.linkLogin(params)
              : of(null).pipe(this.pipeToNavigate());
          },
        ),
      )
      .subscribe();
  }

  private pipeToNavigate() {
    return pipe(
      switchMap(() =>
        from(
          this.router.navigate(['.'], {
            relativeTo: this.route,
            queryParams: { handler: null, linkUserId: null, linkTenantId: null, token: null },
            queryParamsHandling: 'merge',
          }),
        ),
      ),
      tap(() => location.reload()),
    );
  }

  constructor() {
    this.listenToQueryParams();
  }

  linkLogin(input: Public.Web.Areas.Account.Controllers.Models.LinkUserLoginInfo) {
    return this.identityLinkLoginService.linkLogin(input).pipe(
      catchError(this.handleLinkLoginError),
      switchMap(res => {
        if(res == null)
        {
            return of(null);
        }
        if (res.tenant_domain) {
          const now = new Date().valueOf();
          const token = {
            access_token: res.access_token,
            refresh_token: res.refresh_token,
            access_token_stored_at: now,
            expires_at: now + res.expires_in * 1000,
          };
          location.href = `${res.tenant_domain}?handler=linkLogin&token=${JSON.stringify(token)}`;
          return of(null);
        }

        localStorage.setItem('access_token', res.access_token);
        if (res.refresh_token) localStorage.setItem('refresh_token', res.refresh_token);
        return of(null).pipe(this.pipeToNavigate());
      }),
    );
  }
    

}

0
alexander.nikonov created 11 days ago
I've tried this, but it does not work right yet. First of all, this still fails when res is null (when the link login is unsuccessful in my case): localStorage.setItem('access_token', res.access_token);

But even if i prevent this by using:

private handleLinkLoginError = (err: HttpErrorResponse) => { this.toaster.error(err.error?.error_description); return EMPTY; };

there is an (unnecessary) chain of API requests to finally stay under the same user - prior to handleLinkLoginError: there is API request to 'abp/application-configuration' and so on - that usually take place when you visit an ordinary page (i have no idea why this is happening in this scenario).

Instead, the logic in my understanding should be as simple as follows: if the linked user is denied - current UI user should see the corresponding error toaster and nothing extra should happen (no page reloads or other API calls), the UI user should stay at the same page. Maybe for that, the specific logic needs to be implemented on server-side in a different way - not via IOpenIddictServerHandler?
0
alexander.nikonov created 11 days ago
I have tried to use this approach in my HttpInteceptor (instead of handleLinkLoginError, when it's "too late"):

catchError((error: HttpErrorResponse) => { if (error.status === 400 && error.url?.includes('/connect/token') && error.error?.error === 'access_denied') { return EMPTY; } return throwError(() => error); })

In this case, no extra requests (like "/abp/application-configuration") come through as expected. On the other hand, I do not see the original error toaster, because I do not propagate the error. Besides - i am even not sure it would be a right way: maybe reacing on /connect/token like this (even preventing further requests) is an incorrect behavior... In fact, after catching the error like this, I observed some issues when logging in with an active user afterwards.
0

liangshiwei created 11 days ago
Support Team Fullstack Developer

Hi,

You can try
0

alexander.nikonov created 11 days ago

It does not solve the problem:
0

liangshiwei created 11 days ago
Support Team Fullstack Developer

Hi,

What's the problem now
0

alexander.nikonov created 11 days ago

As i noted before, i expect no further API requests coming through after unsuccessful link login. As you can see from the screenshot, despite returning error 400 by connect/token, API come through. Why? The login switch was unsuccessful - all i expect to see is an error toaster.
0

liangshiwei created 11 days ago
Support Team Fullstack Developer

I can't reproduce the problem.

It will get the application information again, which is the correct behavior
0

alexander.nikonov created 11 days ago

I see. So you are confirming, that 1, 2 that follow the /connect/token requests is inevitable even if i do not actually switch to the different login?

Alright, if those ABP requests are required.

But my problem is that as you can see from the screenshot above, there is a bunch of API requests between ABP requests take place.

The one which is important here is only the first request (it causes error 401, which is taken over by our error handler - it redirects to Home page and so on, I will deal with that later).

All these requests are initiated from Home page.The mentioned Home page resides in the external module:

Could you please give some clue / insight what might invoke this first request? In your video, the current page left intact: there are only 'application-configuration' and 'application-localization' requests.

If you do not have ideas / recomendations (from applicable ABP 'best practices' point of view here) - well, it's alright. I will try to investigate myself: there is a lot of custom code here.
0

liangshiwei created 11 days ago
Support Team Fullstack Developer

Hi,

yes, I recommend you create a new project to check it and compare it with your project.
0

alper created 18 hours ago
Support Team Director

hi, we reopened the question
0
alexander.nikonov created 15 hours ago
Hi, thank you for the reopening.

I am afraid I do need your further assistance here. So I managed to eliminate extra API request at Home page which took place between /abp/application-configuration and /abp/application-localization and returning error 401 (for the reason still unknown to me):

However I still cannot figure out everything that followed /connect/token with 400 return code ('token_error'). Since I cannot share our code, and on the other hand it makes no sense to track things on a test example since it is a very simplified version of the code, I will kindly ask you to clarify how ABP works in this workflow and why exactly this way:

why there are duplicating /abp/application-configuration requests here? There are no our requests which might initiate them, so it looks like it is some inner ABP mechanism;

you cannot see it from the ticket, but during /abp/application-configuration request our current page is redirected after unsuccessful link account login change. Even if we are currently on Home page - we are still redirected to Home page again. I have been setting breakpoints in our code, thinking it might be caused by our global error handler, but it responds to other error codes, not 400. So I thought it might have something to do with the way ABP handles this;

when I try to initiate "Link Accounts" dialog from another page (not Home page) - I still see error 401 after 'token_error' response from API requests of this page - just before the mentioned redirects to Home page. Error 401 makes no sense to me in the failed switch link account scenario, because the current user is never unauthorized. I would like you to explain why this could happen. Also, during the aforementioned redirection to the Home page, I briefly see "Welcome to the application. This is a startup project based on the ABP framework. For more information, visit abp.io" markup - which might indicate that the current user is really unauthorized at some point (makes no sense to me).
0

liangshiwei created 4 hours ago
Support Team Fullstack Developer

why there are duplicating /abp/application-configuration requests here? There are no our requests which might initiate them, so it looks like it is some inner ABP mechanism;

Some of them are Preflight requests, it's part of CQRS to pre-check if the server supports cross-domain requests.

https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request

It is automatically sent by the browser, that's why you will see many same requests here.

you cannot see it from the ticket, but during /abp/application-configuration request our current page is redirected after unsuccessful link account login change. Even if we are currently on Home page - we are still redirected to Home page again. I have been setting breakpoints in our code, thinking it might be caused by our global error handler, but it responds to other error codes, not 400. So I thought it might have something to do with the way ABP handles this;

when I try to initiate "Link Accounts" dialog from another page (not Home page) - I still see error 401 after 'token_error' response from API requests of this page - just before the mentioned redirects to Home page. Error 401 makes no sense to me in the failed switch link account scenario, because the current user is never unauthorized. I would like you to explain why this could happen. Also, during the aforementioned redirection to the Home page, I briefly see "Welcome to the application. This is a startup project based on the ABP framework. For more information, visit abp.io" markup - which might indicate that the current user is really unauthorized at some point (makes no sense to me).

Seems like angular will clear the login status after Link Accounts. you can describe what kind of behavior you want, and I will try to help you.
0

alexander.nikonov created 4 hours ago

Replying this right away:

Some of them are Preflight requests, it's part of CQRS to pre-check if the server supports cross-domain requests.

Those are not prefetch. Please pay attention that both of them are with code 200. Prefetch is 204. So those are really duplicates for some reason...
0

liangshiwei created 4 hours ago
Support Team Fullstack Developer

okay, could you share a test project that can reproduce the problem?(you can try use a new project to reproduce) I will check it. thanks.

shiwei.liang@volosoft.com