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

grpc connector #1020

Open
rio opened this issue Aug 9, 2017 · 23 comments
Open

grpc connector #1020

rio opened this issue Aug 9, 2017 · 23 comments

Comments

@rio
Copy link

rio commented Aug 9, 2017

Have there been any plans/thoughts on implementing a grpc connector?
One that will just implement the PasswordConnector and RefreshConnector interfaces.
Just to make it easier for people to use Dex as a frontend for token generation and all that jazz more in line with what Hydra is doing and without needing to fork the project to have your custom connector implemented.

@fforootd
Copy link

fforootd commented Aug 9, 2017

Hi @rio

As of today, we decided to do exactly that in an forked version of dex.
Our idea was to test it first internally and contribute it afterwards.

I will ask the guy working on the connector tomorrow about the status.

@rio
Copy link
Author

rio commented Aug 9, 2017

That's great! I was just implementing it just to see how it would work. Hopefully we could pull you fork back in.

@ericchiang
Copy link
Contributor

ericchiang commented Aug 9, 2017

I think a pluggable connector for out-of-tree development would be great. Committing to an interface is probably the biggest design hurdle here.

@ericchiang
Copy link
Contributor

Out of curiosity, what provider are you trying to hook into? My general experience is that making things pluggable is good but also doesn't magically produce more development. It'd be great to understand what the current pain points are.

@fforootd
Copy link

We internally use a grpc services as part of our identity solution, somewhat like an ldap but it does only store identity's (username, password and user related data).
The reason to not use an LDAP based approach is that we use the same authentication interface for the auth process between our microservices (every microservice gets itself a token from this service)

So we thought about building a more generic grpc connector where one can define his endpoint and the rpc request name from protobuf, as well as the message parameters to map his own values against the connector's.

@rio
Copy link
Author

rio commented Aug 10, 2017

We are in the process to move stuff over to a new architecture but we of course need to be backwards compatible until it's time. One of the things we're running into is not being able to extend the openid tokens with groups and what not in Dex's local password database using the grpc api.

At the same time we were thinking to not store the users in there at all and since we're going to use grpc more and more we thought to try to see if we could connect Dex to our own user storage until we flesh out if and how we want Dex to do password storage for us and if we can live without the groups extendability for example.

We also have a legacy where the combination of username and password will determine which customer this user will be logged into. So same username but different passwords will get you different customers. Transitioning to a single canonical user record will take time as we use those different user ids for permissions. Which is why having extra logic during the grpc login call to our backend would still be necessary.

Or we'll just have to bite the bullet and merge all users first and update our entire permissions system.
What kind of permissions/authz systems are you guys using in combination with Dex?

@ericchiang
Copy link
Contributor

For some background, dex is committed to not becoming an user management or authorization solution. That's why so many of the "local" user objects are so underdeveloped. If we could remove them we would, but they're too important for bootstrapping.

I like the idea of letting external systems manage users without having to implement a crazy protocol like LDAP. I wonder if an establish REST based specification like SCIM might be more appropriate than trying to define our own gRPC API.

http://www.simplecloud.info/

@rio
Copy link
Author

rio commented Aug 11, 2017

Thanks for that link, nice to see some example for user management standardization. However I think that implementing this api will push dex to a more user management function which I feel complicates things. As I noted before, it only needs to be the Login rpc call to be implemented and optionally also Refresh.

Here is an example simple protobuf spec based on the structs and interfaces in https://github.com/coreos/dex/blob/master/connector/connector.go

syntax = "proto3";

package grpc;

message Scopes {
    bool offline_access = 1;
    bool groups = 2;
}

message Identity {
    string user_id = 1;
    string username = 2;
    string email = 3;
    bool email_verified = 4;
    repeated string groups = 5;
}

message LoginRequest {
    string username = 1;
    string password = 2;
    Scopes scopes = 3;
}

message LoginResponse {
    Identity identity = 1;
    bool valid_password = 2;
}

message RefreshRequest {
    Scopes scopes = 1;
    Identity identity = 2;
}

service Connector {
    rpc Login(LoginRequest) returns (LoginResponse) {};
    rpc Refresh(RefreshRequest) returns (Identity) {};
}

This is all we would need to get going.

Edit: Added refresh rpc for completeness.

@rio
Copy link
Author

rio commented Aug 11, 2017

I've commited this and generated code with a version that can complie but not run here btw https://github.com/Rio/dex/tree/1020-grpc-connector

Not sure how far @fforootd is yet but I thought it couldn't hurt.

@livio-a
Copy link

livio-a commented Aug 11, 2017

Hi @rio

We came up with more or less the same solution. As we don't need groups at the time of login (they will be loaded later from an other grpc service), I didn't handle them for now.
For a general connector I would certainly add them as well. Also the scopes and field email_verified could further be added.

So here's our solution: https://github.com/workshop21/dex/tree/1020-grpc-connector
Any suggestions on improvements or what ever would be great...

btw: You asked about the the authz systems we're using. We created as small service (also written in go) where we store permissions (a combination of tenant, user and application) and provide a grpc interface. This interface provides the necessary information for our internal token service which produces a thing that we call "RoleToken". This is basically a JWT with private claims corresponding to the users permissions.
We do this out of the reason because we don't want to provide "id_token" who are getting to big to the users. It also solves a problem with token blacklisting, because these "RoleTokens" tend to have a really short lifetime (about 60s).

@rio
Copy link
Author

rio commented Aug 11, 2017

Whoah great! Looks like it should work nicely. It is at the same level feature wise as the local connector if i'm not mistaken so that would be a good start. I do have some questions though, mostly related to my rookieness and OCD I guess ;)

