Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenIddict migration issues #20232

Open
1 task done
agustinsilvano opened this issue Jul 16, 2024 · 1 comment
Open
1 task done

OpenIddict migration issues #20232

agustinsilvano opened this issue Jul 16, 2024 · 1 comment
Labels

Comments

@agustinsilvano
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Description

I started with the migration from Identity Server to OpenIddict, my resources were:

The first two shouldn't be consolidated under the same page, right? I found it confusing having them as separate pages.

Overall the approach was the one suggested by Anto Subash, I created an auxiliary solution with the same name, copied and pasted the AuthServer project in my solution folder, and added the reference to the solution file. The goal is to delete the old IdentityServer Web API to keep things consistent. Then I started to dig into the migration issues.

As far as I know OpenIddict only stores in the final token the sub claim, and the abp team added a set of claims to have the application functional without any extra code.

I had IdentityServer using JwtBearerOptions to validate the token parameters, I was validating the token expiration(exp claim is present in the token but is not coming through when validating the token), and the audience (aud claim that is not present in the token anymore).
Questions here are:

  1. How can I validate the token's expiration on the API side?
  2. I found that there are some methods of OpenIddict for adding audiences to the validators. Should we use the ValidationBuilders on the API side (using separate auth web api project)?

Following with the custom claims, I found that you are adding the claims (and its destinations) for the properties that abp requires.
That includes the tenantid claim, and I can successfully see that claim when decoding my JWT access token. The problem arises when ABP tries to establish the current tenant based on that property, seems like ABP cannot get it.

To overcome the issues described I implemented a claims principal handler to try to fix them:

  1. The exp issue couldn't be fixed, the exp claim is shown when the token is decoded but the delegate that handles the expiration is not receiving that claim. Is some sort of filtering in the middle? Can I write that validation using this instead of the old TokenValidationParameters in Identity Server?

  2. The audiences seem to be a non-standard parameter (ref), and the "closest" is a resources. Do we have the resources available while creating the applications in the OpenIddictDataSeedContributor? Anyway, I tried to add them by hardcoding values but they weren't coming through the token. Can I validate them using the link sent in the bullet above?

  3. The tenantid claim is not being resolved properly although is in the token when decoded. I found that having the following code on my custom implementation of IAbpOpenIddictClaimsPrincipalHandler makes the application able to read the claim and indeed resolve the current tenant properly:

     // Make sure the claims are marked as destinations for the access token
     foreach (var claim in identity.Claims)
     {
        claim.SetDestinations(OpenIddictConstants.Destinations.AccessToken);
     }

    That shows up a lot of claims in the token, even AspNet.Identity.SecurityStamp which is meant to be hidden, right?
    I couldn't figure out why that assignment is doing the trick, I tried to only change the destination of the tenantid claim (as you are already doing in your base code) but it did not work. Might be another claim the one that is affecting the multi-tenancy module to resolve the current tenant.
    Note: I am also sending the __tenant as a header, but the CurrentTenant is not resolved properly either.

  4. A side effect of the code sent in the bullet above, the Roles are not coming with the CurrentUser. This is how the
    CurrentUser looks like at the AppService level with the custom code:
    with_custom_code

    And this is how it looks like without the custom code:
    without_custom_code

    Note that by default the email, phone number, surname, and tenant id are not being mapped. In both scenarios, the option MapInboundClaims is enabled to use the map options that you already have.

Let me know if further information is required to clarify the issues that I'm facing.

Thanks.

Agustin.

Reproduction Steps

No response

Expected behavior

No response

Actual behavior

No response

Regression?

No response

Known Workarounds

No response

Version

8.0.6

User Interface

Common (Default)

Database Provider

EF Core (Default)

Tiered or separate authentication server

Separate Auth Server

Operation System

Windows (Default)

Other information

No response

@agustinsilvano
Copy link
Author

Any idea of the issues raised?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant