Skip to content
This repository has been archived by the owner on Sep 18, 2021. It is now read-only.

IncludeAllClaimsForUser not working #621

Closed
iMacTia opened this issue Dec 9, 2014 · 5 comments
Closed

IncludeAllClaimsForUser not working #621

iMacTia opened this issue Dec 9, 2014 · 5 comments
Assignees
Labels

Comments

@iMacTia
Copy link

iMacTia commented Dec 9, 2014

Hi there,
I'm currently working with IdentityServer v3 Beta 3-4 and I'm testing one of the latest introduced features:
IncludeAllClaimsForUser.
To test it, I'm using the InMemoryScopeStore like in your samples, so in my Scopes class I'm defining some scope to test my application.
One of this is the classic openid scope, which I defined like this:

new Scope
{
    Name = "openid",
    DisplayName = "Your user identifier",
    Required = true,
    Type = ScopeType.Identity,
    //IncludeAllClaimsForUser = true,
    Claims = new List<ScopeClaim>
    {
        new ScopeClaim("LoginId", true)
    }
}

It's actually working, and when I call the userInfo endpoint, I correctly get

{ "LoginId" : "234" }

But, if I comment the Claims property and instead uncomment the IncludeAllClaimsForUser, I get an empty JSON object...

Am I doing something wrong? Is there any known issue with this functionality?
Thanks in advance

@johnkors
Copy link
Contributor

johnkors commented Dec 9, 2014

If you comment out the Claims property, I guess you've defined a scope without claims?

Also, if you want the scopes defined in the standard, there are some helpers you could use instead of doing it yourself.

https://github.com/thinktecture/Thinktecture.IdentityServer.v3/blob/dev/source/Core/Models/StandardScopes.cs

StandardScopes.ProfileAlwaysInclude
StandardScopes.Profile

@leastprivilege
Copy link
Member

The UserService must support that feature - basically when the include all option is set - we don't send any filter to the GetProfileAsync method on IUserService. This is a sign for the user service to return all claims - but it must be implemented like that.

@iMacTia
Copy link
Author

iMacTia commented Dec 9, 2014

Thanks @johnkors for your link.
Of course this is only for test purposes, I will shortly need to implement my custom Database-based scopes, so I can't use StandardScopes.
However, I had a look at them, and I found this:

        public static Scope AllClaims
        {
            get
            {
                return new Scope
                {
                    Name = Constants.StandardScopes.AllClaims,
                    DisplayName = Resources.Scopes.AllClaimsDisplayName,
                    Type = ScopeType.Identity,
                    Emphasize = true,
                    IncludeAllClaimsForUser = true
                };
            }
        }

So seems that if you enable the IncludeAllClaimsForUser, you don't need to set also the Claims one, also because it is set to an empty List in the constructor.

@leastprivilege : I'm using a custom implementation of MembershipReboot as UserService. I have the "standard" Thinktecture.IdentityServer.v3.MembershipReboot plugin that use a custom UserAccountRepository to manage users. If the GetProfileAsync is part of our custom code, I'll fix it. But I'm wondering if it could be a problem with the MembershipReboot UserService.

Thanks

@iMacTia
Copy link
Author

iMacTia commented Dec 9, 2014

And yeah, I'm taking a look at the MembershipReboot plugin, and it seems that the method GetProfileDataAsync (row 55) doesn't manage this. Am I wrong? It seems that I can't fix it from my code. Should I open an Issue in that Repository?

@leastprivilege
Copy link
Member

Then open an issue on the MR repo - or send a PR. thanks - @brockallen fyi

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

No branches or pull requests

3 participants