Why would you not just implement a grpc service inside of the protobuf file and use the generated client for it instead of manually setting the grpc methods in the config and calling grpc.Invoke? I can imagine giving somebody the option to use a different method path because of their own implementation if they don't want to or can use a generated server from the protobuf file. But I feel that by default being able to generate a server from the protobuf is a great time saver.

If the plan is to merge this into upstream I feel we should add scopes, email_verified and groups support just to be complete as you suggested. And of course add tests ;) Having a generated client and server for testing could be handy.

Are you running this in production now? Just curious. Also about authz; I get not wanting to throw around big id_tokens. What kind of authz style are you using RBAC? ABAC? Something else? And what libraries? Sorry for the loads of questions, I'm still figuring out what I want to use for permissions and looking at others is the best thing imho.

@rio
Copy link
Author

rio commented Aug 11, 2017

I also see that google/api/annotations.proto is imported in grpc.proto but not used. You wanted to use grpc-gateway?

@ericchiang
Copy link
Contributor

Quick point: we haven't committed to a gRPC interface so it's way too early to be digging into the implementation.

As we've hit with Kubernetes, making an internal interface external through a webhook or gRPC means that it's almost impossible to change that interface in the future. For example, what if we wanted to add a new, required field to the login response? Today we could make that change all in one place. With this proposal we break compatibility with our external system.

That's why I pointed to solutions like SCIM. At least someone else has done the hard work of establishing the protocol contracts.

I personally don't want to commit to the existing connector interfaces until the end of time. If the proposal here is "create a gRPC interface that mirrors the current Go interfaces" I'm against it.

@fforootd
Copy link

fforootd commented Aug 11, 2017 via email

@fforootd
Copy link

@ericchiang i will quickly clarify what we are trying to achieve.

The gRPC Interface should be just an other connector similar to an ldap connector, which connects to our IDP. Our IDP stores all the necessary information about the user identity and also provides a SCIM interface for identity provisioning with third parties, but this has nothing todo with DEX.

I unterstand your point about not want to commit to the existing Go Interface until end of the world :-)
But in that case, if something changes, i guess it would be sufficient to refactor the gRPC connector like all the others, or do i get something wrong here?

@ericchiang
Copy link
Contributor

The gRPC Interface should be just an other connector similar to an ldap connector, which connects to our IDP. Our IDP stores all the necessary information about the user identity and also provides a SCIM interface for identity provisioning with third parties, but this has nothing todo with DEX.

Yep. I'm trying to identify if there's an actual real protocol we can implement that would allow dex to connect to your IdP, such as SCIM.

I unterstand your point about not want to commit to the existing Go Interface until end of the world :-)
But in that case, if something changes, i guess it would be sufficient to refactor the gRPC connector like all the others, or do i get something wrong here?

We can't control the server side implementation of that gRPC interface (the thing the connector would connect to). Dex would only be able to modify that interface in ways that maintained backwards compatibility with the external interface.

This would prevent us form doing things like refactoring the connector to understand per-user data instead of just per-login data (#863 (comment)).

@glerchundi
Copy link

@ericchiang @rithujohn191

I know that it's preferable to let other specifications do the hard job but I think including this extension point would open up dex to wide variety of integrations.

I tried to draw the current possibilities to attach dex to a custom user database and I found myself connecting a myriad of services down the road (dex->LDAP->{inmem,sql or dex->SCIM->custom connector or ...). It's crazy. This proposal would be a real, easy and practically immediate way to get things done.

So the question is, is there any chance with this issue or should we start thinking in other solutions?

@vyshane
Copy link
Contributor

vyshane commented Feb 19, 2018

I have also been looking at integrating dex with an external user service, and have arrived at a similar solution: A connector that implements the PasswordConnector and RefreshConnector and talks to the external service via gRPC. Then I found this issue.

What can we do to get something like this merged into dex? Would love to not have to maintain a fork of dex just so I can use this connector.

@srenatus
Copy link
Contributor

srenatus commented Sep 6, 2018

I'd like to revive this. Having a generic gRPC connector seems very worthwhile.

@ericchiang @rithujohn191 What do you think, is the concern from above (from #1020 (comment)) still valid, and blocking this?

I would suppose that a future direction could also include some sort of backwards-compatibility (with potential limitations), but in the near term, I think a gPRC connector could unlock many simplifications for adopters. Also, it could be a counter-proposal to all the "enhance user-management" requests. 🤔

Update: Thinking about it, any future changes on the gRPC interface would perhaps be simpler to adjust with for people than what happens right now: complete forks (👋 https://github.com/concourse/dex), or embedding for custom connectors (#1288).

@ericchiang
Copy link
Contributor

If it reduces the number of people asking for user management in dex, I'd be open to exposing a gRPC interface for sending a username and password.

I will note that that's what SCIM is though 😃 (REST instead of gRPC):

http://www.simplecloud.info/
https://tools.ietf.org/html/rfc7644#section-3.2
https://tools.ietf.org/html/rfc7644#section-3.4.1
http://www.simplecloud.info/#Implementations2

@amdonov
Copy link

amdonov commented Sep 6, 2018

I'd like to see the interface support more data than is available in the current Identity object. Assuming dex is going to to support a UserInfo endpoint eventually, it would be nice to have a flexible way to return attributes that are connector specific. For example, birthday, picture, etc.

@ericchiang
Copy link
Contributor

If someone wants to send a SCIM or gRPC PR I'm happy to review it.

@nabokihms
Copy link
Member

Related issue #1907

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

No branches or pull requests

9 participants