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

Additional information for connector login method + additional claims #1400

Open
lstoll opened this issue Feb 7, 2019 · 3 comments
Open

Comments

@lstoll
Copy link
Contributor

lstoll commented Feb 7, 2019

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's Login method already, and could be added to the connector.CallbackConnector's HandleCallback method.

For the additional claims path, we could add a ExtraClaims map[string]interface{} to connector.Identity.

This should add a path for addressing things like #552 at the connector level.

Thoughts?

@srenatus
Copy link
Contributor

srenatus commented Feb 8, 2019

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.

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?

@lstoll
Copy link
Contributor Author

lstoll commented Feb 8, 2019

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.

@srenatus
Copy link
Contributor

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.

😍 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?

Doing this via the userinfo endpoint feels a little off to me - this information would be scoped to the individual token that was issued.

💯 that's alright; thanks for clarifying. Token claims seem like the better approach indeed.

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.

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. 😃

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

No branches or pull requests

2 participants