-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Additional information for connector login method + additional claims #1400
Comments
I wonder, would it also work for you if dex had a user-info endpoint that included this extra information? The most recent attempt to add one is #1315, I believe. There clearly are many people who want more information 😉 Personally, I'm torn about putting it into claims vs the userinfo endpoint. Bigger ID tokens can easily cause trouble, so I'm leaning a bit towards the latter. What do you think? |
I think there's a place for both. Userinfo is a great place for more information about the user - like photos, contact details, other user-scoped information. For one of the cases that's driving this, we're experimenting with a connector that leans on webauthn. Webauthn allows us to request different levels of information about the user, like presence or verification. Ideally we could specify what level we're after as a scope, and have the returned token indicate this so any token consumers can verify the level it was issued with. We'd also like to be able to do a similar thing with 2fa in our ldap connector - specify a scope indicating if we need a second factor or not, and have the returned token show this state. Doing this via the userinfo endpoint feels a little off to me - this information would be scoped to the individual token that was issued. Trying to return it from the userinfo endpoint would mean we'd need to store the scopes for each credential somewhere, and try and associate it back to the unique credential used to call the userinfo endpoint. While I'm expanding on things, may as well expand on the reasons for passing the Client ID - our upstream auth system contains the entire companies set of users, which is a lot of people. For many systems letting them through is fine, but for some we'd like to enforce some access control at the connector level rather than in the consumer itself. This would allow us to add that check. Overall I'd like to be extending the connector interface in a way that allows the connector to optionally take more responsibility for things, with some reasonable defaults like there are now. As an aside, I know there's been some talk about external connectors (#1020, #1302) but we've been happy with just vendoring dex so far. |
😍 that sounds very interesting. Something that's not entirely clear to me, though -- you're intention is to have a webauthn connector in dex, not to make dex satisfy the webauthn spec, correct?
💯 that's alright; thanks for clarifying. Token claims seem like the better approach indeed.
So what's on your mind, concretely, to be added? I'm just trying to get a grip on this. I think right now, the connector ID already is present in the claims... so, you must be after something more. We've also got a bit of a discussion around opening up the claims to being processed using a general-purpose rule language, via OPA #1378. Maybe there's some sweet spot here serving both purposes... 💭 Thanks for letting me in on your plans btw. I suppose it's potentially difficult to strike a balance between generally-useful-additions and fitting dex into very specific set of requirements, but that doesn't mean that we should just throw in the towel without trying. I'd always lean towards trying; for what it's worth. 😃 |
We use some custom connectors with dex. This has been working fine, but we'd like to extend the functionality a bit. Specifically it would be nice to have the connector make access decisions based on the client id, and be able to return an identity with claims beyond the base set.
Currently this doesn't seem to be possible. In one experiment i've hacked around this by mounting all the handlers in some custom code and capturing the auth ID information, but this isn't particularly nice and doesn't help the additional claims case.
I'd be interested in adding this to dex, but wanted to confirm the approach before doing the work.
For the client id to connector case, my thought was including that in the requests context. This is passed to the
connector.PasswordConnector
'sLogin
method already, and could be added to theconnector.CallbackConnector
'sHandleCallback
method.For the additional claims path, we could add a
ExtraClaims map[string]interface{}
toconnector.Identity
.This should add a path for addressing things like #552 at the connector level.
Thoughts?
The text was updated successfully, but these errors were encountered: